Bug 1138721 - Application reputation should check mac file extensions. r=mmc, a=lizzard
authorFrancois Marier <francois@mozilla.com>
Tue, 31 Mar 2015 21:24:00 -0400
changeset 266975 1889221f549157378503579d60b8fa7dd89373f7
parent 266974 091fa522326bd7bc6619902d419761b55278f8d3
child 266976 bbdecb2586cb973a053a5ac8dbf44e157a31d8fe
push id830
push userraliiev@mozilla.com
push dateFri, 19 Jun 2015 19:24:37 +0000
treeherdermozilla-release@932614382a68 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmmc, lizzard
bugs1138721
milestone39.0a2
Bug 1138721 - Application reputation should check mac file extensions. r=mmc, a=lizzard Specify the download type when doing a remote lookup.
toolkit/components/downloads/ApplicationReputation.cpp
toolkit/components/downloads/csd.pb.h
toolkit/components/downloads/test/unit/test_app_rep_maclinux.js
toolkit/components/downloads/test/unit/test_app_rep_windows.js
--- a/toolkit/components/downloads/ApplicationReputation.cpp
+++ b/toolkit/components/downloads/ApplicationReputation.cpp
@@ -136,19 +136,22 @@ private:
   // signed.
   ClientDownloadRequest mRequest;
 
   // The response from the application reputation query. This is read in chunks
   // as part of our nsIStreamListener implementation and may contain embedded
   // NULLs.
   nsCString mResponse;
 
-  // Returns true if the file is likely to be binary on Windows.
+  // Returns true if the file is likely to be binary.
   bool IsBinaryFile();
 
+  // Returns the type of download binary for the file.
+  ClientDownloadRequest::DownloadType GetDownloadType(const nsAString& aFilename);
+
   // Clean up and call the callback. PendingLookup must not be used after this
   // function is called.
   nsresult OnComplete(bool shouldBlock, nsresult rv);
 
   // Wrapper function for nsIStreamListener.onStopRequest to make it easy to
   // guarantee calling the callback
   nsresult OnStopRequestInternal(nsIRequest *aRequest,
                                  nsISupports *aContext,
@@ -370,35 +373,61 @@ PendingLookup::IsBinaryFile()
   nsresult rv = mQuery->GetSuggestedFileName(fileName);
   if (NS_FAILED(rv)) {
     LOG(("No suggested filename [this = %p]", this));
     return false;
   }
   LOG(("Suggested filename: %s [this = %p]",
        NS_ConvertUTF16toUTF8(fileName).get(), this));
   return
-    // Executable extensions for MS Windows, from
-    // https://code.google.com/p/chromium/codesearch#chromium/src/chrome/common/safe_browsing/download_protection_util.cc&l=14
+    // From https://code.google.com/p/chromium/codesearch#chromium/src/chrome/common/safe_browsing/download_protection_util.cc&l=14
+    // Android extensions
     StringEndsWith(fileName, NS_LITERAL_STRING(".apk")) ||
+    // Windows extensions
     StringEndsWith(fileName, NS_LITERAL_STRING(".bas")) ||
     StringEndsWith(fileName, NS_LITERAL_STRING(".bat")) ||
     StringEndsWith(fileName, NS_LITERAL_STRING(".cab")) ||
     StringEndsWith(fileName, NS_LITERAL_STRING(".cmd")) ||
     StringEndsWith(fileName, NS_LITERAL_STRING(".com")) ||
     StringEndsWith(fileName, NS_LITERAL_STRING(".exe")) ||
     StringEndsWith(fileName, NS_LITERAL_STRING(".hta")) ||
     StringEndsWith(fileName, NS_LITERAL_STRING(".msi")) ||
     StringEndsWith(fileName, NS_LITERAL_STRING(".pif")) ||
     StringEndsWith(fileName, NS_LITERAL_STRING(".reg")) ||
     StringEndsWith(fileName, NS_LITERAL_STRING(".scr")) ||
     StringEndsWith(fileName, NS_LITERAL_STRING(".vb")) ||
     StringEndsWith(fileName, NS_LITERAL_STRING(".vbs")) ||
+    // Mac extensions
+    StringEndsWith(fileName, NS_LITERAL_STRING(".app")) ||
+    StringEndsWith(fileName, NS_LITERAL_STRING(".dmg")) ||
+    StringEndsWith(fileName, NS_LITERAL_STRING(".osx")) ||
+    StringEndsWith(fileName, NS_LITERAL_STRING(".pkg")) ||
+    // Archives _may_ contain binaries
     StringEndsWith(fileName, NS_LITERAL_STRING(".zip"));
 }
 
