Bug 1017140: Strip url params for application reputation lookups (r=paolo)

toolkit/components/downloads/ApplicationReputation.cpp

@@ -148,6 +148,10 @@ private:
                                  nsresult aResult,
                                  bool* aShouldBlock);
 
+  // Strip url parameters, fragments, and user@pass fields from the URI spec
+  // using nsIURL. If aURI is not an nsIURL, returns the original nsIURI.spec.
+  nsresult GetStrippedSpec(nsIURI* aUri, nsACString& spec);
+
   // Escape '/' and '%' in certificate attribute values.
   nsCString EscapeCertificateAttribute(const nsACString& aAttribute);
 
@@ -591,7 +595,7 @@ PendingLookup::AddRedirects(nsIArray* aRedirects)
     // Add the spec to our list of local lookups. The most recent redirect is
     // the last element.
     nsCString spec;
-    rv = uri->GetSpec(spec);
+    rv = GetStrippedSpec(uri, spec);
     NS_ENSURE_SUCCESS(rv, rv);
     mAnylistSpecs.AppendElement(spec);
     LOG(("ApplicationReputation: Appending redirect %s\n", spec.get()));
@@ -618,6 +622,34 @@ PendingLookup::StartLookup()
   return rv;
 }
 
+nsresult
+PendingLookup::GetStrippedSpec(nsIURI* aUri, nsACString& escaped)
+{
+  // If aURI is not an nsIURL, we do not want to check the lists or send a
+  // remote query.
+  nsresult rv;
+  nsCOMPtr<nsIURL> url = do_QueryInterface(aUri, &rv);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  rv = url->GetScheme(escaped);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  nsCString temp;
+  rv = url->GetHostPort(temp);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  escaped.Append("://");
+  escaped.Append(temp);
+
+  rv = url->GetFilePath(temp);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  // nsIUrl.filePath starts with '/'
+  escaped.Append(temp);
+
+  return NS_OK;
+}
+
 nsresult
 PendingLookup::DoLookupInternal()
 {
@@ -628,9 +660,11 @@ PendingLookup::DoLookupInternal()
   NS_ENSURE_SUCCESS(rv, rv);
 
   nsCString spec;
-  rv = uri->GetSpec(spec);
+  rv = GetStrippedSpec(uri, spec);
   NS_ENSURE_SUCCESS(rv, rv);
+
   mAnylistSpecs.AppendElement(spec);
+
   ClientDownloadRequest_Resource* resource = mRequest.add_resources();
   resource->set_url(spec.get());
   resource->set_type(ClientDownloadRequest::DOWNLOAD_URL);
@@ -639,7 +673,7 @@ PendingLookup::DoLookupInternal()
   rv = mQuery->GetReferrerURI(getter_AddRefs(referrer));
   if (referrer) {
     nsCString spec;
-    rv = referrer->GetSpec(spec);
+    rv = GetStrippedSpec(referrer, spec);
     NS_ENSURE_SUCCESS(rv, rv);
     mAnylistSpecs.AppendElement(spec);
     resource->set_referrer(spec.get());
@@ -774,7 +808,7 @@ PendingLookup::SendRemoteQueryInternal()
   rv = mQuery->GetSourceURI(getter_AddRefs(uri));
   NS_ENSURE_SUCCESS(rv, rv);
   nsCString spec;
-  rv = uri->GetSpec(spec);
+  rv = GetStrippedSpec(uri, spec);
   NS_ENSURE_SUCCESS(rv, rv);
   mRequest.set_url(spec.get());
 

toolkit/components/downloads/test/unit/test_app_rep.js

@@ -16,9 +16,9 @@ let ALLOW_LIST = 0;
 let BLOCK_LIST = 1;
 let NO_LIST = 2;
 
-let whitelistedURI = createURI("http://whitelisted.com");
-let exampleURI = createURI("http://example.com");
-let blocklistedURI = createURI("http://blocklisted.com");
+let whitelistedURI = createURI("http://foo:bar@whitelisted.com/index.htm#junk");
+let exampleURI = createURI("http://user:password@example.com/i.html?foo=bar");
+let blocklistedURI = createURI("http://baz:qux@blocklisted.com?xyzzy");
 
 function readFileToString(aFilename) {
   let f = do_get_file(aFilename);
@@ -248,6 +248,25 @@ add_test(function test_unlisted() {
   });
 });
 
+add_test(function test_non_uri() {
+  Services.prefs.setCharPref("browser.safebrowsing.appRepURL",
+                             "http://localhost:4444/download");
+  let counts = get_telemetry_counts();
+  let listCounts = counts.listCounts;
+  // No listcount is incremented, since the sourceURI is not an nsIURL
+  let source = NetUtil.newURI("data:application/octet-stream,ABC");
+  do_check_false(source instanceof Ci.nsIURL);
+  gAppRep.queryReputation({
+    sourceURI: source,
+    fileSize: 12,
+  }, function onComplete(aShouldBlock, aStatus) {
+    do_check_eq(Cr.NS_OK, aStatus);
+    do_check_false(aShouldBlock);
+    check_telemetry(counts.total + 1, counts.shouldBlock, listCounts);
+    run_next_test();
+  });
+});
+
 add_test(function test_local_blacklist() {
   Services.prefs.setCharPref("browser.safebrowsing.appRepURL",
                              "http://localhost:4444/download");