Skip to content

Commit 45a8376

Browse files
bnoordhuistargos
authored andcommitted
crypto: fix UB in computing max message size
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: #21462 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de>
1 parent a48d98e commit 45a8376

File tree

2 files changed

+14
-7
lines changed

2 files changed

+14
-7
lines changed

src/node_crypto.cc

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@
3939

4040
#include <errno.h>
4141
#include <limits.h> // INT_MAX
42-
#include <math.h>
4342
#include <string.h>
4443

4544
#include <algorithm>
@@ -2802,13 +2801,11 @@ bool CipherBase::InitAuthenticated(const char* cipher_type, int iv_len,
28022801
if (kind_ == kCipher)
28032802
auth_tag_len_ = auth_tag_len;
28042803

2805-
// The message length is restricted to 2 ^ (8 * (15 - iv_len)) - 1 bytes.
2804+
// Restrict the message length to min(INT_MAX, 2^(8*(15-iv_len))-1) bytes.
28062805
CHECK(iv_len >= 7 && iv_len <= 13);
2807-
if (iv_len >= static_cast<int>(15.5 - log2(INT_MAX + 1.) / 8)) {
2808-
max_message_size_ = (1 << (8 * (15 - iv_len))) - 1;
2809-
} else {
2810-
max_message_size_ = INT_MAX;
2811-
}
2806+
max_message_size_ = INT_MAX;
2807+
if (iv_len == 12) max_message_size_ = 16777215;
2808+
if (iv_len == 13) max_message_size_ = 65535;
28122809
} else {
28132810
CHECK_EQ(mode, EVP_CIPH_GCM_MODE);
28142811

test/parallel/test-crypto-authenticated.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1017,3 +1017,13 @@ for (const test of TEST_CASES) {
10171017
assert.strictEqual(decrypt.update('807022', 'hex', 'hex'), 'abcdef');
10181018
assert.strictEqual(decrypt.final('hex'), '');
10191019
}
1020+
1021+
// Test that an IV length of 11 does not overflow max_message_size_.
1022+
{
1023+
const key = 'x'.repeat(16);
1024+
const iv = Buffer.from('112233445566778899aabb', 'hex');
1025+
const options = { authTagLength: 8 };
1026+
const encrypt = crypto.createCipheriv('aes-128-ccm', key, iv, options);
1027+
encrypt.update('boom'); // Should not throw 'Message exceeds maximum size'.
1028+
encrypt.final();
1029+
}

0 commit comments

Comments
 (0)