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 437855 32672615c4732a6b9c6f14787fdad0600849031f
parent 437854 380cac58bbc59d23fc68442a0c36e4fbd4d0563f
child 437856 def8d4c0a39f707cab7d90d60a2281aca3e28b50
push id69804
push userrhelmer@mozilla.com
push dateMon, 24 Sep 2018 07:24:45 +0000
treeherderautoland@32672615c473 [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;