Bug 638297: don't zero-pad buffers, reject short IVs. r=philiKON
authorRichard Newman <rnewman@mozilla.com>
Mon, 07 Mar 2011 11:53:10 -0800
changeset 63344 cf2463e452bac530deb85e22075d02f30ceb560a
parent 63343 bd8f30e6eb18762ba16d90cfc38a5a4770e941a1
child 63345 eff51222337e295a29e087f43064718313570ecc
push id1
push userroot
push dateTue, 10 Dec 2013 15:46:25 +0000
reviewersphiliKON
bugs638297
Bug 638297: don't zero-pad buffers, reject short IVs. r=philiKON
services/crypto/modules/WeaveCrypto.js
services/crypto/tests/unit/test_crypto_crypt.js
--- a/services/crypto/modules/WeaveCrypto.js
+++ b/services/crypto/modules/WeaveCrypto.js
@@ -476,17 +476,19 @@ WeaveCrypto.prototype = {
         return "" + outputBuffer.readString() + "";
     },
 
     _commonCrypt : function (input, inputLength, output, outputLength, symmetricKey, iv, operation) {
         this.log("_commonCrypt() called");
         iv = atob(iv);
 
         // We never want an IV longer than the block size, which is 16 bytes
-        // for AES.
+        // for AES. Neither do we want one smaller; throw in that case.
+        if (iv.length < this.blockSize)
+            throw "IV too short; must be " + this.blockSize + " bytes.";
         if (iv.length > this.blockSize)
             iv = iv.slice(0, this.blockSize);
 
         // We use a single IV SECItem for the sake of efficiency. Fill it here.
         this.byteCompressInts(iv, this._ivSECItemContents, iv.length);
 
         let ctx, symKey, ivParam;
         try {
@@ -632,28 +634,23 @@ WeaveCrypto.prototype = {
     //
     // Utility functions
     //
 
     /**
      * Compress a JS string into a C uint8 array. count is the number of
      * elements in the destination array. If the array is smaller than the
      * string, the string is effectively truncated. If the string is smaller
-     * than the array, the array is 0-padded.
+     * than the array, the array is not 0-padded.
      */
     byteCompressInts : function byteCompressInts (jsString, intArray, count) {
         let len = jsString.length;
         let end = Math.min(len, count);
-
         for (let i = 0; i < end; i++)
-            intArray[i] = jsString.charCodeAt(i) % 256; // convert to bytes
-
-        // Must zero-pad.
-        for (let i = len; i < count; i++)
-            intArray[i] = 0;
+            intArray[i] = jsString.charCodeAt(i) & 0xFF;  // convert to bytes.
     },
 
     // Expand a normal C string (1-byte chars) into a JS string (2-byte chars)
     // EG, for "ABC",  0x41, 0x42, 0x43 --> 0x0041, 0x0042, 0x0043
     byteExpand : function (charArray) {
         let expanded = "";
         let len = charArray.length;
         let intData = ctypes.cast(charArray, ctypes.uint8_t.array(len));
--- a/services/crypto/tests/unit/test_crypto_crypt.js
+++ b/services/crypto/tests/unit/test_crypto_crypt.js
@@ -98,18 +98,18 @@ function test_SECItem_byteCompressInts()
   Components.utils.import("resource://gre/modules/ctypes.jsm");
 
   let item1 = cryptoSvc.makeSECItem("abcdefghi", false);
   do_check_true(!item1.isNull());
   let intData = ctypes.cast(item1.contents.data, ctypes.uint8_t.array(8).ptr).contents;
 
   // Fill it too short.
   cryptoSvc.byteCompressInts("MMM", intData, 8);
-  for (let i = 0; i < 8; ++i)
-    do_check_eq(intData[i], [77, 77, 77, 0, 0, 0, 0, 0, 0][i]);
+  for (let i = 0; i < 3; ++i)
+    do_check_eq(intData[i], [77, 77, 77][i]);
 
   // Fill it too much. Doesn't buffer overrun.
   cryptoSvc.byteCompressInts("NNNNNNNNNNNNNNNN", intData, 8);
   for (let i = 0; i < 8; ++i)
     do_check_eq(intData[i], "NNNNNNNNNNNNNNNN".charCodeAt(i));
 }
 
 function test_encrypt_decrypt() {
@@ -133,16 +133,27 @@ function test_encrypt_decrypt() {
   do_check_eq(clearText, mySecret);
   do_check_neq(cipherText, mySecret); // just to be explicit
 
 
   // Do some more tests with a fixed key/iv, to check for reproducable results.
   key = "St1tFCor7vQEJNug/465dQ==";
   iv  = "oLjkfrLIOnK2bDRvW4kXYA==";
 
+  _("Testing small IV.");
+  mySecret = "YWJjZGVmZ2hpamtsbW5vcHFyc3R1dnd4eXo=";
+  shortiv  = "YWJj";           // "abc": Less than 16.
+  let err;
+  try {
+    cryptoSvc.encrypt(mySecret, key, shortiv);
+  } catch (ex) {
+    err = ex;
+  }
+  do_check_true(!!err);
+
   // Test small input sizes
   mySecret = "";
   cipherText = cryptoSvc.encrypt(mySecret, key, iv);
   clearText = cryptoSvc.decrypt(cipherText, key, iv);
   do_check_eq(cipherText, "OGQjp6mK1a3fs9k9Ml4L3w==");
   do_check_eq(clearText, mySecret);
 
   mySecret = "x";