1200263 - 'Help → Report Web Forgery ...' does nothing

Status: RESOLVED FIXED

(SeaMonkey :: Security, defect)

Description

[Arthur K. (he/him)]

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:41.0) Gecko/20100101 Firefox/41.0 SeaMonkey/2.38
Build ID: 20150831085605

Steps to reproduce:

Clicked Help > Report Web Forgery.


Actual results:

It did not take me to the report phishing page site.


Expected results:

It should take me to the report phishing page site. Works in 2.37. Tested with https://l10n.mozilla-community.org/~akalla/unofficial/seamonkey/nightly/ Tested with 2.38.

Comment 1

[Arthur K. (he/him)]

Still busted in 2.39 20151021030937.

Comment 2

[Rainer Bielefeld]

REPRODUCIBLE with  en-US (German language pack) SeaMonkey 2.38  (Windows NT 6.1; WOW64; rv:41.0)  Gecko/20100101 Firefox/41.0 Build 20150923195647  (Classic Theme) on German WIN7 64bit

Additional Info:
a) What it should do you find here <https://wiki.mozilla.org/SeaMonkey/Help#Report_Web_Forgery_...>
b) Works fine with FF 44.0a1 (2015-10-28)

Comment 3

[Rainer Bielefeld]

e) Error console tells
   Timestamp: 01.11.2015 10:04:58
   Error: TypeError: URI is undefined
   Source File: resource://gre/modules/SafeBrowsing.jsm
   Line: 114

Comment 4

[Rainer Bielefeld]

c) Regression, worked fine with "SeaMonkey 2.35 (Windows NT 6.1; WOW64; rv:38.0)
   Gecko/20100101 Build 20150827182544 (Classic Theme) on German WIN7 64bit
d) currently only reproduced with WIN

Comment 5

[Rainer Bielefeld]

d): Also REPRODUCIBLE with  German SeaMonkey 2.38  (X11; Linux s86_64; rv:41.0)
    Gecko/20100101 Firefox/41.0 Build 20150923193515  (Classic Theme) 
    on VirtualBox Ubuntu 14

Comment 6

[Rainer Bielefeld]

e) Still REPRODUCIBLE with  German SeaMonkey 2.41a1 Mozilla/5.0 (Windows NT 6.1; 
   Win64; x64; rv:44.0 from akalla download area)  Gecko/20100101  Firefox/ 44.0
   Build 20150926114543, (Classic Theme) on German WIN7 64bit

f) Already REPRODUCIBLE with  SeaMonkey 2.38a1  (Windows NT 6.1; WOW64; rv:41.0 
   nightly by Philip Chee)  Gecko/20100101 Firefox/41.0 Build 20150620233542  
   (Classic Theme) on German WIN7 64bit

c): Was still ok with  SeaMonkey 2.38a1  (Windows NT 6.1; WOW64; rv:41.0 nightly
    from unknown Source)  Gecko/20100101 Firefox/41.0 Build 20150523065810 
    (Classic Theme) on German WIN7 64bit

Comment 7

[Frank-Rainer Grahl (:frg)]

Attachment: 1200263-reporturl.patch

Reporturl function changed. Now takes an URI as the second parameter.

Comment 8

[Frank-Rainer Grahl (:frg)]

Attachment: 1200263-reporturl-V2.patch

I copied the wrong patch out of my directory. It still contained an error. URI.clone would fail with the old one in SafeBrowsing.jsm.

Comment 9

[Justin Wood (:Callek)]

Comment on attachment 8702083 [details] [diff] [review]
1200263-reporturl-V2.patch

Review of attachment 8702083 [details] [diff] [review]:
-----------------------------------------------------------------

redirect review to :Ratty, I won't be able to get to this too soon

Comment 10

[Tristan Miller]

This bug is blocking Bug 1220707.  Could someone with the requisite permissions please mark it as such?

Comment 11

[Philip Chee]

Comment on attachment 8702083 [details] [diff] [review]
1200263-reporturl-V2.patch

>      // Remove the query to avoid including potentially sensitive data
>      if (pageUri instanceof Components.interfaces.nsIURL)
>        pageUri.query = "";
Remove this chunk as well since SafeBrowsing.getReportURL() already does this.

r=me with the above fixed.

Comment 12

[Frank-Rainer Grahl (:frg)]

Attachment: Patch to fix getReportURL function

Revised patch with review+ from Philip Chee carried forward.

Comment 13

[Philip Chee]

> -    var reportUrl = SafeBrowsing.getReportURL(aName);
.......
> +    reportUrl = SafeBrowsing.getReportURL(aName, pageUri);
> +
>      return reportUrl;

You removed the var so you need to reinsert it:
    var reportUrl = SafeBrowsing.getReportURL(aName, pageUri);
    return reportUrl;

Or even simpler:
    return SafeBrowsing.getReportURL(aName, pageUri);

I'll fix this on checkin.

Comment 14

[Frank-Rainer Grahl (:frg)]

Thanks. You are right. I didn't catch it. Wonder why it worked. Just tested again without the var.

FRG

Comment 15

[Philip Chee]

Pushed to comm-central:
http://hg.mozilla.org/comm-central/rev/afa5af5768bd

I also added the comment change from Bug 920951 so you should remove the whole hunk over there.

Comment 16

[Philip Chee]

> Thanks. You are right. I didn't catch it. Wonder why it worked. Just tested
> again without the var.

Without the var it becomes a global var and gets hoisted to the top of the file.

Comment 17

[Frank-Rainer Grahl (:frg)]

Thanks. Still learning:)

Will redo the patch in Bug 920951 anyway to put the comments in and start with a clean base.

FRG

Comment 18

[Philip Chee]

Comment on attachment 8722191 [details] [diff] [review]
Patch to fix  getReportURL function

[Approval Request Comment]
Regression caused by (bug #): various....

User impact if declined:
Click Help > Report Web Forgery. Does not take the user to the report phishing page site.

Testing completed (on m-c, etc.): comm-central

Risk to taking this patch (and alternatives if risky): none - bustage fix.

String changes made by this patch:

Comment 19

[rsx11m]

Comment on attachment 8722191 [details] [diff] [review]
Patch to fix  getReportURL function

Switching branch requests to reflect 2.44 to 2.45 merges.

Comment 20

[Ian Neal]

Comment on attachment 8722191 [details] [diff] [review]
Patch to fix  getReportURL function

a=me for c-b/c-r/c-esr

Comment 21

[rsx11m]

http://hg.mozilla.org/releases/comm-beta/rev/c65354f52d32

Comment 22

[rsx11m]

http://hg.mozilla.org/releases/comm-release/rev/8693b6d72a35
http://hg.mozilla.org/releases/comm-esr45/rev/10d7aed3d052