Last Comment Bug 871548 - Query params sent when reporting a phishing site could contain sensitive info.
: Query params sent when reporting a phishing site could contain sensitive info.
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: General (show other bugs)
: Trunk
: All All
: -- normal (vote)
: seamonkey2.21
Assigned To: Philip Chee
:
Mentors:
Depends on:
Blocks: 477718
  Show dependency treegraph
 
Reported: 2013-05-13 07:33 PDT by Philip Chee
Modified: 2013-06-01 09:43 PDT (History)
0 users
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
fixed
fixed


Attachments
Patch v1.0 Strip query params. (1.46 KB, patch)
2013-05-13 07:39 PDT, Philip Chee
neil: review+
iann_bugzilla: approval‑comm‑aurora+
Details | Diff | Splinter Review

Description Philip Chee 2013-05-13 07:33:42 PDT
From Firefox Bug 368106:
> However, Firefox is sending the full URL of the reported site -- including the 
> query string (http://foo.com/foo?querystring). To avoid privacy problems, the 
> browser probably shouldn't be sending the query part at all. Stripping out the 
> query values might also be an option (so that ...?user=me&pw=secret is 
> submitted as ...?user=&pw=).
> 
> It seems like just the hostname and URL path should be enough to identify a 
> phishing site.
Comment 1 Philip Chee 2013-05-13 07:39:00 PDT
Created attachment 748818 [details] [diff] [review]
Patch v1.0 Strip query params.

> +    // XXX: .clone() or cloneIgnoringRef() ?
> +    var pageUri = getBrowser().currentURI.cloneIgnoringRef();
Firefox uses .clone(). Would cloneIgnoringRef() be better?
Comment 2 neil@parkwaycc.co.uk 2013-05-13 08:57:56 PDT
(In reply to Philip Chee from comment #1)
> Firefox uses .clone(). Would cloneIgnoringRef() be better?
Makes sense.
Comment 3 neil@parkwaycc.co.uk 2013-05-13 08:59:50 PDT
Comment on attachment 748818 [details] [diff] [review]
Patch v1.0 Strip query params.

Seems reasonable (but without the XXX of course).
Comment 4 Philip Chee 2013-05-31 11:55:17 PDT
Pushed: http://hg.mozilla.org/comm-central/rev/fe1ad12926b0
Comment 5 Philip Chee 2013-05-31 12:05:13 PDT
Comment on attachment 748818 [details] [diff] [review]
Patch v1.0 Strip query params.

Note: Firefox Bug 368106 landed on Firefox23

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 477718 - Implement Phishing Protection (a.k.a. Safe Browsing) support in SeaMonkey

User impact if declined: Sensitive privacy information could leak.

Testing completed (on m-c, etc.): I've been running with this patch for about a fortnight and the Firefox changeset has been in m-c since 2013-05-07.

Risk to taking this patch (and alternatives if risky): Risk is low but since this problem is hypothetical it could ride the trains instead.

String or IDL/UUID changes made by this patch: None
Comment 6 Philip Chee 2013-06-01 09:43:31 PDT
Pushed to comm-aurora:
http://hg.mozilla.org/releases/comm-aurora/rev/8d0d26c68023

Note You need to log in before you can comment on or make changes to this bug.