Bug 734076 (CVE-2012-1966)

XSS with context menu

VERIFIED FIXED in Firefox 14

Status

()

Firefox
Security
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: moz_bug_r_a4, Assigned: Gavin)

Tracking

({sec-high})

unspecified
Firefox 15
sec-high
Points:
---

Firefox Tracking Flags

(firefox13 wontfix, firefox14+ verified, firefox15 fixed, firefox-esr1014+ fixed)

Details

(Whiteboard: [sg:high][advisory-tracking+])

Attachments

(2 attachments, 9 obsolete attachments)

(Reporter)

Description

5 years ago
http://mxr.mozilla.org/mozilla-central/source/browser/base/content/nsContextMenu.js

showOnlyThisFrame, viewMedia and viewBGImage disallow javascript: url but allow data: url.  Thus, it's possible to perform an XSS attack using data: url.
(Reporter)

Comment 1

5 years ago
Created attachment 604053 [details]
testcase 1 - showOnlyThisFrame

This tries to get cookies for www.mozilla.org.
(Reporter)

Comment 2

5 years ago
Created attachment 604054 [details]
testcase 2 - viewMedia

This tries to get cookies for www.mozilla.org.
(Reporter)

Comment 3

5 years ago
Created attachment 604057 [details]
testcase 3 - viewBGImage

This tries to get cookies for www.mozilla.org.
This is probably in the context-menu code. Toolkit?
Component: Security → Security
Product: Core → Toolkit
Whiteboard: [sg:high]
Drew, can you look at this asap?
Assignee: nobody → adw
Drew is on PTO this week. Re-assigning to David.
Assignee: adw → ddahl
Component: Security → Security
Product: Toolkit → Firefox
QA Contact: toolkit → firefox
The easy fix is to s/DISALLOW_SCRIPT/DISALLOW_INHERIT_PRINCIPAL/ everywhere in nsContextMenu. That might break some legitimate cases, though (e.g. viewing a data: image) - it might be nicer to use openLinkIn, and pass in disallowInheritPrincipal=true.

Comment 8

5 years ago
I will have an "easy fix" patch after a partial clobber. 

Gavin: I am thinking we will need a full blown browser chrome test with mouse events, right? If so, is there an existing test to model after?

Comment 9

5 years ago
Created attachment 606458 [details] [diff] [review]
Patch 1 - Easy Fix - Does not work

All test cases still work with this "easy fix" patch applied
Attachment #606458 - Flags: feedback?(gavin.sharp)

Comment 10

