Last Comment Bug 706249 - "ASSERTION: We've overflowed the mSpec buffer" in nsStandardURL::BuildNormalizedSpec
: "ASSERTION: We've overflowed the mSpec buffer" in nsStandardURL::BuildNormali...
Status: VERIFIED FIXED
[sg:critical][qa!]
: assertion, testcase
Product: Core
Classification: Components
Component: Networking (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla11
Assigned To: Randell Jesup [:jesup]
:
Mentors:
Depends on:
Blocks: 343943
  Show dependency treegraph
 
Reported: 2011-11-29 13:36 PST by Jesse Ruderman
Modified: 2012-02-23 17:27 PST (History)
6 users (show)
hskupin: in‑testsuite+
hskupin: in‑litmus-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-
wontfix
-
verified
+
verified
+
verified
unaffected


Attachments
testcase (24 bytes, text/html)
2011-11-29 13:36 PST, Jesse Ruderman
no flags Details
stack trace (2.53 KB, text/plain)
2011-11-29 13:37 PST, Jesse Ruderman
no flags Details
Patch with tests (4.84 KB, patch)
2011-11-29 19:44 PST, Randell Jesup [:jesup]
bzbarsky: review+
akeybl: approval‑mozilla‑aurora+
akeybl: approval‑mozilla‑beta+
Details | Diff | Splinter Review

Description Jesse Ruderman 2011-11-29 13:36:44 PST
Created attachment 577738 [details]
testcase

###!!! ASSERTION: We've overflowed the mSpec buffer!: 'mSpec.Length() <= approxLen', file netwerk/base/src/nsStandardURL.cpp, line 697

The last major change to this function was in bug 125608.
Comment 1 Jesse Ruderman 2011-11-29 13:37:02 PST
Created attachment 577739 [details]
stack trace
Comment 2 Randell Jesup [:jesup] 2011-11-29 17:52:01 PST
Caused by the null password entry.  Could fix by omitting it if empty, but that would be a change in behavior and it's not clear if that would even be correct (i.e. a null password is a password), so enforce adding one byte for a password (for the ':') even if the field is empty.  Patch follows.
Comment 3 Randell Jesup [:jesup] 2011-11-29 19:44:48 PST
Created attachment 577837 [details] [diff] [review]
Patch with tests

Tested; asserts without code fix, no assert with it
Comment 4 Randell Jesup [:jesup] 2011-11-29 19:51:17 PST
Worst-case analysis of this bug is it writes a '\0' to the byte following the allocation in some (not all) cases where the password is given but empty.
Comment 5 Boris Zbarsky [:bz] 2011-12-01 13:55:27 PST
Comment on attachment 577837 [details] [diff] [review]
Patch with tests

r=me
Comment 6 Randell Jesup [:jesup] 2011-12-02 21:27:08 PST
inbound via https://hg.mozilla.org/integration/mozilla-inbound/rev/8304db7e46bb

The original bug never existed in 1.9.x -> unaffected

Once it's green and merged to m-c I'll ask for approvals for Aurora and Beta
Comment 7 Randell Jesup [:jesup] 2011-12-03 04:42:35 PST
Merged to m-c
Comment 8 Randell Jesup [:jesup] 2011-12-03 04:44:56 PST
Comment on attachment 577837 [details] [diff] [review]
Patch with tests

Now in m-c; we should get it into aurora and beta soon
Comment 9 Alex Keybl [:akeybl] 2011-12-05 13:28:58 PST
Comment on attachment 577837 [details] [diff] [review]
Patch with tests

[Triage Comment]
Please land this sg:crit bug on aurora/beta asap so that we can bake this on beta for ~2 weeks.
Comment 10 Randell Jesup [:jesup] 2011-12-06 02:25:35 PST
Aurora: https://hg.mozilla.org/releases/mozilla-aurora/rev/12c96ed8154d
Beta: https://hg.mozilla.org/releases/mozilla-beta/rev/9f3a16bf8afc

Tracking for FF9 was confused - it is affected, and it was also approved for beta, so marking fixed.  Didn't touch tracking (JST?)
Comment 11 Henrik Skupin (:whimboo) 2011-12-15 04:51:58 PST
Verified fixed with the following tinderbox debug builds:

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:11.0a1) Gecko/20111214 Firefox/11.0a1

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:10.0a2) Gecko/20111214 Firefox/10.0a2

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:9.0) Gecko/20111212 Firefox/9.0

Mozilla/5.0 (X11; Linux i686; rv:9.0) Gecko/20111208 Firefox/9.0

I was not able to check on Windows because all the tinderbox debug builds seem to be broken and I can't start those.

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