Bug 296850 (sa15601)

Frame injection spoofing with targets (bug 246448 returns - SA15601)

RESOLVED FIXED

Status

()

Core
Document Navigation
RESOLVED FIXED
12 years ago
11 years ago

People

(Reporter: dveditz, Assigned: jst)

Tracking

({fixed-aviary1.0.5, fixed1.7.9, regression})

Trunk
fixed-aviary1.0.5, fixed1.7.9, regression
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.7.9 +
blocking-aviary1.0.5 +
blocking1.8b3 +
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:fix] workaround: force new windows to open in tabs, URL)

Attachments

(5 attachments)

(Reporter)

Description

12 years ago
Frame injection spoofing came back. It was fixed by bug 246448 in Mozilla 1.7
and remained fixed through Firefox 1.0.2

Firefox 1.0.3 and Mozilla 1.7.7 are vulnerable again (as is the trunk/Deer Park).
(Reporter)

Updated

12 years ago
No longer depends on: 246448
(Reporter)

Updated

12 years ago
Flags: blocking1.8b3+
Flags: blocking1.7.9+
Flags: blocking-aviary1.0.5+
Whiteboard: [sg:fix]

Comment 1

12 years ago
Can an automated regression test for this bug be added to the build process so
it won't rear its head again?
URL to test with for now:

data:text/html,<a
href="http://web.mit.edu/bzbarsky/www/testcases/security/frameset.html"
target="_blank">Click here first</a><a href="http://www.mozilla.org"
target="frameUniqueNameNoOneWillThinkOf">Click here second</a>
OK, this is a regression from bug 289204.  Here's what happens:

When we try to do the targeted load, we look for an existing named target.  The
search fails, since there is no target we can access.  We then go ahead and open
a new named window via the window watcher.  Now opening a named window actually
just calls nsWindowWatcher::OpenWindowJS.  This, since bug 289204 was fixed,
pushes the JSContext of the window on the stack, since the caller is C++ code
and thus IsCallerChrome tests true.

So we go ahead and push this JSContext on the stack, then try to open the
window.  But first, we look for an existing window by this name (since
window.open doesn't do such a search itself and all -- this is where the search
happens).  Now when we end up in nsDocShell::ValidateOrigin, we have a JSContext
on the stack.  So the call to GetSubjectPrincipal returns a principal (it
returns null before the change for bug 289204!).  So we do a IsCapabilityEnabled
check for UniversalBrowserWrite (since we have a principal).  But
IsCapabilityEnabled returns PR_TRUE if there is no stack frame (which there
isn't, in this case), no matter whether a JSContext is on the stack or not.  So
we think we have permission to access the docshell and we do the load.

As a stopgap, we could make IsCapabilityEnabled return !cx instead of PR_TRUE
for the !fp case...  Not sure what else that would affect, though.
Blocks: 289204
Or perhaps IsCapabilityEnabled should get the principal from cx like
GetSubjectPrincipal does for cases when fp is null?

Comment 5

12 years ago
Created attachment 185528 [details]
sidebar testcase

This is uncommon example but produces the same results.

Comment 6

12 years ago
This doesn't work for me. The attempted injection loads in a new tab. Could this
be a config option protecting me?

(Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050601
Firefox/1.0+)
> Could this be a config option protecting me?

Yes.
(Assignee)

Comment 8

12 years ago
Created attachment 185581 [details] [diff] [review]
Move the JS context pushing code into windowwatcher

This fixes this bug by moving the JS context pushing code from
nsGlobalWindow::OpenInternal() into the windowwatcher code that does the actual
opening. By doing this we can delay the pushing of the callee windows context
onto the stack to past the place where we look for an existing window, and that
way things work as they should again.
Attachment #185581 - Flags: superreview?(brendan)
Attachment #185581 - Flags: review?(bzbarsky)
Comment on attachment 185581 [details] [diff] [review]
Move the JS context pushing code into windowwatcher

Excellent.
Attachment #185581 - Flags: review?(bzbarsky) → review+
(Reporter)

Updated

12 years ago
Whiteboard: [sg:fix] → [sg:fix] workaround: force new windows to open in tabs
Comment on attachment 185581 [details] [diff] [review]
Move the JS context pushing code into windowwatcher

sr+a=me for trunk and both branches.

/be
Attachment #185581 - Flags: superreview?(brendan)
Attachment #185581 - Flags: superreview+
Attachment #185581 - Flags: approval1.8b3+
Attachment #185581 - Flags: approval1.7.9+
Attachment #185581 - Flags: approval-aviary1.0.5+

Updated

12 years ago
Summary: Frame injection spoofing (bug 246448 returns - SA15601) → Frame injection spoofing with targets (bug 246448 returns - SA15601)
(Assignee)

Comment 11

12 years ago
Created attachment 185617 [details] [diff] [review]
aviary branch version of the above.
(Assignee)

Comment 12

12 years ago
Comment on attachment 185581 [details] [diff] [review]
Move the JS context pushing code into windowwatcher

For the record, there's an unused nsCOMPtr<nsIJSContextStack> stack variable in
this trunk patch, I'll remove that before checking in (already removed in the
branch version of this diff)
(Assignee)