5 years ago
(In reply to David Dahl :ddahl from comment #9)
> Created attachment 606458 [details] [diff] [review]
> Patch 1 - Easy Fix - Does not work
> 
> All test cases still work with this "easy fix" patch applied

Sorry gavin, my emacs just had to remove some tabs or something in that patch. Sigh.

Comment 11

5 years ago
(In reply to Gavin Sharp (use gavin@gavinsharp.com for email) from comment #7)
> it might be nicer to use openLinkIn, and pass in disallowInheritPrincipal=true.

Are you saying that instead of using:

this.browser.loadURI(frameURL, referrer ? makeURI(referrer) : null);

change it to:

    openLinkIn(frameURL, "window", 
               { charset: doc.characterSet,
                 referrerURI: referrer ? makeURI(referrer) : null, disallowInheritPrincipal: true});

inside of the function showOnlyThisFrame, etc ?

Comment 12

5 years ago
(In reply to David Dahl :ddahl from comment #11)

This appears to work inside 'showOnlyThisFrame':

    // this.browser.loadURI(frameURL, referrer ? makeURI(referrer) : null);
    openLinkIn(frameURL, "window",
               { charset: doc.characterSet,
                 referrerURI: referrer ? makeURI(referrer) : null,
                 disallowInheritPrincipal: true });

The script was unable to display the cookie values - it was able to load mozilla.org into the frame

The other areas of concern are all using 'openUILink' which does not have a params argument, should this be changed in utilityOverlay.js?

I'll tinker with it...

Updated

5 years ago
Attachment #606458 - Flags: feedback?(gavin.sharp)

Comment 13

5 years ago
Created attachment 606715 [details] [diff] [review]
Patch 2 - targeting is incorrect

This patch fixes the cookie value snooping, but targets things to new tabs or a new window.
Attachment #606458 - Attachment is obsolete: true
Attachment #606715 - Flags: feedback?(gavin.sharp)
You want "current" to avoid changing the targeting behavior (it's the only case where disallowInheritPrincipal matters).

Rather than changing openUILink/openUILink's signature, you can just switch to using openLinkIn directly. I guess bug 631500 would be useful here.
Attachment #606715 - Flags: feedback?(gavin.sharp)

Comment 15

5 years ago
Created attachment 606775 [details] [diff] [review]
Patch 3 - working - needs tests

Ok, this works... also my targeting was wrong in the prev. patch as I had the tabs pinned, whoops.
Attachment #606715 - Attachment is obsolete: true
Attachment #606775 - Flags: feedback?(gavin.sharp)

Comment 16

5 years ago
I am unsure how to test this where we are targeting the iframe. I see this test: http://mxr.mozilla.org/mozilla-central/source/browser/base/content/test/browser_plainTextLinks.js#17

That approach will work for the background image, correct?
"current" doesn't actually maintain the current behavior. What openUILink does depends on the event.

Updated

5 years ago
Depends on: 631500

Updated

5 years ago
OS: Windows XP → All
Hardware: x86 → All
Comment on attachment 606775 [details] [diff] [review]
Patch 3 - working - needs tests

Looks like you can make use of the new functionality added in bug 631500 and avoid needing to switch from openUILink.
Attachment #606775 - Flags: feedback?(gavin.sharp)

Comment 19

5 years ago
Created attachment 607560 [details] [diff] [review]
Patch 4

I think this patch is correct, the params do leave out the 'charset' property as openUILinkIn does not accept one. 

Should I follow the testing method in this test? mxr.mozilla.org/mozilla-central/source/browser/base/content/test/browser_bug424101.js#19

Also, can this test method be used with the "show only this frame" test?
Attachment #606775 - Attachment is obsolete: true
Attachment #607560 - Flags: feedback?
Comment on attachment 607560 [details] [diff] [review]
Patch 4

>-    openUILink(viewURL, e, null, null, null, null, doc.documentURIObject );
>+    openUILinkIn(viewURL, "current", { disallowInheritPrincipal: true,
>+                                       allowThirdPartyFixup: false,
>+                                       postData: null,
>+                                       referrerURI: doc.documentURIObject });

Use openUILink instead of openUILinkIn. Remove "allowThirdPartyFixup: false" and "postData: null".

Comment 21

5 years ago
Created attachment 607621 [details] [diff] [review]
Patch 5
Attachment #607560 - Attachment is obsolete: true
Attachment #607621 - Flags: feedback?(dao)
Attachment #607560 - Flags: feedback?

Comment 22

5 years ago
(In reply to Dão Gottwald [:dao] from comment #20)

> Use openUILink instead of openUILinkIn. Remove "allowThirdPartyFixup: false"
> and "postData: null".

Sorry. I am a bit distracted right now by the ID work week. I think I have it now.

Updated

5 years ago
Attachment #607621 - Flags: feedback?(dao)

Comment 23

5 years ago
Comment on attachment 607621 [details] [diff] [review]
Patch 5

># HG changeset patch
># Parent 7bbac97c3ce2e3008e37dea1b2a229b2ebd343f3
>
>diff --git a/browser/base/content/nsContextMenu.js b/browser/base/content/nsContextMenu.js
>--- a/browser/base/content/nsContextMenu.js
>+++ b/browser/base/content/nsContextMenu.js
>@@ -762,17 +762,20 @@ nsContextMenu.prototype = {
>   // Open clicked-in frame in the same window.
>   showOnlyThisFrame: function() {
>     var doc = this.target.ownerDocument;
>     var frameURL = doc.location.href;
> 
>     urlSecurityCheck(frameURL, this.browser.contentPrincipal,
>                      Ci.nsIScriptSecurityManager.DISALLOW_SCRIPT);
>     var referrer = doc.referrer;
>-    this.browser.loadURI(frameURL, referrer ? makeURI(referrer) : null);
>+    openUILink(frameURL, "current", { disallowInheritPrincipal: true,
>+                                      allowThirdPartyFixup: false,
>+                                      postData: null,
>+                                      referrerURI: doc.documentURIObject });
>   },
> 
>   // View Partial Source
>   viewPartialSource: function(aContext) {
>     var focusedWindow = document.commandDispatcher.focusedWindow;
>     if (focusedWindow == window) 
>       focusedWindow = content;
> 
>@@ -834,17 +837,20 @@ nsContextMenu.prototype = {
>     else {
>       viewURL = this.mediaURL;
>       urlSecurityCheck(viewURL,
>                        this.browser.contentPrincipal,
>                        Ci.nsIScriptSecurityManager.DISALLOW_SCRIPT);
>     }
> 
>     var doc = this.target.ownerDocument;
>-    openUILink(viewURL, e, null, null, null, null, doc.documentURIObject );
>+    openUILink(viewURL, "current", { disallowInheritPrincipal: true,
>+                                     allowThirdPartyFixup: false,
>+                                     postData: null,
>+                                     referrerURI: doc.documentURIObject });
>   },
> 
>   saveVideoFrameAsImage: function () {
>     urlSecurityCheck(this.mediaURL, this.browser.contentPrincipal,
>                      Ci.nsIScriptSecurityManager.DISALLOW_SCRIPT);
>     let name = "";
>     try {
>       let uri = makeURI(this.mediaURL);
>@@ -870,17 +876,20 @@ nsContextMenu.prototype = {
>   },
> 
>   // Change current window to the URL of the background image.
>   viewBGImage: function(e) {
>     urlSecurityCheck(this.bgImageURL,
>                      this.browser.contentPrincipal,
>                      Ci.nsIScriptSecurityManager.DISALLOW_SCRIPT);
>     var doc = this.target.ownerDocument;
>-    openUILink(this.bgImageURL, e, null, null, null, null, doc.documentURIObject );
>+    openUILink(this.bgImageURL, "current", { disallowInheritPrincipal: true,
>+                                             allowThirdPartyFixup: false,
>+                                             postData: null,
>+                                             referrerURI: doc.documentURIObject });
>   },
> 
>   disableSetDesktopBackground: function() {
>     // Disable the Set as Desktop Background menu item if we're still trying
>     // to load the image or the load failed.
>     if (!(this.target instanceof Ci.nsIImageLoadingContent))
>       return true;
>
Attachment #607621 - Attachment is obsolete: true

Comment 24

5 years ago
Created attachment 607623 [details] [diff] [review]
Patch 6

Sorry about the spam
Attachment #607623 - Flags: feedback?(dao)
Comment on attachment 607623 [details] [diff] [review]
Patch 6

openUILink's second argument needs to be the event object
Attachment #607623 - Flags: feedback?(dao) → feedback-

Comment 26

5 years ago
Created attachment 607654 [details] [diff] [review]
Patch 7

It seems like 'this' is the event for 2 of the methods, but one of them is passed in 'e', correct?
Attachment #607623 - Attachment is obsolete: true
Attachment #607654 - Flags: feedback?(dao)
(In reply to David Dahl :ddahl from comment #26)
> Created attachment 607654 [details] [diff] [review]
> Patch 7
> 
> It seems like 'this' is the event for 2 of the methods, but one of them is
> passed in 'e', correct?

No... The two openUILink calls with the 'e' argument should remain openUILink calls with the 'e' argument. this.browser.loadURI you probably want to replace with openUILinkIn, since you have no event object there.

Comment 28

5 years ago
Created attachment 607691 [details] [diff] [review]
Patch 8
Attachment #607654 - Attachment is obsolete: true
Attachment #607691 - Flags: feedback?(dao)
Attachment #607654 - Flags: feedback?(dao)
Comment on attachment 607691 [details] [diff] [review]
Patch 8

>     urlSecurityCheck(frameURL, this.browser.contentPrincipal,
>                      Ci.nsIScriptSecurityManager.DISALLOW_SCRIPT);

Do we still need this?

>-    var referrer = doc.referrer;
>-    this.browser.loadURI(frameURL, referrer ? makeURI(referrer) : null);
>+    openUILinkIn(frameURL, "current", { disallowInheritPrincipal: true,
>+                                        referrerURI: doc.documentURIObject });

doc.referrer -> doc.documentURIObject looks like an unintended change

Comment 30

5 years ago
(In reply to Dão Gottwald [:dao] from comment #29)

> Do we still need this?
> 
> >-    var referrer = doc.referrer;
> >-    this.browser.loadURI(frameURL, referrer ? makeURI(referrer) : null);
> >+    openUILinkIn(frameURL, "current", { disallowInheritPrincipal: true,
> >+                                        referrerURI: doc.documentURIObject });
> 
> doc.referrer -> doc.documentURIObject looks like an unintended change

using the referrer variable causes this:

JavaScript error: , line 0: uncaught exception: [Exception... "Could not convert JavaScript argument arg 2 [nsIWebNavigation.loadURI]"  nsresult: "0x80570009 (NS_ERROR_XPC_BAD_CONVERT_JS)"  location: "JS frame :: chrome://global/content/bindings/browser.xml :: loadURIWithFlags :: line 193"  data: no]

with doc.documentURIObject, it works.
I think we need to keep the security checks - they serve a purpose beyond simply blocking javascript: (e.g. preventing loads of chrome-privileged pages).
(In reply to David Dahl :ddahl from comment #30)
> > doc.referrer -> doc.documentURIObject looks like an unintended change
> 
> using the referrer variable causes this:

That doesn't really make any sense - in fact openUILinkIn doesn't even use aCharset for the "current" case (this seems like an existing bug). How are you testing exactly?

Comment 33

5 years ago
(In reply to Gavin Sharp (use gavin@gavinsharp.com for email) from comment #32)
> How are you testing exactly?

I am testing via the test case attached to the bug: 

https://bug734076.bugzilla.mozilla.org/attachment.cgi?id=604053&t=2ZPFzMpYg0

Comment 34

5 years ago
(In reply to Gavin Sharp (use gavin@gavinsharp.com for email) from comment #32)
> That doesn't really make any sense - in fact openUILinkIn doesn't even use
> aCharset for the "current" case (this seems like an existing bug). How are
> you testing exactly?
So the LOAD_FLAGS used when this error occurs is '262144' - well that is what is dumped to the console in: http://mxr.mozilla.org/mozilla-central/source/toolkit/content/widgets/browser.xml#193

I'll keep digging...

Updated

5 years ago
Attachment #607691 - Flags: feedback?(dao)

Comment 35

5 years ago
I spent a little time working on browser chrome tests for this bug, but it requires cross domain loading of pages, which seems like something we don't want to do in our automated test suite. Is there an example of this kind of test or should this be manually tested?
The test suite supports cross-domain stuff, as long as you stick to domain names that the test proxy maps to the test http server.  See http://mxr.mozilla.org/mozilla-central/source/build/pgo/server-locations.txt for the list of supported hostnames and whatnot.

Comment 37

5 years ago
Created attachment 609720 [details] [diff] [review]
Patch 9 Test problem

Integrating the tests here - the test cases fail onload:

TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/browser/base/content/test/browser_bug_734076.js | an unexpected uncaught JS exception reported through window.onerror - uncaught exception: Load of  from http://example.com/browser/browser/base/content/test/test-bug-734076.html denied. at :0

Why would the same script throw like this inside a mochitest, but not in Nightly?

(The page does render, I am unsure what that error means.)
Attachment #607691 - Attachment is obsolete: true
Attachment #609720 - Flags: feedback?(dao)
urlSecurityCheck (called by viewBGImage) apparently throws this exception.
You can just use expectUncaughtException.

Comment 40

5 years ago
(In reply to Dão Gottwald [:dao] from comment #38)
> urlSecurityCheck (called by viewBGImage) apparently throws this exception.

How do you provide the proper event when calling viewBGImage?
You either open the menu for real and synthesize a mouse event or you fake the event object, but this doesn't have much to do with what I wrote, since urlSecurityCheck doesn't look at the event object.
Comment on attachment 609720 [details] [diff] [review]
Patch 9 Test problem

>+++ b/browser/base/content/test/browser_bug_734076.js

>+function tabLoad1(aEvent) {
>+  browser.removeEventListener(aEvent.type, arguments.callee, true);
>+  document.popupNode = document.firstChild;

document.firstChild is a browser.xul node.

Comment 43

5 years ago
(In reply to Dão Gottwald [:dao] from comment #41)
> You either open the menu for real and synthesize a mouse event or you fake
> the event object

When I try to synthesize the event, I am never able to open the contextmenu in content. I am however able to open one for chrome:

EventUtils.synthesizeMouse(tab.linkedBrowser.contentWindow.document.firstChild,
                           10,
                           10,
                           { type: "contextmenu", button: 2},
                           tab.linkedBrowser.contentWindow);

When I attempt to fake it and open the context menu programmatically, the browser opens about:logo

  document.popupNode = tab.linkedBrowser.contentWindow.document.firstChild;
  let contentAreaContextMenu = document.getElementById("contentAreaContextMenu");
  let contextMenu = new nsContextMenu(contentAreaContextMenu, gBrowser); // does not seem to open the menu
  expectUncaughtException();

  contextMenu.viewBGImage({ctrlKey: true}); // causes the load of about:logo


Can you explain how you would do this or point me to an example?
contentWindow.document.firstChild is <html> where you set about:logo as the background image, so this seems to be working as expected. If you want to get the body's background, use contentWindow.document.body. However, you need to make sure to do this /after/ the content page sets the background image.
Can we get an updated patch here, David? I do so hate sg:highs in the Firefox component.

Comment 46

5 years ago
(In reply to Johnathan Nightingale [:johnath] from comment #45)
> Can we get an updated patch here, David? I do so hate sg:highs in the
> Firefox component.

I apologize, this bug slipped off of my radar.

I am seeing completely different behavior using the nsContextMenu API vs testing by hand with mouse clicks.

I am quite sure the tests for this bug will take me a long time to write. In the interests of brevity, a more skilled mochitest writer (with EventUtils skillz) might be a better way to get this patch landed quickly.
Created attachment 616324 [details] [diff] [review]
patch with tests
Assignee: ddahl → gavin.sharp
Attachment #609720 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #616324 - Flags: review?(dao)
Attachment #609720 - Flags: feedback?(dao)
Comment on attachment 616324 [details] [diff] [review]
patch with tests

>diff --git a/browser/base/content/nsContextMenu.js b/browser/base/content/nsContextMenu.js

>   showOnlyThisFrame: function() {
>     var doc = this.target.ownerDocument;

>     var referrer = doc.referrer;
>+    openUILinkIn(frameURL, "current", { disallowInheritPrincipal: true,
>+                                        referrerURI: referrer ? makeURI(referrer) : null });

This code does actually look bogus (I don't think it makes sense to use the iframe's referrer as a referrer for the new load), but I'll file a separate bug about that (it also applies to the other frame menu items).
We probably also need to fix openLinkInCurrent. I forgot to audit for other places where we load things into the current page.
(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #48)
> This code does actually look bogus

(filed bug 746781)

Updated

5 years ago
Attachment #616324 - Flags: review?(dao) → review+
Keywords: sec-high
(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #49)
> We probably also need to fix openLinkInCurrent

This turns out to be unnecessary since the plain text link detection code will only activate for http[s] links. I audited the file for other potential "current" loads and didn't find any.
(We may still want to add some additional belt-and-braces protection to openLinkInCurrent, but I don't think we need to block this on it.)
https://hg.mozilla.org/integration/mozilla-inbound/rev/78e5d85cb6ac
Target Milestone: --- → Firefox 15
Comment on attachment 616324 [details] [diff] [review]
patch with tests

[Approval Request Comment]
Regression caused by (bug #): N/A
User impact if declined: XSS vulnerability if use can be convinced to use certain context menu functionality ("View Image", "Show only this frame", "View background image")
Testing completed (on m-c, etc.): Landed on inbound May 7th
Risk to taking this patch (and alternatives if risky): low risk - only affects specific context menu functionality, and test coverage is good.
String changes made by this patch: None
Attachment #616324 - Flags: approval-mozilla-aurora?
Given that the patch depends on bug 631500 (which is only on Aurora), backporting to beta would be a bit trickier. I'm not sure whether we should bother.
https://hg.mozilla.org/mozilla-central/rev/78e5d85cb6ac
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
status-firefox15: --- → fixed
Resolution: --- → FIXED
Comment on attachment 616324 [details] [diff] [review]
patch with tests

[Triage Comment]
Low risk sg:high fix. Approved for Aurora 14.

Is the ESR affected as well? If so, it sounds like we may decide not to fix there as well given bug 631500 is a dep.
Attachment #616324 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Yes, ESR is also affected. I can just look into also backporting bug 631500.
status-firefox-esr10: --- → affected
status-firefox13: --- → affected
status-firefox14: --- → affected
tracking-firefox-esr10: --- → 14+
Verified fixed in 5/13 Nightly trunk build.

Why did we check the tests in on an SG:High bug before release?
Status: RESOLVED → VERIFIED
I don't think the tests reveal much more than the patch does, fwiw (it's certainly not a "just copy this code to own someone" kind of test).

I think I'd like to skip trying to backport a fix for this to ESR. sg:high might overstate the importance of this...
Created attachment 636003 [details] [diff] [review]
ESR-10 patch

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: 
Fix Landed on Version: Firefox 15
Risk to taking this patch (and alternatives if risky): some context menu options could break. test coverage here is pretty good, though.
String or UUID changes made by this patch: none
Attachment #636003 - Flags: approval-mozilla-esr10?
tracking-firefox14: --- → ?
Attachment #616324 - Flags: approval-mozilla-beta?
Looks like I forgot to check this in on Aurora 14 when I got that approval :(

So I'm re-requesting approval for Beta 14.
status-firefox13: affected → wontfix
(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #60)
> I think I'd like to skip trying to backport a fix for this to ESR. sg:high
> might overstate the importance of this...

In case it wasn't clear from the activity, I had a change of heart.
(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #63)
> (In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment
> #60)
> > I think I'd like to skip trying to backport a fix for this to ESR. sg:high
> > might overstate the importance of this...
> 
> In case it wasn't clear from the activity, I had a change of heart.

Do you think this needs to be rushed into beta in that case? Or can we let this ride the trains at this point?
tracking-firefox-esr10: 14+ → ?
I don't think it needs to be "rushed" any more than any other security bug. I think we should take it on beta, though. We're only halfway through the cycle, so I don't really see any reason not to take it (ESR is a bit more complicated given dependency on bug 631500, but even that is pretty straightforward I think).
Comment on attachment 616324 [details] [diff] [review]
patch with tests

[Triage Comment]
This will go into the fifth beta, so we'll still have one more opportunity to fix any regressions. Approved for Beta.
Attachment #616324 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Updated

5 years ago
Attachment #636003 - Flags: approval-mozilla-esr10? → approval-mozilla-esr10+

Updated

5 years ago
tracking-firefox-esr10: ? → 14+
tracking-firefox14: ? → +
https://hg.mozilla.org/releases/mozilla-beta/rev/a017dff24aa4
status-firefox14: affected → fixed
https://hg.mozilla.org/releases/mozilla-esr10/rev/a7b0106943ba
status-firefox-esr10: affected → fixed
Whiteboard: [sg:high] → [sg:high][advisory-tracking+]
Alias: CVE-2012-1966
This is not fixed in 14.0.1. I just tried with the release candidate build and compared it against trunk (7/15) and Firefox 13.0.1 behavior. Firefox 14.0.1 is behaving the same as 13.0.1 and following the various "view..." context menu items in the attached testcases is showing a data:url source on both 13.0.1 and 14.0.1 but not nightly.
status-firefox14: fixed → affected
All right, there are issues with the PoC's. Gavin whipped up a new one and I reconfirmed that this is properly fixed in 14.0.1 and trunk.
status-firefox14: affected → verified
Al, where is the test you used to verify this? I'd like to test against ESR and 15.
There are three tests attached to this bug. AFAIK, that's what I used.
Okay, I'll try using those tests. I guess my confusion was in comment 69 and 70. It wasn't clear to me that those tests were ones which Gavin had "fixed".
Group: core-security
You need to log in before you can comment on or make changes to this bug.