Closed Bug 368106 Opened 14 years ago Closed 8 years ago
Query params sent when reporting a phishing site could contain sensitive info
A security alert from Finjan was recently issued (see URL), which reported that Google's phishing blacklist contained usernames, passwords, etc. It appears that Google is now filtering their list to remove such entries. 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. Also, check to make sure we strip http://user:pass@host/ type URLs. Addendum: In an old urlclassifier.sqlite, I see a few URLs in the form "http://[google IP]search?q=cache:xxxxxxxxxx". This implies one can phish from legitimate sites using just the query string. Proxy-like sites, such as translate.google.com, have been used in the past for other tricks like this. So there may be a security-vs-privacy tug of war here.
Summary: Query parms sent when reporting a phishing site, could contain sensitive info → Query params sent when reporting a phishing site could contain sensitive info
Simple to fix, seems like we should do it. getReportURL in browser/base/content/browser-safebrowsing.js just needs to clone the currentURI, see whether it's instanceof Ci.nsIURL, and if so strip out the "query" portion.
Version: 2.0 Branch → unspecified
Assignee: nobody → maxli
Attachment #745636 - Flags: review?(gavin.sharp)
Comment on attachment 745636 [details] [diff] [review] Patch Adding a comment about why we're stripping the query seems worthwhile (e.g. "remove query to avoid including potentially sensitive data"). I'd also re-name the variable "pageURI" now that it's an nsIURI, since that is more conventional (that you then QI it to nsIURL is potentially confusing, but not a big deal). Looks good otherwise!
Attachment #745636 - Flags: review?(gavin.sharp) → review+
Patch with fixes
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 23
You need to log in before you can comment on or make changes to this bug.