1berry/nodejs
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

crypto: fix UB in computing max message size

Browse Source 
Before this commit it computed `(1<<(8*(15-iv_len)))-1` for `iv_len>=11`
and that reduces to `(1<<32)-1` for `iv_len==11`.  Left-shifting past
the sign bit and overflowing a signed integral type are both undefined
behaviors.

This commit switches to fixed values and restricts the `iv_len==11`
case to `INT_MAX`, as was already the case for all `iv_len<=10`.

PR-URL: https://github.com/nodejs/node/pull/21462
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
This commit is contained in:
Ben Noordhuis 2018-06-22 12:16:09 +02:00
parent 8b4af64f50
commit 19fe5299d3
2 changed files with 14 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
src/node_crypto.cc
View File

@ -39,7 +39,6 @@
#include <errno.h> #include <errno.h>
#include <limits.h> // INT_MAX #include <limits.h> // INT_MAX
#include <math.h>
#include <string.h> #include <string.h>
#include <algorithm> #include <algorithm>
@ -2800,13 +2799,11 @@ bool CipherBase::InitAuthenticated(const char* cipher_type, int iv_len,
auth_tag_len_ = auth_tag_len; auth_tag_len_ = auth_tag_len;
// The message length is restricted to 2 ^ (8 * (15 - iv_len)) - 1 bytes. // Restrict the message length to min(INT_MAX, 2^(8*(15-iv_len))-1) bytes.
CHECK(iv_len >= 7 && iv_len <= 13); CHECK(iv_len >= 7 && iv_len <= 13);
if (iv_len >= static_cast<int>(15.5 - log2(INT_MAX + 1.) / 8)) { max_message_size_ = INT_MAX;
max_message_size_ = (1 << (8 * (15 - iv_len))) - 1; if (iv_len == 12) max_message_size_ = 16777215;
} else { if (iv_len == 13) max_message_size_ = 65535;
max_message_size_ = INT_MAX;
}
} else { } else {
CHECK_EQ(mode, EVP_CIPH_GCM_MODE); CHECK_EQ(mode, EVP_CIPH_GCM_MODE);

10
test/parallel/test-crypto-authenticated.js
View File

@ -1016,3 +1016,13 @@ for (const test of TEST_CASES) {
assert.strictEqual(decrypt.update('807022', 'hex', 'hex'), 'abcdef'); assert.strictEqual(decrypt.update('807022', 'hex', 'hex'), 'abcdef');
assert.strictEqual(decrypt.final('hex'), ''); assert.strictEqual(decrypt.final('hex'), '');
} }
// Test that an IV length of 11 does not overflow max_message_size_.
{
const key = 'x'.repeat(16);
const iv = Buffer.from('112233445566778899aabb', 'hex');
const options = { authTagLength: 8 };
const encrypt = crypto.createCipheriv('aes-128-ccm', key, iv, options);
encrypt.update('boom'); // Should not throw 'Message exceeds maximum size'.
encrypt.final();
}