Bug 1533216 - check we actually got some certs in collect_certs r=jcj
authorDana Keeler <dkeeler@mozilla.com>
Wed, 20 Mar 2019 13:03:49 -0700
changeset 15064 1473dd7efe2ce4f8722a33ebb03a3425e09887de
parent 15062 e8136553ef86fb9dd04119ee66014f624fbcf18d
child 15065 fbf1fe996ba1332509286761edd2af0a1a1c6a84
push id3309
push userjjones@mozilla.com
push dateWed, 20 Mar 2019 23:40:20 +0000
reviewersjcj
bugs1533216
Bug 1533216 - check we actually got some certs in collect_certs r=jcj Test Plan: https://treeherder.mozilla.org/#/jobs?repo=nss-try&revision=1b44b7588cb9a6806701394c08bf64d13f84b982 Reviewers: jcj Reviewed By: jcj Bug #: 1533216 Differential Revision: https://phabricator.services.mozilla.com/D23752
gtests/certdb_gtest/certdb_gtest.gyp
gtests/certdb_gtest/decode_certs_unittest.cc
gtests/certdb_gtest/manifest.mn
lib/pkcs7/certread.c
--- a/gtests/certdb_gtest/certdb_gtest.gyp
+++ b/gtests/certdb_gtest/certdb_gtest.gyp
@@ -7,23 +7,25 @@
     '../common/gtest.gypi',
   ],
   'targets': [
     {
       'target_name': 'certdb_gtest',
       'type': 'executable',
       'sources': [
         'alg1485_unittest.cc',
+        'decode_certs_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',
       ]
     }
   ],
   'variables': {
     'module': 'nss'
   }
 }
new file mode 100644
--- /dev/null
+++ b/gtests/certdb_gtest/decode_certs_unittest.cc
@@ -0,0 +1,28 @@
+/* -*- 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 "cert.h"
+#include "prerror.h"
+#include "secerr.h"
+
+class DecodeCertsTest : public ::testing::Test {};
+
+TEST_F(DecodeCertsTest, EmptyCertPackage) {
+  // This represents a PKCS#7 ContentInfo with a contentType of
+  // '2.16.840.1.113730.2.5' (Netscape data-type cert-sequence) and a content
+  // consisting of an empty SEQUENCE. This is valid ASN.1, but it contains no
+  // certificates, so CERT_DecodeCertFromPackage should just return a null
+  // pointer.
+  unsigned char emptyCertPackage[] = {0x30, 0x0f, 0x06, 0x09, 0x60, 0x86,
+                                      0x48, 0x01, 0x86, 0xf8, 0x42, 0x02,
+                                      0x05, 0xa0, 0x02, 0x30, 0x00};
+  EXPECT_EQ(nullptr, CERT_DecodeCertFromPackage(
+                         reinterpret_cast<char*>(emptyCertPackage),
+                         sizeof(emptyCertPackage)));
+  EXPECT_EQ(SEC_ERROR_BAD_DER, PR_GetError());
+}
--- a/gtests/certdb_gtest/manifest.mn
+++ b/gtests/certdb_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 = \
       alg1485_unittest.cc \
+      decode_certs_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
 
--- a/lib/pkcs7/certread.c
+++ b/lib/pkcs7/certread.c
@@ -487,24 +487,26 @@ loser:
 typedef struct {
     PLArenaPool *arena;
     SECItem cert;
 } collect_args;
 
 static SECStatus
 collect_certs(void *arg, SECItem **certs, int numcerts)
 {
-    SECStatus rv;
-    collect_args *collectArgs;
-
-    collectArgs = (collect_args *)arg;
-
-    rv = SECITEM_CopyItem(collectArgs->arena, &collectArgs->cert, *certs);
-
-    return (rv);
+    collect_args *collectArgs = (collect_args *)arg;
+    if (!collectArgs || !collectArgs->arena) {
+        PORT_SetError(SEC_ERROR_INVALID_ARGS);
+        return SECFailure;
+    }
+    if (numcerts < 1 || !certs || !*certs) {
+        PORT_SetError(SEC_ERROR_BAD_DER);
+        return SECFailure;
+    }
+    return SECITEM_CopyItem(collectArgs->arena, &collectArgs->cert, *certs);
 }
 
 /*
  * read an old style ascii or binary certificate
  */
 CERTCertificate *
 CERT_DecodeCertFromPackage(char *certbuf, int certlen)
 {