Bug 937265 - fake scam warning for auto-linked ip address. r=squib
authorMagnus Melin <mkmelin+mozilla@iki.fi>
Mon, 25 Nov 2013 21:18:42 +0200
changeset 16928 a430a521cacfdc5c00b603f09941b29bf9d397f0
parent 16927 8d4803345e7c50bebd7c15e5a8ecfaee6404a995
child 16929 8f768cf18b432fec7f1758f98b082d4d021a68e5
push id1074
push userbugzilla@standard8.plus.com
push dateMon, 03 Feb 2014 22:47:23 +0000
treeherdercomm-beta@6b791b5369ed [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssquib
bugs937265
Bug 937265 - fake scam warning for auto-linked ip address. r=squib
mail/base/content/phishingDetector.js
mail/test/mozmill/message-header/test-phishing-bar.js
--- a/mail/base/content/phishingDetector.js
+++ b/mail/base/content/phishingDetector.js
@@ -121,38 +121,39 @@ var gPhishingDetector = {
     try {
       hrefURL = Services.io.newURI(aUrl, null, null);
     } catch(ex) { return; }
 
     // only check for phishing urls if the url is an http or https link.
     // this prevents us from flagging imap and other internally handled urls
     if (hrefURL.schemeIs('http') || hrefURL.schemeIs('https'))
     {
-      var linkTextURL = {};
-
       // The link is not suspicious if the visible text is the same as the URL,
       // even if the URL is an IP address. URLs are commonly surrounded by
       // < > or "" (RFC2396E) - so strip those from the link text before comparing.
       if (aLinkText)
         aLinkText = aLinkText.replace(/^<(.+)>$|^"(.+)"$/, "$1$2");
 
       var failsStaticTests = false;
-      if (aLinkText != aUrl)
+      // If the link text and url differs by something other than a trailing
+      // slash, do some further checks.
+      if (aLinkText != aUrl &&
+          aLinkText.replace(/\/+$/, "") != aUrl.replace(/\/+$/, ""))
       {
         if (this.mCheckForIPAddresses)
         {
           let unobscuredHostNameValue = this.isLegalIPAddress(hrefURL.host, true);
           if (unobscuredHostNameValue)
             failsStaticTests = !this.isLegalLocalIPAddress(unobscuredHostNameValue);
         }
 
         if (!failsStaticTests && this.mCheckForMismatchedHosts)
         {
           failsStaticTests = (aLinkText &&
-            this.misMatchedHostWithLinkText(hrefURL, aLinkText, linkTextURL))
+            this.misMatchedHostWithLinkText(hrefURL, aLinkText))
         }
       }
 
       // Lookup the url against our local list. We want to do this even if the url fails our static
       // test checks because the url might be in the white list.
       if (this.mPhishingWarden)
         this.mPhishingWarden.isEvilURL(gFolderDisplay.selectedMessage,
                                        failsStaticTests, aUrl,
@@ -207,34 +208,33 @@ var gPhishingDetector = {
      }
    },   
 
   /**
    * Private helper method to determine if the link node contains a user visible
    * url with a host name that differs from the actual href the user would get taken to.
    * i.e. <a href="http://myevilsite.com">http://mozilla.org</a>
    * 
-   * @return true if aHrefURL.host matches the host of the link node text. 
-   * @return aLinkTextURL the nsIURI for the link node text
+   * @return true if aHrefURL.host does NOT match the host of the link node text
    */
-  misMatchedHostWithLinkText: function(aHrefURL, aLinkNodeText, aLinkTextURL)
+  misMatchedHostWithLinkText: function(aHrefURL, aLinkNodeText)
   {
     // gatherTextUnder puts a space between each piece of text it gathers,
     // so strip the spaces out (see bug 326082 for details).
     aLinkNodeText = aLinkNodeText.replace(/ /g, "");
 
     // only worry about http and https urls
     if (aLinkNodeText)
     {
       // does the link text look like a http url?
        if (aLinkNodeText.search(/(^http:|^https:)/) != -1)
        {
-         aLinkTextURL.value = Services.io.newURI(aLinkNodeText, null, null);
+         let linkURI = Services.io.newURI(aLinkNodeText, null, null);
          // compare hosts, but ignore possible www. prefix
-         return !(aHrefURL.host.replace(/^www\./, "") == aLinkTextURL.value.host.replace(/^www\./, ""));
+         return !(aHrefURL.host.replace(/^www\./, "") == linkURI.host.replace(/^www\./, ""));
        }
     }
 
     return false;
   },
 
   /**
    * If the current message has been identified as an email scam, prompts the user with a warning
--- a/mail/test/mozmill/message-header/test-phishing-bar.js
+++ b/mail/test/mozmill/message-header/test-phishing-bar.js
@@ -28,16 +28,20 @@ function setupModule(module) {
   }
 
   folder = create_folder("PhishingBarA");
   add_message_to_folder(folder, create_message({body: {
     body: '<a href="http://www.evil.com/google/">http://www.google.com</a>',
     contentType: "text/html"
   }}));
   add_message_to_folder(folder, create_message());
+  add_message_to_folder(folder, create_message({body: {
+    body: 'check out http://130.128.4.1. and http://130.128.4.2/.',
+    contentType: "text/plain"
+  }}));
 }
 
 /**
  * Make sure the notification shows, and goes away once the Ignore menuitem
  * is clicked.
  */
 function assert_ignore_works(aController) {
   wait_for_notification_to_show(aController, kBoxId, kNotificationValue);
@@ -83,23 +87,33 @@ function test_ignore_phishing_warning_fr
 function test_ignore_phishing_warning_from_eml_attachment() {
   let thisFilePath = os.getFileForPath(__file__);
   let file = os.getFileForPath(os.abspath("./evil-attached.eml", thisFilePath));
 
   let msgc = open_message_from_file(file);
 
   // Make sure the root message shows the phishing bar.
   wait_for_notification_to_show(msgc, kBoxId, kNotificationValue);
-  //function assert_notification_displayed(aController, aValue, aDisplayed) 
 
   // Open the attached message.
   plan_for_new_window("mail:messageWindow");
   msgc.e("attachmentList").getItemAtIndex(0).attachment.open();
   let msgc2 = wait_for_new_window("mail:messageWindow");
   wait_for_message_display_completion(msgc2, true);
 
   // Now make sure the attached message shows the phishing bar.
   wait_for_notification_to_show(msgc2, kBoxId, kNotificationValue);
 
   close_window(msgc2);
   close_window(msgc);
 }
 
+/**
+ * Test that when viewing a message with an auto-linked ip address, we don't
+ * get a warning. We'll have http://130.128.4.1 vs. http://130.128.4.1/
+ */
+function test_no_phishing_warning_for_ip_sameish_text() {
+  be_in_folder(folder);
+  select_click_row(2); // Mail with Public IP address.
+  assert_notification_displayed(mc, kBoxId, kNotificationValue, false); // not shown
+}
+
+