Closed Bug 464998 Opened 12 years ago Closed 12 years ago
integer overflow in ns
+++ This bug was initially created as a clone of Bug #455987 +++ +++ This bug was initially created as a clone of Bug #445117 +++ bug 455987 tried to fix this, but I added when I should have subtracted the size of the terminator in nsEscapeHTML2.
The patch that wrote up looks right (based on the simple tests cases we ran it through), but it feels like a lot of work to ensure something bad doesn't happen a developer does something bad with the APIs. I think we should just redefine these APIs to fail of you pass more than x-Mb of data or some other huge but possibly reasonable number. If you pass 4gb of data to nsEscapeHTML2, you are asking for trouble. :-/
Seems like it would be good to have xpcom/tests/TestEscape.cpp to test for these problems, no? Then you know what's actually been tried and what hasn't been tried in the tests, and perhaps this error might have been caught.
+1 great idea.
> If you pass 4gb of data to nsEscapeHTML2, you are asking for trouble. :-/ this is correct though known. check: Bug 367721 limiting virtual memory (mainly 64bit) Bug 445295 misuse of int32 and int64 in API on 64 bit platforms
offtopic: does someone know how the ibm bug |ptr[negativeidx] = stuff;| was discovered - manual human inspection, static analyzer or else? if it was static analyzer (which they may wish to promote for cash reasons) they may have more like this and probably *hydra checks for [negativeidx] may be considered.
Whiteboard: [sg:moderate] → [sg:moderate][needs r/sr bsmedberg]
Whiteboard: [sg:moderate][needs r/sr bsmedberg] → [sg:moderate]
Comment on attachment 348268 [details] [diff] [review] Get the sign right Approved for 188.8.131.52 and 184.108.40.206. a=ss for release-drivers
Attachment #348268 - Flags: approval1.9.1b2? → approval1.9.1b2+
Flags: blocking1.9.1? → blocking1.9.1+
Target Milestone: --- → mozilla1.9.1
Pushed to m-c: http://hg.mozilla.org/mozilla-central/rev/79948e2ddec5
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
no tests? qq
>no tests? qq there is a testcase in Bug 445117 for basically the same, though it is a perl script generating folder. testcases for this bug will use significant amount of VM.
Should I trust your ability to change the sign, Dan? :-)
You can also check Bonsai. 1.9.0: http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2008-11-18+12%3A45&maxdate=2008-11-18+12%3A45&cvsroot=%2Fcvsroot 1.8.1: http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=MOZILLA_1_8_BRANCH&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2008-11-18+12%3A45&maxdate=2008-11-18+12%3A45&cvsroot=%2Fcvsroot
You take the fun out of teasing Mr. Veditz but, yes, it is clearly fixed.
Comment on attachment 348268 [details] [diff] [review] Get the sign right a=asac for 1.8.0
Attachment #348268 - Flags: approval1.8.0.next? → approval1.8.0.next+
Hey Al, can you verify this bug across trunk and 1.9.1? I don't have access to bug 445117 due to security clearances :(
You need to log in before you can comment on or make changes to this bug.