bug 1125503 - when canonicalizing hostnames, check string length before calling Last() r=mmc
authorDavid Keeler <dkeeler@mozilla.com>
Mon, 26 Jan 2015 12:47:50 -0800
changeset 239477 35f422e0053ccd5bb3d6096675222f0a97985be8
parent 239433 d0116a0ee30dff3a4cdeecbb2681a6d195692c2c
child 239478 8361462f2a02c9535b315a7b6e55df9513f5540b
push id500
push userjoshua.m.grant@gmail.com
push dateThu, 29 Jan 2015 01:48:36 +0000
reviewersmmc
bugs1125503
milestone38.0a1
bug 1125503 - when canonicalizing hostnames, check string length before calling Last() r=mmc
security/manager/boot/src/PublicKeyPinningService.cpp
security/manager/boot/src/nsSiteSecurityService.cpp
security/manager/ssl/tests/unit/test_sts_fqdn.js
--- a/security/manager/boot/src/PublicKeyPinningService.cpp
+++ b/security/manager/boot/src/PublicKeyPinningService.cpp
@@ -370,13 +370,14 @@ PublicKeyPinningService::ChainHasValidPi
                               enforceTestMode, time);
 }
 
 nsAutoCString
 PublicKeyPinningService::CanonicalizeHostname(const char* hostname)
 {
   nsAutoCString canonicalizedHostname(hostname);
   ToLowerCase(canonicalizedHostname);
-  while (canonicalizedHostname.Last() == '.') {
+  while (canonicalizedHostname.Length() > 0 &&
+         canonicalizedHostname.Last() == '.') {
     canonicalizedHostname.Truncate(canonicalizedHostname.Length() - 1);
   }
   return canonicalizedHostname;
 }
--- a/security/manager/boot/src/nsSiteSecurityService.cpp
+++ b/security/manager/boot/src/nsSiteSecurityService.cpp
@@ -274,22 +274,24 @@ nsSiteSecurityService::GetHost(nsIURI* a
 {
   nsCOMPtr<nsIURI> innerURI = NS_GetInnermostURI(aURI);
   if (!innerURI) {
     return NS_ERROR_FAILURE;
   }
 
   nsAutoCString host;
   nsresult rv = innerURI->GetAsciiHost(host);
-
-  if (NS_FAILED(rv) || host.IsEmpty()) {
-    return NS_ERROR_UNEXPECTED;
+  if (NS_FAILED(rv)) {
+    return rv;
   }
 
   aResult.Assign(PublicKeyPinningService::CanonicalizeHostname(host.get()));
+  if (aResult.IsEmpty()) {
+    return NS_ERROR_UNEXPECTED;
+  }
 
   return NS_OK;
 }
 
 static void
 SetStorageKey(nsAutoCString& storageKey, nsCString& hostname, uint32_t aType)
 {
   storageKey = hostname;
--- a/security/manager/ssl/tests/unit/test_sts_fqdn.js
+++ b/security/manager/ssl/tests/unit/test_sts_fqdn.js
@@ -37,9 +37,20 @@ function run_test() {
 
   SSService.removeState(Ci.nsISiteSecurityService.HEADER_HSTS, uri, 0);
   do_check_false(SSService.isSecureHost(Ci.nsISiteSecurityService.HEADER_HSTS,
                                         "example.com", 0));
   do_check_false(SSService.isSecureHost(Ci.nsISiteSecurityService.HEADER_HSTS,
                                         "example.com.", 0));
   do_check_false(SSService.isSecureHost(Ci.nsISiteSecurityService.HEADER_HSTS,
                                         "example.com..", 0));
+
+  // Somehow creating this malformed URI succeeds - we need to handle it
+  // gracefully.
+  uri = Services.io.newURI("https://../foo", null, null);
+  do_check_eq(uri.host, "..");
+  try {
+    SSService.isSecureURI(Ci.nsISiteSecurityService.HEADER_HSTS, uri, 0);
+    do_check_false(true); // this shouldn't run
+  } catch (e) {
+    do_check_eq(e.result, Cr.NS_ERROR_UNEXPECTED);
+  }
 }