Bug 1396616 - Update nssUTF8_Length to RFC 3629 and fix buffer overrun. r=nss-reviewers,jschanck

Differential Revision: https://phabricator.services.mozilla.com/D139790

8 files changed, 293 insertions, 20 deletions

diff --git a/gtests/base_gtest/Makefile b/gtests/base_gtest/Makefile
new file mode 100644
index 000000000..0d547e080
--- /dev/null
+++ b/gtests/base_gtest/Makefile
@@ -0,0 +1,43 @@
+#! gmake
+#
+# 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/.
+
+#######################################################################
+# (1) Include initial platform-independent assignments (MANDATORY).   #
+#######################################################################
+
+include manifest.mn
+
+#######################################################################
+# (2) Include "global" configuration information. (OPTIONAL)          #
+#######################################################################
+
+include $(CORE_DEPTH)/coreconf/config.mk
+
+#######################################################################
+# (3) Include "component" configuration information. (OPTIONAL)       #
+#######################################################################
+
+
+#######################################################################
+# (4) Include "local" platform-dependent assignments (OPTIONAL).      #
+#######################################################################
+
+include ../common/gtest.mk
+
+#######################################################################
+# (5) Execute "global" rules. (OPTIONAL)                              #
+#######################################################################
+
+include $(CORE_DEPTH)/coreconf/rules.mk
+
+#######################################################################
+# (6) Execute "component" rules. (OPTIONAL)                           #
+#######################################################################
+
+
+#######################################################################
+# (7) Execute "local" rules. (OPTIONAL).                              #
+#######################################################################
diff --git a/gtests/base_gtest/base_gtest.gyp b/gtests/base_gtest/base_gtest.gyp
new file mode 100644
index 000000000..408908f97
--- /dev/null
+++ b/gtests/base_gtest/base_gtest.gyp
@@ -0,0 +1,31 @@
+# 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/.
+{
+  'includes': [
+    '../../coreconf/config.gypi',
+    '../common/gtest.gypi',
+  ],
+  'targets': [
+    {
+      'target_name': 'base_gtest',
+      'type': 'executable',
+      'sources': [
+        'utf8_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:nssutil3',
+        '<(DEPTH)/lib/ssl/ssl.gyp:ssl3',
+        '<(DEPTH)/lib/nss/nss.gyp:nss3',
+        '<(DEPTH)/lib/smime/smime.gyp:smime3',
+        '<(DEPTH)/lib/base/base.gyp:nssb',
+      ]
+    }
+  ],
+  'variables': {
+    'module': 'nss'
+  }
+}
diff --git a/gtests/base_gtest/manifest.mn b/gtests/base_gtest/manifest.mn
new file mode 100644
index 000000000..07506c96c
--- /dev/null
+++ b/gtests/base_gtest/manifest.mn
@@ -0,0 +1,23 @@
+#
+# 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/.
+CORE_DEPTH = ../..
+DEPTH      = ../..
+MODULE = nss
+
+CPPSRCS = \
+      utf8_unittest.cc \
+      $(NULL)
+
+INCLUDES += -I$(CORE_DEPTH)/gtests/google_test/gtest/include \
+            -I$(CORE_DEPTH)/gtests/common \
+            -I$(CORE_DEPTH)/cpputil
+
+REQUIRES = nspr nss libdbm gtest
+
+PROGRAM = base_gtest
+
+EXTRA_LIBS = $(DIST)/lib/$(LIB_PREFIX)gtest.$(LIB_SUFFIX) $(EXTRA_OBJS) \
+             $(DIST)/lib/$(LIB_PREFIX)nssb.$(LIB_SUFFIX) \
+             $(DIST)/lib/$(LIB_PREFIX)gtestutil.$(LIB_SUFFIX)
diff --git a/gtests/base_gtest/utf8_unittest.cc b/gtests/base_gtest/utf8_unittest.cc
new file mode 100644
index 000000000..16e01cdab
--- /dev/null
+++ b/gtests/base_gtest/utf8_unittest.cc
@@ -0,0 +1,150 @@
+/* -*- 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 "gtest/gtest.h"
+
+#include "nss.h"
+#include "base.h"
+#include "secerr.h"
+
+namespace nss_test {
+
+class Utf8Test : public ::testing::Test {};
+
+// Tests nssUTF8_Length rejects overlong forms, surrogates, etc.
+TEST_F(Utf8Test, Utf8Length) {
+  PRStatus status;
+
+  EXPECT_EQ(0u, nssUTF8_Length("", &status));
+  EXPECT_EQ(PR_SUCCESS, status);
+
+  // U+0000..U+007F
+  EXPECT_EQ(1u, nssUTF8_Length("\x01", &status));
+  EXPECT_EQ(PR_SUCCESS, status);
+  EXPECT_EQ(1u, nssUTF8_Length("\x7F", &status));
+  EXPECT_EQ(PR_SUCCESS, status);
+
+  // lone trailing byte
+  EXPECT_EQ(0u, nssUTF8_Length("\x80", &status));
+  EXPECT_EQ(PR_FAILURE, status);
+  EXPECT_EQ(NSS_ERROR_INVALID_STRING, NSS_GetError());
+  EXPECT_EQ(0u, nssUTF8_Length("\xBF", &status));
+  EXPECT_EQ(PR_FAILURE, status);
+  EXPECT_EQ(NSS_ERROR_INVALID_STRING, NSS_GetError());
+
+  // overlong U+0000..U+007F
+  EXPECT_EQ(0u, nssUTF8_Length("\xC0\x80", &status));
+  EXPECT_EQ(PR_FAILURE, status);
+  EXPECT_EQ(NSS_ERROR_INVALID_STRING, NSS_GetError());
+  EXPECT_EQ(0u, nssUTF8_Length("\xC1\xBF", &status));
+  EXPECT_EQ(PR_FAILURE, status);
+  EXPECT_EQ(NSS_ERROR_INVALID_STRING, NSS_GetError());
+
+  // U+0080..U+07FF
+  EXPECT_EQ(2u, nssUTF8_Length("\xC2\x80", &status));
+  EXPECT_EQ(PR_SUCCESS, status);
+  EXPECT_EQ(2u, nssUTF8_Length("\xDF\xBF", &status));
+  EXPECT_EQ(PR_SUCCESS, status);
+
+  // overlong U+0000..U+07FF
+  EXPECT_EQ(0u, nssUTF8_Length("\xE0\x80\x80", &status));
+  EXPECT_EQ(PR_FAILURE, status);
+  EXPECT_EQ(NSS_ERROR_INVALID_STRING, NSS_GetError());
+  EXPECT_EQ(0u, nssUTF8_Length("\xE0\x9F\xBF", &status));
+  EXPECT_EQ(PR_FAILURE, status);
+  EXPECT_EQ(NSS_ERROR_INVALID_STRING, NSS_GetError());
+
+  // U+0800..U+D7FF
+  EXPECT_EQ(3u, nssUTF8_Length("\xE0\xA0\x80", &status));
+  EXPECT_EQ(PR_SUCCESS, status);
+  EXPECT_EQ(3u, nssUTF8_Length("\xE0\xBF\xBF", &status));
+  EXPECT_EQ(PR_SUCCESS, status);
+  EXPECT_EQ(3u, nssUTF8_Length("\xE1\x80\x80", &status));
+  EXPECT_EQ(PR_SUCCESS, status);
+  EXPECT_EQ(3u, nssUTF8_Length("\xEC\xBF\xBF", &status));
+  EXPECT_EQ(PR_SUCCESS, status);
+  EXPECT_EQ(3u, nssUTF8_Length("\xED\x80\x80", &status));
+  EXPECT_EQ(PR_SUCCESS, status);
+  EXPECT_EQ(3u, nssUTF8_Length("\xED\x9F\xBF", &status));
+  EXPECT_EQ(PR_SUCCESS, status);
+
+  // lone surrogate
+  EXPECT_EQ(0u, nssUTF8_Length("\xED\xA0\x80", &status));
+  EXPECT_EQ(PR_FAILURE, status);
+  EXPECT_EQ(NSS_ERROR_INVALID_STRING, NSS_GetError());
+  EXPECT_EQ(0u, nssUTF8_Length("\xED\xBF\xBF", &status));
+  EXPECT_EQ(PR_FAILURE, status);
+  EXPECT_EQ(NSS_ERROR_INVALID_STRING, NSS_GetError());
+
+  // U+E000..U+FFFF
+  EXPECT_EQ(3u, nssUTF8_Length("\xEE\x80\x80", &status));
+  EXPECT_EQ(PR_SUCCESS, status);
+  EXPECT_EQ(3u, nssUTF8_Length("\xEF\xBF\xBF", &status));
+  EXPECT_EQ(PR_SUCCESS, status);
+
+  // overlong U+0000..U+FFFF
+  EXPECT_EQ(0u, nssUTF8_Length("\xF0\x80\x80\x80", &status));
+  EXPECT_EQ(PR_FAILURE, status);
+  EXPECT_EQ(NSS_ERROR_INVALID_STRING, NSS_GetError());
+  EXPECT_EQ(0u, nssUTF8_Length("\xF0\x8F\xBF\xBF", &status));
+  EXPECT_EQ(PR_FAILURE, status);
+  EXPECT_EQ(NSS_ERROR_INVALID_STRING, NSS_GetError());
+
+  // U+10000..U+10FFFF
+  EXPECT_EQ(4u, nssUTF8_Length("\xF0\x90\x80\x80", &status));
+  EXPECT_EQ(PR_SUCCESS, status);
+  EXPECT_EQ(4u, nssUTF8_Length("\xF0\xBF\xBF\xBF", &status));
+  EXPECT_EQ(PR_SUCCESS, status);
+  EXPECT_EQ(4u, nssUTF8_Length("\xF1\x80\x80\x80", &status));
+  EXPECT_EQ(PR_SUCCESS, status);
+  EXPECT_EQ(4u, nssUTF8_Length("\xF3\xBF\xBF\xBF", &status));
+  EXPECT_EQ(PR_SUCCESS, status);
+  EXPECT_EQ(4u, nssUTF8_Length("\xF4\x80\x80\x80", &status));
+  EXPECT_EQ(PR_SUCCESS, status);
+  EXPECT_EQ(4u, nssUTF8_Length("\xF4\x8F\xBF\xBF", &status));
+  EXPECT_EQ(PR_SUCCESS, status);
+
+  // out of Unicode range
+  EXPECT_EQ(0u, nssUTF8_Length("\xF4\x90\x80\x80", &status));
+  EXPECT_EQ(PR_FAILURE, status);
+  EXPECT_EQ(NSS_ERROR_INVALID_STRING, NSS_GetError());
+  EXPECT_EQ(0u, nssUTF8_Length("\xF4\xBF\xBF\xBF", &status));
+  EXPECT_EQ(PR_FAILURE, status);
+  EXPECT_EQ(NSS_ERROR_INVALID_STRING, NSS_GetError());
+  EXPECT_EQ(0u, nssUTF8_Length("\xF5\x80\x80\x80", &status));
+  EXPECT_EQ(PR_FAILURE, status);
+  EXPECT_EQ(NSS_ERROR_INVALID_STRING, NSS_GetError());
+  EXPECT_EQ(0u, nssUTF8_Length("\xF7\xBF\xBF\xBF", &status));
+  EXPECT_EQ(PR_FAILURE, status);
+  EXPECT_EQ(NSS_ERROR_INVALID_STRING, NSS_GetError());
+
+  // former 5-byte sequence
+  EXPECT_EQ(0u, nssUTF8_Length("\xF8\x80\x80\x80\x80", &status));
+  EXPECT_EQ(PR_FAILURE, status);
+  EXPECT_EQ(NSS_ERROR_INVALID_STRING, NSS_GetError());
+  EXPECT_EQ(0u, nssUTF8_Length("\xFB\xBF\xBF\xBF\xBF", &status));
+  EXPECT_EQ(PR_FAILURE, status);
+  EXPECT_EQ(NSS_ERROR_INVALID_STRING, NSS_GetError());
+
+  // former 6-byte sequence
+  EXPECT_EQ(0u, nssUTF8_Length("\xFC\x80\x80\x80\x80\x80", &status));
+  EXPECT_EQ(PR_FAILURE, status);
+  EXPECT_EQ(NSS_ERROR_INVALID_STRING, NSS_GetError());
+  EXPECT_EQ(0u, nssUTF8_Length("\xFD\xBF\xBF\xBF\xBF\xBF", &status));
+  EXPECT_EQ(PR_FAILURE, status);
+  EXPECT_EQ(NSS_ERROR_INVALID_STRING, NSS_GetError());
+
+  // invalid lead byte
+  EXPECT_EQ(0u, nssUTF8_Length("\xFE", &status));
+  EXPECT_EQ(PR_FAILURE, status);
+  EXPECT_EQ(NSS_ERROR_INVALID_STRING, NSS_GetError());
+  EXPECT_EQ(0u, nssUTF8_Length("\xFF", &status));
+  EXPECT_EQ(PR_FAILURE, status);
+  EXPECT_EQ(NSS_ERROR_INVALID_STRING, NSS_GetError());
+
+  nss_DestroyErrorStack();
+}
+}
diff --git a/gtests/manifest.mn b/gtests/manifest.mn
index e53663596..c7846a553 100644
--- a/gtests/manifest.mn
+++ b/gtests/manifest.mn
@@ -23,6 +23,7 @@ endif
 ifneq ($(NSS_BUILD_SOFTOKEN_ONLY),1)
 ifneq ($(NSS_BUILD_UTIL_ONLY),1)
 NSS_SRCDIRS = \
+	base_gtest \
 	certdb_gtest \
 	certhigh_gtest \
 	cryptohi_gtest \
@@ -37,6 +38,7 @@ NSS_SRCDIRS = \
 	pkcs11testmodule \
 	$(NULL)
 
+base_gtest: common
 certdb_gtest: common
 certhigh_gtest: common
 cryptohi_gtest: common
diff --git a/lib/base/utf8.c b/lib/base/utf8.c
index 6d7b6a015..9fe378010 100644
--- a/lib/base/utf8.c
+++ b/lib/base/utf8.c
@@ -307,31 +307,54 @@ nssUTF8_Length(const NSSUTF8 *s, PRStatus *statusOpt)
 #endif /* NSSDEBUG */
 
     /*
-     * From RFC 2044:
+     * From RFC 3629:
      *
-     * UCS-4 range (hex.)           UTF-8 octet sequence (binary)
-     * 0000 0000-0000 007F   0xxxxxxx
-     * 0000 0080-0000 07FF   110xxxxx 10xxxxxx
-     * 0000 0800-0000 FFFF   1110xxxx 10xxxxxx 10xxxxxx
-     * 0001 0000-001F FFFF   11110xxx 10xxxxxx 10xxxxxx 10xxxxxx
-     * 0020 0000-03FF FFFF   111110xx 10xxxxxx 10xxxxxx 10xxxxxx 10xxxxxx
-     * 0400 0000-7FFF FFFF   1111110x 10xxxxxx ... 10xxxxxx
+     * UTF8-octets = *( UTF8-char )
+     * UTF8-char   = UTF8-1 / UTF8-2 / UTF8-3 / UTF8-4
+     * UTF8-1      = %x00-7F
+     * UTF8-2      = %xC2-DF UTF8-tail
+     * UTF8-3      = %xE0 %xA0-BF UTF8-tail / %xE1-EC 2( UTF8-tail ) /
+     *               %xED %x80-9F UTF8-tail / %xEE-EF 2( UTF8-tail )
+     * UTF8-4      = %xF0 %x90-BF 2( UTF8-tail ) / %xF1-F3 3( UTF8-tail ) /
+     *               %xF4 %x80-8F 2( UTF8-tail )
+     * UTF8-tail   = %x80-BF
      */
 
     while (0 != *c) {
         PRUint32 incr;
-        if ((*c & 0x80) == 0) {
+        if (*c < 0x80) {
             incr = 1;
-        } else if ((*c & 0xE0) == 0xC0) {
+        } else if (*c < 0xC2) {
+            nss_SetError(NSS_ERROR_INVALID_STRING);
+            goto loser;
+        } else if (*c < 0xE0) {
             incr = 2;
-        } else if ((*c & 0xF0) == 0xE0) {
+        } else if (*c == 0xE0) {
+            if (c[1] < 0xA0) {
+                nss_SetError(NSS_ERROR_INVALID_STRING);
+                goto loser;
+            }
             incr = 3;
-        } else if ((*c & 0xF8) == 0xF0) {
+        } else if (*c < 0xF0) {
+            if (*c == 0xED && c[1] > 0x9F) {
+                nss_SetError(NSS_ERROR_INVALID_STRING);
+                goto loser;
+            }
+            incr = 3;
+        } else if (*c == 0xF0) {
+            if (c[1] < 0x90) {
+                nss_SetError(NSS_ERROR_INVALID_STRING);
+                goto loser;
+            }
+            incr = 4;
+        } else if (*c < 0xF4) {
+            incr = 4;
+        } else if (*c == 0xF4) {
+            if (c[1] > 0x8F) {
+                nss_SetError(NSS_ERROR_INVALID_STRING);
+                goto loser;
+            }
             incr = 4;
-        } else if ((*c & 0xFC) == 0xF8) {
-            incr = 5;
-        } else if ((*c & 0xFE) == 0xFC) {
-            incr = 6;
         } else {
             nss_SetError(NSS_ERROR_INVALID_STRING);
             goto loser;
@@ -345,17 +368,17 @@ nssUTF8_Length(const NSSUTF8 *s, PRStatus *statusOpt)
             nss_SetError(NSS_ERROR_VALUE_TOO_LARGE);
             goto loser;
         }
+#endif /* PEDANTIC */
 
         {
-            PRUint8 *d;
+            const PRUint8 *d;
             for (d = &c[1]; d < &c[incr]; d++) {
-                if ((*d & 0xC0) != 0xF0) {
+                if ((*d & 0xC0) != 0x80) {
                     nss_SetError(NSS_ERROR_INVALID_STRING);
                     goto loser;
                 }
             }
         }
-#endif /* PEDANTIC */
 
         c += incr;
     }
diff --git a/nss.gyp b/nss.gyp
index dfc723259..5bbbcf152 100644
--- a/nss.gyp
+++ b/nss.gyp
@@ -206,6 +206,7 @@
             'cmd/vfychain/vfychain.gyp:vfychain',
             'cmd/vfyserv/vfyserv.gyp:vfyserv',
             'cmd/mpitests/mpitests.gyp:mpi_tests',
+            'gtests/base_gtest/base_gtest.gyp:base_gtest',
             'gtests/certhigh_gtest/certhigh_gtest.gyp:certhigh_gtest',
             'gtests/cryptohi_gtest/cryptohi_gtest.gyp:cryptohi_gtest',
             'gtests/der_gtest/der_gtest.gyp:der_gtest',
diff --git a/tests/gtests/gtests.sh b/tests/gtests/gtests.sh
index e026b09a4..4e04b6100 100755
--- a/tests/gtests/gtests.sh
+++ b/tests/gtests/gtests.sh
@@ -101,7 +101,7 @@ gtest_cleanup()
 }
 
 ################## main #################################################
-GTESTS="${GTESTS:-certhigh_gtest certdb_gtest der_gtest pk11_gtest util_gtest freebl_gtest softoken_gtest sysinit_gtest smime_gtest mozpkix_gtest}"
+GTESTS="${GTESTS:-base_gtest certhigh_gtest certdb_gtest der_gtest pk11_gtest util_gtest freebl_gtest softoken_gtest sysinit_gtest smime_gtest mozpkix_gtest}"
 gtest_init "$0"
 gtest_start
 gtest_cleanup