fixed1.9.1, verified22.214.171.124, verified126.96.36.199
+++ 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.
Created attachment 348268 [details] [diff] [review] Get the sign right
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
Fix checked into 1.8 and 1.9.0 branches
Keywords: fixed220.127.116.11, fixed18.104.22.168
Pushed to m-c: http://hg.mozilla.org/mozilla-central/rev/79948e2ddec5
Status: NEW → RESOLVED
Last Resolved: 10 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.
Keywords: fixed22.214.171.124, fixed126.96.36.199 → verified188.8.131.52, verified184.108.40.206
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.