Closed Bug 1452908 Opened 2 years ago Closed Last year

In about:blocked the link to the issue description ("reported as containing malicious software") isn't escaped properly, potentially making it contain incorrect information

Categories

(Toolkit :: Safe Browsing, defect, P3)

defect

Tracking

()

VERIFIED FIXED
mozilla64
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- wontfix
firefox59 --- wontfix
firefox60 --- wontfix
firefox61 --- wontfix
firefox62 --- wontfix
firefox63 --- verified
firefox64 --- verified

People

(Reporter: ecfbugzilla, Assigned: prathiksha)

References

Details

(Keywords: regression)

Attachments

(1 file)

When a website is blocked due to being malicious, you might have a link to the reported issue ("reported as containing malicious software") and some people might be inclined to take a look. This is currently not a good idea, the code under https://dxr.mozilla.org/mozilla-central/rev/83de58ddda2057f1cb949537f6b111e3b115ea3d/browser/base/content/content.js#187 fails to apply URI encoding to the page URL when producing this link, meaning that parameters from the malicious website URL might get injected there.

Steps to reproduce:

1. Enter http://sherpamail.com/?&site=http://sherpa.com/ into the location bar. I don't know anything about this site but it is currently blocked by Google SafeBrowsing.
2. Click "See details" button.
3. Click "reported as containing malicious software" link.

Expected results:

The page https://transparencyreport.google.com/safe-browsing/search?url=http:%2F%2Fsherpamail.com%2F shows up which tells you "This site is unsafe."

Actual results:

In Firefox 59.0.2 and 61.0a1 (2018-04-08) nightly the page https://transparencyreport.google.com/safe-browsing/search?url=http:%2F%2Fsherpa.com%2F shows up and tells you "No unsafe content found." People are likely to overlook the fact that this statement applies to a different website (sherpa.com rather than sherpamail.com) and assume a false positive. So they will be more inclined to use the "ignore the risk" link.
This is definitely a bug, and quite unfortunate, but it's not clear to me this is a security problem in any reasonable way. For one, safe browsing is a defense in depth thing. For another, it's not possible to manipulate the warning page in any other way, nor can the warning page be easilly framed, and so it's not really reasonably possible to tell people to go click 'see details', then click the broken link, then read that page, then come back to where they were and then accept the warning - all the while ignoring the full URL in the URL bar, which is correctly displayed and highlighted. As a result, I don't think this is an issue that needs to stay hidden, even if it's one that ought to be fixed.

Prathiksha, would you be interested in taking this?
Group: toolkit-core-security
Flags: needinfo?(prathikshaprasadsuman)
Summary: Link to issue description isn't escaped, can be manipulated → In about:blocked the link to the issue description ("reported as containing malicious software") isn't escaped properly, potentially making it contain incorrect information
Any patch here probably wants to land after bug 1452604.
See Also: → 1452604
(In reply to :Gijs (he/him) from comment #1)
> Prathiksha, would you be interested in taking this?

Yes
Assignee: nobody → prathikshaprasadsuman
Status: NEW → ASSIGNED
Flags: needinfo?(prathikshaprasadsuman)
Priority: -- → P3
(In reply to :Gijs (he/him) from comment #1)
> This is definitely a bug, and quite unfortunate, but it's not clear to me
> this is a security problem in any reasonable way.

Well, it's a way for something sending traffic to a malicious website to keep that traffic flowing a bit even after the site has been blacklisted by Google.

> As a result, I don't think this is an issue that needs to stay hidden

Sure, it's your call.
Flags: sec-bounty?
Flags: sec-bounty? → sec-bounty-
Unassigning myself from this bug. Sorry, I don't have time to work on this at the moment.
Assignee: prathikshaprasadsuman → nobody
Status: ASSIGNED → NEW
Fix the about:blocked link while checking Safe Browsing site status.
Assignee: nobody → prathikshaprasadsuman
Status: NEW → ASSIGNED
Comment on attachment 9013570 [details]
Bug 1452908 - Fix the about:blocked link while checking Safe Browsing site status. r?johannh

Johann Hofmann [:johannh] has approved the revision.
Attachment #9013570 - Flags: review+
Pushed by prathikshaprasadsuman@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/6a7574d2dfb8
Fix the about:blocked link while checking Safe Browsing site status. r=johannh
https://hg.mozilla.org/mozilla-central/rev/6a7574d2dfb8
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Please nominate this for Beta approval when you get a chance. It would also be nice to get QA to verify this fix, but it appears we'll need to find a new site to reproduce since the one from #c0 is no longer on the SafeBrowsing list.
Flags: qe-verify+
Flags: needinfo?(prathikshaprasadsuman)
(In reply to Ryan VanderMeulen [:RyanVM] from comment #10)
> Please nominate this for Beta approval when you get a chance. It would also
> be nice to get QA to verify this fix, but it appears we'll need to find a
> new site to reproduce since the one from #c0 is no longer on the
> SafeBrowsing list.

A possible test site is https://testsafebrowsing.appspot.com/s/phishing.html?foo=bar&site=mozilla.org

In the broken case we land on

https://transparencyreport.google.com/safe-browsing/search?url=mozilla.org

(report for mozilla.org)

in the fixed case we land on

https://transparencyreport.google.com/safe-browsing/search?url=https:%2F%2Ftestsafebrowsing.appspot.com%2Fs%2Fphishing.html%3Ffoo%3Dbar%26site%3Dmozilla.org
Comment on attachment 9013570 [details]
Bug 1452908 - Fix the about:blocked link while checking Safe Browsing site status. r?johannh

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: Bug 1452908

User impact if declined: The user may be misled into believing that an unsafe site is safe.

Is this code covered by automated tests?: No

Has the fix been verified in Nightly?: No

Needs manual test from QE?: Yes

If yes, steps to reproduce: See https://bugzilla.mozilla.org/show_bug.cgi?id=1452908#c0 and https://bugzilla.mozilla.org/show_bug.cgi?id=1452908#c11

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Does not break any existing code.

String changes made/needed: None.
Flags: needinfo?(prathikshaprasadsuman)
Attachment #9013570 - Flags: approval-mozilla-beta?
Comment on attachment 9013570 [details]
Bug 1452908 - Fix the about:blocked link while checking Safe Browsing site status. r?johannh

One liner, bug well understood and I verified in Nightly, uplift approved for 63 beta 13, thanks.
Attachment #9013570 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:64.0) Gecko/20100101 Firefox/64.0
Build ID: 20181008100121

Verified as fixed on the latest Nightly build. 

Waiting for the 63 beta 13 version.
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:63.0) Gecko/20100101 Firefox/63.0
Build ID: 20181008155858

Verified as fixed on the latest Beta build (v63beta13).

Site used for testing: 
http://www.wicar.org/resources.html
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.