Closed Bug 296850 (sa15601) Opened 20 years ago Closed 20 years ago

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

Categories

(Core :: DOM: Navigation, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: dveditz, Assigned: jst)

References

()

Details

(Keywords: fixed-aviary1.0.5, fixed1.7.9, regression, Whiteboard: [sg:fix] workaround: force new windows to open in tabs)

Attachments

(5 files)

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).
No longer depends on: 246448
Flags: blocking1.8b3+
Flags: blocking1.7.9+
Flags: blocking-aviary1.0.5+
Whiteboard: [sg:fix]
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?
Attached file sidebar testcase (deleted) —
This is uncommon example but produces the same results.
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.
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+
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+
Summary: Frame injection spoofing (bug 246448 returns - SA15601) → Frame injection spoofing with targets (bug 246448 returns - SA15601)
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)
Attached patch fix mozilla_1_7_branch's fire (deleted) — Splinter Review
jst's patch burned Mozilla 1.7 branch
Attachment #185636 - Flags: approval1.7.9?
Comment on attachment 185636 [details] [diff] [review] fix mozilla_1_7_branch's fire a=#developers
Attachment #185636 - Flags: approval1.7.9?
(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.
Setting this pref in all.js for the Suite worked for me ... pref("browser.frame.validate_origin", true);
(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).
> > 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 ...
Fixed on trunk and branches.
Status: NEW → RESOLVED
Closed: 20 years ago
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...
(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. ***
*** Bug 297820 has been marked as a duplicate of this bug. ***
Alias: sa15601
*** Bug 299189 has been marked as a duplicate of this bug. ***
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
(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
Flags: testcase+
Flags: in-testsuite+ → in-testsuite?
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: