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 249477 ba6239fc70f5b316ee8e50a40e369d9676a34dcc
parent 249476 1a8e41dad9fe7cb613e4653d5b9c8e29e7e15521
child 249478 2f54a6683cf5e240e889db3188be562132cb90a8
push id4489
push userraliiev@mozilla.com
push dateMon, 23 Feb 2015 15:17:55 +0000
treeherdermozilla-beta@fd7c3dc24146 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmmc, sledru
bugs1125503
milestone37.0a2
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
@@ -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);
+  }
 }