Bug 1491737 - ensure that PrioEncoder handles bad public keys correctly r=hsivonen
authorRobert Helmer <rhelmer@mozilla.com>
Mon, 24 Sep 2018 06:37:40 +0000
changeset 493574 32672615c4732a6b9c6f14787fdad0600849031f
parent 493573 380cac58bbc59d23fc68442a0c36e4fbd4d0563f
child 493575 def8d4c0a39f707cab7d90d60a2281aca3e28b50
push id9984
push userffxbld-merge
push dateMon, 15 Oct 2018 21:07:35 +0000
treeherdermozilla-beta@183d27ea8570 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewershsivonen
bugs1491737
milestone64.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1491737 - ensure that PrioEncoder handles bad public keys correctly r=hsivonen Differential Revision: https://phabricator.services.mozilla.com/D6096
dom/prio/PrioEncoder.cpp
dom/prio/test/gtest/TestPrioEncoder.cpp
--- a/dom/prio/PrioEncoder.cpp
+++ b/dom/prio/PrioEncoder.cpp
@@ -49,22 +49,16 @@ PrioEncoder::Encode(GlobalObject& aGloba
   if (!global) {
     aRv.Throw(NS_ERROR_UNEXPECTED);
     return;
   }
 
   SECStatus prio_rv = SECSuccess;
 
   if (!sSingleton) {
-    sSingleton = new PrioEncoder();
-
-    ClearOnShutdown(&sSingleton);
-
-    Prio_init();
-
     nsresult rv;
 
     nsAutoCStringN<CURVE25519_KEY_LEN_HEX + 1> prioKeyA;
     rv = Preferences::GetCString("prio.publicKeyA", prioKeyA);
     if (NS_FAILED(rv)) {
       aRv.Throw(NS_ERROR_UNEXPECTED);
       return;
     }
@@ -79,31 +73,41 @@ PrioEncoder::Encode(GlobalObject& aGloba
     // Check that both public keys are of the right length
     // and contain only hex digits 0-9a-fA-f
     if (!PrioEncoder::IsValidHexPublicKey(prioKeyA)
         || !PrioEncoder::IsValidHexPublicKey(prioKeyB))  {
       aRv.Throw(NS_ERROR_UNEXPECTED);
       return;
     }
 
+    prio_rv = Prio_init();
+
+    if (prio_rv != SECSuccess) {
+      aRv.Throw(NS_ERROR_UNEXPECTED);
+      return;
+    }
+
     prio_rv = PublicKey_import_hex(&sPublicKeyA,
                                    reinterpret_cast<const unsigned char*>(prioKeyA.BeginReading()),
                                    CURVE25519_KEY_LEN_HEX);
     if (prio_rv != SECSuccess) {
       aRv.Throw(NS_ERROR_UNEXPECTED);
       return;
     }
 
     prio_rv = PublicKey_import_hex(&sPublicKeyB,
               reinterpret_cast<const unsigned char*>(prioKeyB.BeginReading()),
               CURVE25519_KEY_LEN_HEX);
     if (prio_rv != SECSuccess) {
       aRv.Throw(NS_ERROR_UNEXPECTED);
       return;
     }
+
+    sSingleton = new PrioEncoder();
+    ClearOnShutdown(&sSingleton);
   }
 
   bool dataItems[] = {
     aPrioParams.mBrowserIsUserDefault,
     aPrioParams.mNewTabPageEnabled,
     aPrioParams.mPdfViewerUsed,
   };
 
--- a/dom/prio/test/gtest/TestPrioEncoder.cpp
+++ b/dom/prio/test/gtest/TestPrioEncoder.cpp
@@ -7,16 +7,50 @@
 #include "gtest/gtest.h"
 
 #include "jsapi.h"
 #include "PrioEncoder.h"
 
 #include "mozilla/Preferences.h"
 #include "mozilla/dom/ScriptSettings.h"
 
+TEST(PrioEncoder, BadPublicKeys)
+{
+  mozilla::dom::AutoJSAPI jsAPI;
+  ASSERT_TRUE(jsAPI.Init(xpc::PrivilegedJunkScope()));
+  JSContext* cx = jsAPI.cx();
+
+  mozilla::Preferences::SetCString("prio.publicKeyA",
+    nsCString(NS_LITERAL_CSTRING("badA")));
+  mozilla::Preferences::SetCString("prio.publicKeyB",
+    nsCString(NS_LITERAL_CSTRING("badB")));
+
+  mozilla::dom::GlobalObject global(cx, xpc::PrivilegedJunkScope());
+
+  nsCString batchID = NS_LITERAL_CSTRING("abc123");
+
+  mozilla::dom::PrioParams prioParams;
+  prioParams.mBrowserIsUserDefault = true;
+  prioParams.mNewTabPageEnabled = true;
+  prioParams.mPdfViewerUsed = false;
+
+  mozilla::dom::RootedDictionary<mozilla::dom::PrioEncodedData> prioEncodedData(cx);
+  mozilla::ErrorResult rv;
+
+  mozilla::dom::PrioEncoder::Encode(global, batchID, prioParams, prioEncodedData, rv);
+  ASSERT_TRUE(rv.Failed());
+
+  // Call again to ensure that the singleton state is consistent.
+  mozilla::dom::PrioEncoder::Encode(global, batchID, prioParams, prioEncodedData, rv);
+  ASSERT_TRUE(rv.Failed());
+
+  // Reset error result so test runner does not fail.
+  rv = mozilla::ErrorResult();
+}
+
 TEST(PrioEncoder, VerifyFull)
 {
   SECStatus prioRv = SECSuccess;
 
   PublicKey pkA = nullptr;
   PublicKey pkB = nullptr;
   PrivateKey skA = nullptr;
   PrivateKey skB = nullptr;