From 45a83760ec46c918359de239e2465cb3865cc1ff Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Fri, 22 Jun 2018 12:16:09 +0200 Subject: [PATCH] crypto: fix UB in computing max message size MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-By: James M Snell Reviewed-By: Tobias Nießen --- src/node_crypto.cc | 11 ++++------- test/parallel/test-crypto-authenticated.js | 10 ++++++++++ 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 73961fd7a5a1d9..20975513f53911 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -39,7 +39,6 @@ #include #include // INT_MAX -#include #include #include @@ -2802,13 +2801,11 @@ bool CipherBase::InitAuthenticated(const char* cipher_type, int iv_len, if (kind_ == kCipher) 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); - if (iv_len >= static_cast(15.5 - log2(INT_MAX + 1.) / 8)) { - max_message_size_ = (1 << (8 * (15 - iv_len))) - 1; - } else { - max_message_size_ = INT_MAX; - } + max_message_size_ = INT_MAX; + if (iv_len == 12) max_message_size_ = 16777215; + if (iv_len == 13) max_message_size_ = 65535; } else { CHECK_EQ(mode, EVP_CIPH_GCM_MODE); diff --git a/test/parallel/test-crypto-authenticated.js b/test/parallel/test-crypto-authenticated.js index 99dee6f5391227..a4747b2f1f49d3 100644 --- a/test/parallel/test-crypto-authenticated.js +++ b/test/parallel/test-crypto-authenticated.js @@ -1017,3 +1017,13 @@ for (const test of TEST_CASES) { assert.strictEqual(decrypt.update('807022', 'hex', 'hex'), 'abcdef'); 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(); +}