Bug 1344380 - gtests for b64 bug and some fixes, r=ttaubert NSS_3_30_BRANCH
authorFranziskus Kiefer <franziskuskiefer@gmail.com>
Fri, 17 Mar 2017 11:59:41 +0100
branchNSS_3_30_BRANCH
changeset 13273 ac34db053672
parent 13206 4f07945ac95f
child 13274 ddf576e246ab
push id2135
push userfranziskuskiefer@gmail.com
push dateWed, 05 Apr 2017 13:22:32 +0000
reviewersttaubert
bugs1344380
Bug 1344380 - gtests for b64 bug and some fixes, r=ttaubert Differential Revision: https://nss-review.dev.mozaws.net/D256#inline-2146
gtests/util_gtest/manifest.mn
gtests/util_gtest/util_b64_unittest.cc
gtests/util_gtest/util_gtest.gyp
lib/util/nssb64d.c
lib/util/nssb64e.c
--- a/gtests/util_gtest/manifest.mn
+++ b/gtests/util_gtest/manifest.mn
@@ -3,16 +3,17 @@
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 CORE_DEPTH = ../..
 DEPTH      = ../..
 MODULE = nss
 
 CPPSRCS = \
 	util_utf8_unittest.cc \
+	util_b64_unittest.cc \
 	$(NULL)
 
 INCLUDES += \
 	-I$(CORE_DEPTH)/gtests/google_test/gtest/include \
 	-I$(CORE_DEPTH)/gtests/common \
 	-I$(CORE_DEPTH)/cpputil \
 	$(NULL)
 
new file mode 100644
--- /dev/null
+++ b/gtests/util_gtest/util_b64_unittest.cc
@@ -0,0 +1,79 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
+ * You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#include <climits>
+#include <memory>
+#include "nssb64.h"
+
+#include "gtest/gtest.h"
+#include "scoped_ptrs.h"
+
+namespace nss_test {
+
+class B64EncodeDecodeTest : public ::testing::Test {
+ public:
+  void TestDecodeStr(const std::string &str) {
+    ScopedSECItem tmp(
+        NSSBase64_DecodeBuffer(nullptr, nullptr, str.c_str(), str.size()));
+    ASSERT_TRUE(tmp);
+    char *out = NSSBase64_EncodeItem(nullptr, nullptr, 0, tmp.get());
+    ASSERT_TRUE(out);
+    ASSERT_EQ(std::string(out), str);
+    PORT_Free(out);
+  }
+  bool TestEncodeItem(SECItem *item) {
+    bool rv = true;
+    char *out = NSSBase64_EncodeItem(nullptr, nullptr, 0, item);
+    rv = !!out;
+    if (out) {
+      ScopedSECItem tmp(
+          NSSBase64_DecodeBuffer(nullptr, nullptr, out, strlen(out)));
+      EXPECT_TRUE(tmp);
+      EXPECT_EQ(SECEqual, SECITEM_CompareItem(item, tmp.get()));
+      PORT_Free(out);
+    }
+    return rv;
+  }
+  bool TestFakeDecode(size_t str_len) {
+    std::string str(str_len, 'A');
+    ScopedSECItem tmp(
+        NSSBase64_DecodeBuffer(nullptr, nullptr, str.c_str(), str.size()));
+    return !!tmp;
+  }
+  bool TestFakeEncode(size_t len) {
+    std::vector<uint8_t> data(len, 0x30);
+    SECItem tmp = {siBuffer, data.data(),
+                   static_cast<unsigned int>(data.size())};
+    return TestEncodeItem(&tmp);
+  }
+
+ protected:
+};
+
+TEST_F(B64EncodeDecodeTest, DecEncTest) { TestDecodeStr("VGhpcyBpcyBOU1Mh"); }
+
+TEST_F(B64EncodeDecodeTest, EncDecTest) {
+  uint8_t data[] = {0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09};
+  SECItem tmp = {siBuffer, data, sizeof(data)};
+  TestEncodeItem(&tmp);
+}
+
+TEST_F(B64EncodeDecodeTest, FakeDecTest) { EXPECT_TRUE(TestFakeDecode(100)); }
+
+TEST_F(B64EncodeDecodeTest, FakeEncDecTest) {
+  EXPECT_TRUE(TestFakeEncode(100));
+}
+
+// These takes a while ...
+TEST_F(B64EncodeDecodeTest, LongFakeDecTest1) {
+  EXPECT_TRUE(TestFakeDecode(0x66666666));
+}
+TEST_F(B64EncodeDecodeTest, LongFakeEncDecTest1) { TestFakeEncode(0x3fffffff); }
+TEST_F(B64EncodeDecodeTest, LongFakeEncDecTest2) {
+  EXPECT_FALSE(TestFakeEncode(0x40000000));
+}
+
+}  // namespace nss_test
--- a/gtests/util_gtest/util_gtest.gyp
+++ b/gtests/util_gtest/util_gtest.gyp
@@ -7,17 +7,18 @@
     '../common/gtest.gypi',
   ],
   'targets': [
     {
       'target_name': 'util_gtest',
       'type': 'executable',
       'sources': [
         'util_utf8_unittest.cc',
-        '<(DEPTH)/gtests/common/gtests.cc'
+        'util_b64_unittest.cc',
+        '<(DEPTH)/gtests/common/gtests.cc',
       ],
       'dependencies': [
         '<(DEPTH)/exports.gyp:nss_exports',
         '<(DEPTH)/gtests/google_test/google_test.gyp:gtest',
         '<(DEPTH)/lib/util/util.gyp:nssutil',
         '<(DEPTH)/lib/nss/nss.gyp:nss_static',
         '<(DEPTH)/lib/pk11wrap/pk11wrap.gyp:pk11wrap_static',
         '<(DEPTH)/lib/cryptohi/cryptohi.gyp:cryptohi',
--- a/lib/util/nssb64d.c
+++ b/lib/util/nssb64d.c
@@ -365,17 +365,17 @@ pl_base64_decode_flush(PLBase64Decoder *
 
 /*
  * The maximum space needed to hold the output of the decoder given
  * input data of length "size".
  */
 static PRUint32
 PL_Base64MaxDecodedLength(PRUint32 size)
 {
-    return ((size * 3) / 4);
+    return size * 0.75;
 }
 
 /*
  * A distinct internal creation function for the buffer version to use.
  * (It does not want to specify an output_fn, and we want the normal
  * Create function to require that.)  If more common initialization
  * of the decoding context needs to be done, it should be done *here*.
  */
--- a/lib/util/nssb64e.c
+++ b/lib/util/nssb64e.c
@@ -277,30 +277,38 @@ pl_base64_encode_flush(PLBase64Encoder *
  * line_length bytes (we will add it at nearest lower multiple of 4).
  * There is no trailing CRLF.
  */
 static PRUint32
 PL_Base64MaxEncodedLength(PRUint32 size, PRUint32 line_length)
 {
     PRUint32 tokens, tokens_per_line, full_lines, line_break_chars, remainder;
 
+    /* This is the maximum length we support. */
+    if (size > 0x3fffffff) {
+        return 0;
+    }
+
     tokens = (size + 2) / 3;
 
-    if (line_length == 0)
+    if (line_length == 0) {
         return tokens * 4;
+    }
 
-    if (line_length < 4) /* too small! */
+    if (line_length < 4) { /* too small! */
         line_length = 4;
+    }
 
     tokens_per_line = line_length / 4;
     full_lines = tokens / tokens_per_line;
     remainder = (tokens - (full_lines * tokens_per_line)) * 4;
     line_break_chars = full_lines * 2;
-    if (remainder == 0)
+    if (remainder == 0) {
         line_break_chars -= 2;
+    }
 
     return (full_lines * tokens_per_line * 4) + line_break_chars + remainder;
 }
 
 /*
  * A distinct internal creation function for the buffer version to use.
  * (It does not want to specify an output_fn, and we want the normal
  * Create function to require that.)  All common initialization of the
@@ -442,23 +450,28 @@ PL_Base64EncodeBuffer(const unsigned cha
                       PRUint32 line_length, char *dest, PRUint32 maxdestlen,
                       PRUint32 *output_destlen)
 {
     PRUint32 need_length;
     PLBase64Encoder *data = NULL;
     PRStatus status;
 
     PR_ASSERT(srclen > 0);
-    if (srclen == 0)
+    if (srclen == 0) {
         return dest;
+    }
 
     /*
      * How much space could we possibly need for encoding this input?
      */
     need_length = PL_Base64MaxEncodedLength(srclen, line_length);
+    if (need_length == 0) {
+        PORT_SetError(SEC_ERROR_INVALID_ARGS);
+        return NULL;
+    }
 
     /*
      * Make sure we have at least that much, if output buffer provided.
      */
     if (dest != NULL) {
         PR_ASSERT(maxdestlen >= need_length);
         if (maxdestlen < need_length) {
             PR_SetError(PR_BUFFER_OVERFLOW_ERROR, 0);
@@ -626,16 +639,20 @@ NSSBase64_EncodeItem(PLArenaPool *arenaO
 
     PORT_Assert(inItem != NULL && inItem->data != NULL && inItem->len != 0);
     if (inItem == NULL || inItem->data == NULL || inItem->len == 0) {
         PORT_SetError(SEC_ERROR_INVALID_ARGS);
         return NULL;
     }
 
     max_out_len = PL_Base64MaxEncodedLength(inItem->len, 64);
+    if (max_out_len == 0) {
+        PORT_SetError(SEC_ERROR_INVALID_ARGS);
+        return NULL;
+    }
 
     if (arenaOpt != NULL)
         mark = PORT_ArenaMark(arenaOpt);
 
     if (out_string == NULL) {
         if (arenaOpt != NULL)
             out_string = PORT_ArenaAlloc(arenaOpt, max_out_len + 1);
         else