+ClientDownloadRequest::DownloadType
+PendingLookup::GetDownloadType(const nsAString& aFilename) {
+  MOZ_ASSERT(IsBinaryFile());
+
+  // From https://code.google.com/p/chromium/codesearch#chromium/src/chrome/common/safe_browsing/download_protection_util.cc&l=46
+  if (StringEndsWith(aFilename, NS_LITERAL_STRING(".zip"))) {
+    return ClientDownloadRequest::ZIPPED_EXECUTABLE;
+  } else if (StringEndsWith(aFilename, NS_LITERAL_STRING(".apk"))) {
+    return ClientDownloadRequest::ANDROID_APK;
+  } else if (StringEndsWith(aFilename, NS_LITERAL_STRING(".app")) ||
+             StringEndsWith(aFilename, NS_LITERAL_STRING(".dmg")) ||
+             StringEndsWith(aFilename, NS_LITERAL_STRING(".osx")) ||
+             StringEndsWith(aFilename, NS_LITERAL_STRING(".pkg"))) {
+    return ClientDownloadRequest::MAC_EXECUTABLE;
+  }
+
+  return ClientDownloadRequest::WIN_EXECUTABLE; // default to Windows binaries
+}
+
 nsresult
 PendingLookup::LookupNext()
 {
   // We must call LookupNext or SendRemoteQuery upon return.
   // Look up all of the URLs that could allow or block this download.
   // Blocklist first.
   if (mBlocklistCount > 0) {
     return OnComplete(true, NS_OK);
@@ -871,34 +900,35 @@ PendingLookup::SendRemoteQueryInternal()
   nsCString sha256Hash;
   rv = mQuery->GetSha256Hash(sha256Hash);
   NS_ENSURE_SUCCESS(rv, rv);
   mRequest.mutable_digests()->set_sha256(sha256Hash.Data());
   nsString fileName;
   rv = mQuery->GetSuggestedFileName(fileName);
   NS_ENSURE_SUCCESS(rv, rv);
   mRequest.set_file_basename(NS_ConvertUTF16toUTF8(fileName).get());
+  mRequest.set_download_type(GetDownloadType(fileName));
 
   if (mRequest.signature().trusted()) {
     LOG(("Got signed binary for remote application reputation check "
          "[this = %p]", this));
   } else {
     LOG(("Got unsigned binary for remote application reputation check "
          "[this = %p]", this));
   }
 
   // Serialize the protocol buffer to a string. This can only fail if we are
   // out of memory, or if the protocol buffer req is missing required fields
   // (only the URL for now).
   std::string serialized;
   if (!mRequest.SerializeToString(&serialized)) {
     return NS_ERROR_UNEXPECTED;
   }
-  LOG(("Serialized protocol buffer [this = %p]: %s", this,
-       serialized.c_str()));
+  LOG(("Serialized protocol buffer [this = %p]: (length=%d) %s", this,
+       serialized.length(), serialized.c_str()));
 
   // Set the input stream to the serialized protocol buffer
   nsCOMPtr<nsIStringInputStream> sstream =
     do_CreateInstance("@mozilla.org/io/string-input-stream;1", &rv);
   NS_ENSURE_SUCCESS(rv, rv);
 
   rv = sstream->SetData(serialized.c_str(), serialized.length());
   NS_ENSURE_SUCCESS(rv, rv);
--- a/toolkit/components/downloads/csd.pb.h
+++ b/toolkit/components/downloads/csd.pb.h
@@ -53,21 +53,22 @@ bool ClientDownloadRequest_ResourceType_
 const ClientDownloadRequest_ResourceType ClientDownloadRequest_ResourceType_ResourceType_MIN = ClientDownloadRequest_ResourceType_DOWNLOAD_URL;
 const ClientDownloadRequest_ResourceType ClientDownloadRequest_ResourceType_ResourceType_MAX = ClientDownloadRequest_ResourceType_TAB_REDIRECT;
 const int ClientDownloadRequest_ResourceType_ResourceType_ARRAYSIZE = ClientDownloadRequest_ResourceType_ResourceType_MAX + 1;
 
 enum ClientDownloadRequest_DownloadType {
   ClientDownloadRequest_DownloadType_WIN_EXECUTABLE = 0,
   ClientDownloadRequest_DownloadType_CHROME_EXTENSION = 1,
   ClientDownloadRequest_DownloadType_ANDROID_APK = 2,
-  ClientDownloadRequest_DownloadType_ZIPPED_EXECUTABLE = 3
+  ClientDownloadRequest_DownloadType_ZIPPED_EXECUTABLE = 3,
+  ClientDownloadRequest_DownloadType_MAC_EXECUTABLE = 4
 };
 bool ClientDownloadRequest_DownloadType_IsValid(int value);
 const ClientDownloadRequest_DownloadType ClientDownloadRequest_DownloadType_DownloadType_MIN = ClientDownloadRequest_DownloadType_WIN_EXECUTABLE;
-const ClientDownloadRequest_DownloadType ClientDownloadRequest_DownloadType_DownloadType_MAX = ClientDownloadRequest_DownloadType_ZIPPED_EXECUTABLE;
+const ClientDownloadRequest_DownloadType ClientDownloadRequest_DownloadType_DownloadType_MAX = ClientDownloadRequest_DownloadType_MAC_EXECUTABLE;
 const int ClientDownloadRequest_DownloadType_DownloadType_ARRAYSIZE = ClientDownloadRequest_DownloadType_DownloadType_MAX + 1;
 
 enum ClientDownloadResponse_Verdict {
   ClientDownloadResponse_Verdict_SAFE = 0,
   ClientDownloadResponse_Verdict_DANGEROUS = 1,
   ClientDownloadResponse_Verdict_UNCOMMON = 2,
   ClientDownloadResponse_Verdict_POTENTIALLY_UNWANTED = 3,
   ClientDownloadResponse_Verdict_DANGEROUS_HOST = 4
@@ -901,16 +902,17 @@ class ClientDownloadRequest : public ::g
   static const int ResourceType_ARRAYSIZE =
     ClientDownloadRequest_ResourceType_ResourceType_ARRAYSIZE;
   
   typedef ClientDownloadRequest_DownloadType DownloadType;
   static const DownloadType WIN_EXECUTABLE = ClientDownloadRequest_DownloadType_WIN_EXECUTABLE;
   static const DownloadType CHROME_EXTENSION = ClientDownloadRequest_DownloadType_CHROME_EXTENSION;
   static const DownloadType ANDROID_APK = ClientDownloadRequest_DownloadType_ANDROID_APK;
   static const DownloadType ZIPPED_EXECUTABLE = ClientDownloadRequest_DownloadType_ZIPPED_EXECUTABLE;
+  static const DownloadType MAC_EXECUTABLE = ClientDownloadRequest_DownloadType_MAC_EXECUTABLE;
   static inline bool DownloadType_IsValid(int value) {
     return ClientDownloadRequest_DownloadType_IsValid(value);
   }
   static const DownloadType DownloadType_MIN =
     ClientDownloadRequest_DownloadType_DownloadType_MIN;
   static const DownloadType DownloadType_MAX =
     ClientDownloadRequest_DownloadType_DownloadType_MAX;
   static const int DownloadType_ARRAYSIZE =
--- a/toolkit/components/downloads/test/unit/test_app_rep_maclinux.js
+++ b/toolkit/components/downloads/test/unit/test_app_rep_maclinux.js
@@ -123,23 +123,23 @@ add_task(function test_setup()
     do_print("Querying remote server for verdict");
     response.setHeader("Content-Type", "application/octet-stream", false);
     let buf = NetUtil.readInputStreamToString(
       request.bodyInputStream,
       request.bodyInputStream.available());
     do_print("Request length: " + buf.length);
     // A garbage response. By default this produces NS_CANNOT_CONVERT_DATA as
     // the callback status.
-    let blob = "this is not a serialized protocol buffer";
+    let blob = "this is not a serialized protocol buffer (the length doesn't match our hard-coded values)";
     // We can't actually parse the protocol buffer here, so just switch on the
     // length instead of inspecting the contents.
-    if (buf.length == 65) {
+    if (buf.length == 67) {
       // evil.com
       blob = createVerdict(true);
-    } else if (buf.length == 71) {
+    } else if (buf.length == 73) {
       // mozilla.com
       blob = createVerdict(false);
     }
     response.bodyOutputStream.write(blob, blob.length);
   });
 
   gHttpServer.start(4444);
 });
--- a/toolkit/components/downloads/test/unit/test_app_rep_windows.js
+++ b/toolkit/components/downloads/test/unit/test_app_rep_windows.js
@@ -223,23 +223,23 @@ add_task(function test_setup()
     do_print("Querying remote server for verdict");
     response.setHeader("Content-Type", "application/octet-stream", false);
     let buf = NetUtil.readInputStreamToString(
       request.bodyInputStream,
       request.bodyInputStream.available());
     do_print("Request length: " + buf.length);
     // A garbage response. By default this produces NS_CANNOT_CONVERT_DATA as
     // the callback status.
-    let blob = "this is not a serialized protocol buffer";
+    let blob = "this is not a serialized protocol buffer (the length doesn't match our hard-coded values)";
     // We can't actually parse the protocol buffer here, so just switch on the
     // length instead of inspecting the contents.
-    if (buf.length == 65) {
+    if (buf.length == 67) {
       // evil.com
       blob = createVerdict(true);
-    } else if (buf.length == 71) {
+    } else if (buf.length == 73) {
       // mozilla.com
       blob = createVerdict(false);
     }
     response.bodyOutputStream.write(blob, blob.length);
   });
 
   gHttpServer.start(4444);
 });