Bug 1125503 - When canonicalizing hostnames, check string length before calling Last(). r=mmc, a=sledru
authorDavid Keeler <dkeeler@mozilla.com>
Mon, 26 Jan 2015 12:47:50 -0800
changeset 243098 e25b169e456b
parent 243097 eea6117858b5
child 243587 7436b2d2e790
push id4402
push userryanvm@gmail.com
push date2015-01-29 16:16 +0000
treeherdermozilla-beta@e25b169e456b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmmc, sledru
bugs1125503
milestone36.0
Bug 1125503 - When canonicalizing hostnames, check string length before calling Last(). r=mmc, a=sledru
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
@@ -275,22 +275,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);
+  }
 }