Bug 1557931 - Stop using ACString parameters in nsICertOverrideService. r=keeler

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

--HG--
extra : moz-landing-system : lando

security/manager/ssl/nsCertOverrideService.cpp

@@ -336,7 +336,9 @@ nsCertOverrideService::RememberValidityOverride(const nsACString& aHostName,
                                                 uint32_t aOverrideBits,
                                                 bool aTemporary) {
   NS_ENSURE_ARG_POINTER(aCert);
-  if (aHostName.IsEmpty()) return NS_ERROR_INVALID_ARG;
+  if (aHostName.IsEmpty() || !IsASCII(aHostName)) {
+    return NS_ERROR_INVALID_ARG;
+  }
   if (aPort < -1) return NS_ERROR_INVALID_ARG;
 
   UniqueCERTCertificate nsscert(aCert->GetCert());
@@ -389,7 +391,8 @@ NS_IMETHODIMP
 nsCertOverrideService::RememberTemporaryValidityOverrideUsingFingerprint(
     const nsACString& aHostName, int32_t aPort,
     const nsACString& aCertFingerprint, uint32_t aOverrideBits) {
-  if (aCertFingerprint.IsEmpty() || aHostName.IsEmpty() || (aPort < -1)) {
+  if (aCertFingerprint.IsEmpty() || aHostName.IsEmpty() ||
+      !IsASCII(aCertFingerprint) || !IsASCII(aHostName) || (aPort < -1)) {
     return NS_ERROR_INVALID_ARG;
   }
 
@@ -409,7 +412,9 @@ nsCertOverrideService::HasMatchingOverride(const nsACString& aHostName,
                                            int32_t aPort, nsIX509Cert* aCert,
                                            uint32_t* aOverrideBits,
                                            bool* aIsTemporary, bool* _retval) {
-  if (aHostName.IsEmpty()) return NS_ERROR_INVALID_ARG;
+  if (aHostName.IsEmpty() || !IsASCII(aHostName)) {
+    return NS_ERROR_INVALID_ARG;
+  }
   if (aPort < -1) return NS_ERROR_INVALID_ARG;
 
   NS_ENSURE_ARG_POINTER(aCert);
@@ -481,6 +486,9 @@ nsresult nsCertOverrideService::AddEntryToList(
 NS_IMETHODIMP
 nsCertOverrideService::ClearValidityOverride(const nsACString& aHostName,
                                              int32_t aPort) {
+  if (aHostName.IsEmpty() || !IsASCII(aHostName)) {
+    return NS_ERROR_INVALID_ARG;
+  }
   if (!NS_IsMainThread()) {
     return NS_ERROR_NOT_SAME_THREAD;
   }

security/manager/ssl/nsICertOverrideService.idl

@@ -54,7 +54,7 @@ interface nsICertOverrideService : nsISupports {
    *  @param aOverrideBits The precise set of errors we want to be overriden
    */
   [must_use]
-  void rememberValidityOverride(in ACString aHostName,
+  void rememberValidityOverride(in AUTF8String aHostName,
                                 in int32_t aPort,
                                 in nsIX509Cert aCert,
                                 in uint32_t aOverrideBits,
@@ -75,9 +75,9 @@ interface nsICertOverrideService : nsISupports {
    */
   [must_use]
   void rememberTemporaryValidityOverrideUsingFingerprint(
-      in ACString aHostName,
+      in AUTF8String aHostName,
       in int32_t aPort,
-      in ACString aCertFingerprint,
+      in AUTF8String aCertFingerprint,
       in uint32_t aOverrideBits);
 
   /**
@@ -96,7 +96,7 @@ interface nsICertOverrideService : nsISupports {
    *  @return Whether an override has been stored for this host+port+cert
    */
   [must_use]
-  boolean hasMatchingOverride(in ACString aHostName,
+  boolean hasMatchingOverride(in AUTF8String aHostName,
                               in int32_t aPort,
                               in nsIX509Cert aCert,
                               out uint32_t aOverrideBits,
@@ -111,7 +111,7 @@ interface nsICertOverrideService : nsISupports {
    *               If it is 0 and aHostName is "all:temporary-certificates",
    *               then all temporary certificates should be cleared.
    */
-  void clearValidityOverride(in ACString aHostName,
+  void clearValidityOverride(in AUTF8String aHostName,
                              in int32_t aPort);
 
   /**

security/manager/ssl/tests/unit/test_cert_overrides.js

@@ -312,8 +312,14 @@ function add_simple_tests() {
     let cert = constructCertFromFile("bad_certs/idn-certificate.pem");
     Assert.ok(certOverrideService.hasMatchingOverride(uri.asciiHost, 8443, cert, {}, {}),
               "IDN certificate should have matching override using ascii host");
-    Assert.ok(!certOverrideService.hasMatchingOverride(uri.displayHost, 8443, cert, {}, {}),
-              "IDN certificate should not have matching override using (non-ascii) host");
+    Assert.throws(() => !certOverrideService.hasMatchingOverride(uri.displayHost, 8443, cert, {}, {}),
+                  /NS_ERROR_ILLEGAL_VALUE/,
+                  "IDN certificate should not have matching override using (non-ascii) host");
+    let invalidHost =
+      uri.asciiHost.replace(/./g, c => String.fromCharCode(c.charCodeAt(0) | 0x100));
+    Assert.throws(() => !certOverrideService.hasMatchingOverride(invalidHost, 8443, cert, {}, {}),
+                  /NS_ERROR_ILLEGAL_VALUE/,
+                  "hasMatchingOverride should not truncate high-bytes");
     run_next_test();
   });