Bug 1413841 - Check for integer overflow in AesTask::DoCrypto(). r=keeler, a=gchang
authorTim Taubert <ttaubert@mozilla.com>
Tue, 28 Nov 2017 10:00:47 +0100
changeset 445099 9db3839609e097e84106c385e53ed12202d77f8a
parent 445098 87b9a23fc0cfc7b88f03f34f0533e5abb4dabe3d
child 445100 35df0c984d4a2ba59a0eb16cbc073545ed13e80b
push id1618
push userCallek@gmail.com
push dateThu, 11 Jan 2018 17:45:48 +0000
treeherdermozilla-release@882ca853e05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskeeler, gchang
bugs1413841
milestone58.0
Bug 1413841 - Check for integer overflow in AesTask::DoCrypto(). r=keeler, a=gchang Summary: After calling mResult.SetLength(mData.Length() + 16) we should check that the integer addition didn't overflow. It seems at the moment impossible to create ArrayBuffers of size >= 0x0xfffffff0, however adding a check here doesn't hurt. mResult.Length() is passed to the PK11 API functions as a maxOut parameter and /should/ be checked by the softoken crypto algorithm implementations. AES-ECB and AES-GCM seem to do that correctly. Reviewers: keeler Reviewed By: keeler Subscribers: mcote, ttaubert, jcj, keeler Bug #: 1413841 Differential Revision: https://phabricator.services.mozilla.com/D188
dom/crypto/WebCryptoTask.cpp
--- a/dom/crypto/WebCryptoTask.cpp
+++ b/dom/crypto/WebCryptoTask.cpp
@@ -711,22 +711,26 @@ private:
     MOZ_ASSERT(slot.get());
     UniquePK11SymKey symKey(PK11_ImportSymKey(slot.get(), mMechanism,
                                               PK11_OriginUnwrap, CKA_ENCRYPT,
                                               &keyItem, nullptr));
     if (!symKey) {
       return NS_ERROR_DOM_INVALID_ACCESS_ERR;
     }
 
+    // Check whether the integer addition would overflow.
+    if (std::numeric_limits<CryptoBuffer::size_type>::max() - 16 < mData.Length()) {
+      return NS_ERROR_DOM_DATA_ERR;
+    }
+
     // Initialize the output buffer (enough space for padding / a full tag)
-    uint32_t dataLen = mData.Length();
-    uint32_t maxLen = dataLen + 16;
-    if (!mResult.SetLength(maxLen, fallible)) {
+    if (!mResult.SetLength(mData.Length() + 16, fallible)) {
       return NS_ERROR_DOM_UNKNOWN_ERR;
     }
+
     uint32_t outLen = 0;
 
     // Perform the encryption/decryption
     if (mEncrypt) {
       rv = MapSECStatus(PK11_Encrypt(symKey.get(), mMechanism, &param,
                                      mResult.Elements(), &outLen,
                                      mResult.Length(), mData.Elements(),
                                      mData.Length()));