Closed
Bug 549566
Opened 14 years ago
Closed 14 years ago
It is possible to cut off beforeunload message by using null bytes
Categories
(Core :: DOM: Events, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: jwkbugzilla, Assigned: smaug)
Details
Attachments
(1 file, 1 obsolete file)
5.79 KB,
patch
|
Details | Diff | Splinter Review |
This is similar to bug 310037 but affects the message popping up for pages with beforeunload event handler. To reproduce copy this into the location bar: javascript:void(window.onbeforeunload=function(){return "foo\0"}) If you then try to navigate away from the current page you will see the message: > Are you sure you want to navigate away from this page? > > foo The last sentence ("Press OK to continue, or Cancel to stay on the current page.") is cut off. I don't think this is a big issue with English but it might be more problematic with other languages, marking it as security sensitive just in case. Stripping null characters in nsDOMBeforeUnloadEvent::SetReturnValue certainly won't do any harm either way. Firefox 3.6 and Minefield build 20100301 are affected, Internet Explorer 8 is not.
Assignee | ||
Updated•14 years ago
|
Assignee: nobody → Olli.Pettay
Assignee | ||
Comment 1•14 years ago
|
||
Do the same as what was done in Bug 310037.
Attachment #429764 -
Flags: review?(jonas)
Comment on attachment 429764 [details] [diff] [review] patch >+nsContentUtils::StripNullChars(const nsAString& aInStr, nsAString& aOutStr) >+{ >+ // In common cases where we don't have nulls in the >+ // string we can simple simply bypass the checking code. >+ PRInt32 firstNullPos = aInStr.FindChar('\0'); >+ if (firstNullPos == kNotFound) { >+ aOutStr.Assign(aInStr); >+ return; >+ } >+ >+ nsAString::const_iterator start, end; >+ aInStr.BeginReading(start); >+ aInStr.EndReading(end); >+ >+ while (start != end) { >+ if (*start != '\0') >+ aOutStr.Append(*start); >+ ++start; >+ } >+} Would probably be worth doing a aOutStr.SetCapacity(aInStr.Length() - 1); call before the last while-loop. Yes, it can waste some memory, but it'll avoid O(n^2) behavior. r=me with that
Attachment #429764 -
Flags: review?(jonas) → review+
Comment 3•14 years ago
|
||
I don't think this needs to be a security bug, the buttons really should be self-explanatory given the initial text. I think we do want to fix this on the old branches though.
Assignee | ||
Comment 4•14 years ago
|
||
Attachment #429764 -
Attachment is obsolete: true
Assignee | ||
Comment 5•14 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/d6c758c21659
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•