crypto: fix UB in computing max message size

bnoordhuis · bnoordhuis · commit 19fe5299d3b5 · 2018-06-25T23:45:14.000+02:00
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: nodejs#21462 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de>
diff --git a/src/node_crypto.cc b/src/node_crypto.cc
@@ -39,7 +39,6 @@
 
 #include <errno.h>
 #include <limits.h>  // INT_MAX
-#include <math.h>
 #include <string.h>
 
 #include <algorithm>
@@ -2800,13 +2799,11 @@ bool CipherBase::InitAuthenticated(const char* cipher_type, int iv_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);
-    if (iv_len >= static_cast<int>(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
@@ -1016,3 +1016,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();
+}