Comment 13

12 years ago
Created attachment 185620 [details] [diff] [review]
1.7 branch version of the above.

Comment 14

12 years ago
Created attachment 185636 [details] [diff] [review]
fix mozilla_1_7_branch's fire

jst's patch burned Mozilla 1.7 branch

Updated

12 years ago
Attachment #185636 - Flags: approval1.7.9?

Comment 15

12 years ago
Comment on attachment 185636 [details] [diff] [review]
fix mozilla_1_7_branch's fire

a=#developers
Attachment #185636 - Flags: approval1.7.9?

Comment 16

12 years ago
(In reply to comment #6)
> This doesn't work for me. The attempted injection loads in a new tab. Could this
> be a config option protecting me?
> 
> (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050601
> Firefox/1.0+)

FYI: The TabMix extension fixes this bug, I assume this is because
target="_blank"'s are opened in new tabs instead of new windows.

Haydn.

Comment 17

12 years ago
Setting this pref in all.js for the Suite worked for me ...

  pref("browser.frame.validate_origin", true); 
Pete, that pref defaults to true....  See
http://lxr.mozilla.org/seamonkey/source/docshell/base/nsDocShell.cpp#3359

Comment 19

12 years ago
(In reply to comment #18)
> Pete, that pref defaults to true....  See
> http://lxr.mozilla.org/seamonkey/source/docshell/base/nsDocShell.cpp#3359

Yes, but I believe because the pref doesn't exist, the call to

  rv = mPrefs->GetBoolPref("browser.frames.enabled", &tmpbool);

returns a failure code, in turn mAllowSubframes is not set to true.

See:

  http://lxr.mozilla.org/seamonkey/source/docshell/base/nsDocShell.cpp#3350

It has already been verified having this pref set to true fixes this issue on
the suite (we applied to 1.6 branch for Linspire). 

I have verified it with binary distributions. 

Mozilla Suite: 1.6, 1.7.5, 1.7.6 - that's as far as I tested.

It doesn't seem to work w/ Firefox (not sure why). 

Comment 20

12 years ago
> 
> See:
> 
>   http://lxr.mozilla.org/seamonkey/source/docshell/base/nsDocShell.cpp#3350

Scratch that, I was looking at the wrong per there ...

Anyway, the point being it is a simple fix for the suite ...
(Assignee)

Comment 21

12 years ago
Fixed on trunk and branches.
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Keywords: fixed-aviary1.0.5, fixed1.7.9
Resolution: --- → FIXED
Pete, Mozilla 1.7.5 and 1.7.6, are not vulnerable to this bug -- this bug was a
regression starting with 1.7.7.  See comment 0.  So I'm not sure what you were
really testing, but it wasn't this bug...

Comment 23

12 years ago
(In reply to comment #22)
> Pete, Mozilla 1.7.5 and 1.7.6, are not vulnerable to this bug -- this bug was a
> regression starting with 1.7.7.  See comment 0.  So I'm not sure what you were
> really testing, but it wasn't this bug...

I used the pref to fix 1.6 which is vulnerable by this bug. Sorry about the 1.7
mixup.
Ah, yes.  In 1.6 the pref defaulted to false.
*** Bug 296736 has been marked as a duplicate of this bug. ***
(Reporter)

Comment 26

12 years ago
*** Bug 297820 has been marked as a duplicate of this bug. ***
(Reporter)

Updated

12 years ago
Alias: sa15601
(Reporter)

Comment 27

12 years ago
*** Bug 299189 has been marked as a duplicate of this bug. ***

Comment 28

12 years ago
Is there a specific patch for this bug fix?

Are the only files fixed related to this security issue:
mozilla/dom/src/base/nsGlobalWindow.cpp and
mozilla/embedding/components/windowwatcher/src/nsWindowWatcher.cpp ?

What are the cvs revision numbers for the fixed files? Does mozilla offer any
webbased cvs (cvsweb) so I can review diffs?

Does this fix also cover http://secunia.com/advisories/15489/ ?

(I am working on updating firefox package for pkgsrc.)
>What are the cvs revision numbers for the fixed files? Does mozilla offer any
>webbased cvs (cvsweb) so I can review diffs?

you can use http://bonsai.mozilla.org, querying by date should hopefully give
you the revision numbers.
> Is there a specific patch for this bug fix?

Yes, it's attached to this bug.

For the rest, see lxr.mozilla.org and bonsai.mozilla.org
(Reporter)

Comment 31

12 years ago
(In reply to comment #28)
> Does this fix also cover http://secunia.com/advisories/15489/ ?

No, that's a completely unrelated issue. See bug 298934

Updated

12 years ago
Flags: testcase+

Updated

11 years ago
Flags: in-testsuite+ → in-testsuite?
You need to log in before you can comment on or make changes to this bug.