Last Comment Bug 545927 - xslt number formatting function crashes on big numbers [@ txRomanCounter::appendNumber(int, nsAString_internal&) ]
: xslt number formatting function crashes on big numbers [@ txRomanCounter::ap...
Status: RESOLVED FIXED
: crash, testcase, verified1.9.1, verified1.9.2
Product: Core
Classification: Components
Component: XSLT (show other bugs)
: 1.9.2 Branch
: x86 Windows 7
: -- critical (vote)
: ---
Assigned To: Boris Zbarsky [:bz] (still a bit busy)
:
:
Mentors:
http://crash-stats.mozilla.com/report...
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-02-12 14:16 PST by Sergey Glazunov
Modified: 2011-06-09 14:58 PDT (History)
9 users (show)
bzbarsky: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
.7+
.7-fixed
.11+
.11-fixed


Attachments
malformed xslt document (209 bytes, text/xml)
2010-02-12 14:18 PST, Sergey Glazunov
no flags Details
xml file to open (55 bytes, text/xml)
2010-02-12 14:19 PST, Sergey Glazunov
no flags Details
one file javascript version (719 bytes, text/html)
2010-02-13 10:40 PST, Sergey Glazunov
no flags Details
Proposed fix (2.48 KB, patch)
2010-02-16 17:54 PST, Boris Zbarsky [:bz] (still a bit busy)
jonas: review+
dveditz: approval1.9.2.7+
dveditz: approval1.9.1.11+
Details | Diff | Splinter Review

Description Sergey Glazunov 2010-02-12 14:16:54 PST
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US) AppleWebKit/532.5 (KHTML, like Gecko) Chrome/4.0.249.89 Safari/532.5
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.1; ru; rv:1.9.2) Gecko/20100115 Firefox/3.6

Mozilla crashes while transforms a large number (like 1000000000000000) into the Roman numeral system using XSLT (<xsl:number value="1000000000000000" format="i"/>).

Reproducible: Always
Comment 1 Sergey Glazunov 2010-02-12 14:18:15 PST
Created attachment 426745 [details]
malformed xslt document
Comment 2 Sergey Glazunov 2010-02-12 14:19:21 PST
Created attachment 426746 [details]
xml file to open
Comment 3 Marcia Knous [:marcia - use ni] 2010-02-12 15:47:56 PST
I don't crash using  Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2) Gecko/20100115 Firefox/3.6. I tried using a trunk build and it seemed when I tried to shut down that Minefield closed. Do you have a crash report from about:crashes (in the URL bar)?
Comment 5 Sergey Glazunov 2010-02-13 01:10:29 PST
i found out where the bug in the code is.
there is the check in the txRomanCounter::appendNumber method:
if (aNumber >= 4000) {
but aNumber is considered as signed int, so it passes the check if it equals at least 2147483648.
Comment 6 Matthias Versen [:Matti] 2010-02-13 07:19:52 PST
0  	xul.dll  	txRomanCounter::appendNumber  	 content/xslt/src/xslt/txXSLTNumberCounters.cpp:239
1 	xul.dll 	txXSLTNumber::createNumber 	content/xslt/src/xslt/txXSLTNumber.cpp:94
2 	xul.dll 	txNumber::execute 	content/xslt/src/xslt/txInstructions.cpp:597
3 	xul.dll 	txXSLTProcessor::execute 	content/xslt/src/xslt/txXSLTProcessor.cpp:104
4 	xul.dll 	txMozillaXSLTProcessor::TransformToDoc 	content/xslt/src/xslt/txMozillaXSLTProcessor.cpp:683
Comment 7 Sergey Glazunov 2010-02-13 10:40:45 PST
Created attachment 426838 [details]
one file javascript version
Comment 8 Boris Zbarsky [:bz] (still a bit busy) 2010-02-16 17:54:25 PST
Created attachment 427251 [details] [diff] [review]
Proposed fix

Serg, thank you for the testcase and analysis!
Comment 9 Jonas Sicking (:sicking) No longer reading bugmail consistently 2010-02-16 17:59:19 PST
Comment on attachment 427251 [details] [diff] [review]
Proposed fix

Sweet, thanks guys!
Comment 10 Boris Zbarsky [:bz] (still a bit busy) 2010-02-19 22:44:18 PST
Pushed as http://hg.mozilla.org/mozilla-central/rev/4c8923d18e2e

I guess we should try to get this in on the branch too....
Comment 11 Boris Zbarsky [:bz] (still a bit busy) 2010-02-19 22:45:05 PST
Comment on attachment 427251 [details] [diff] [review]
Proposed fix

Requesting approval for this "don't try to read memory we haven't mapped" fix.
Comment 12 Mike Beltzner [:beltzner, not reading bugmail] 2010-03-17 12:59:41 PDT
Comment on attachment 427251 [details] [diff] [review]
Proposed fix

Uhm, isn't that the very definition of a security bug?
Comment 13 Boris Zbarsky [:bz] (still a bit busy) 2010-05-27 12:23:33 PDT
> Uhm, isn't that the very definition of a security bug?

Well, it can be, yes.
Comment 14 Daniel Veditz [:dveditz] 2010-06-02 10:20:12 PDT
Is the 1.9.1 branch affected as well?
Comment 15 Daniel Veditz [:dveditz] 2010-06-02 10:20:41 PDT
Comment on attachment 427251 [details] [diff] [review]
Proposed fix

Approved for 1.9.2.5, a=dveditz for release-drivers
Comment 16 Boris Zbarsky [:bz] (still a bit busy) 2010-06-11 20:25:11 PDT
> Is the 1.9.1 branch affected as well?

Yes.
Comment 17 Boris Zbarsky [:bz] (still a bit busy) 2010-06-11 20:39:01 PDT
Pushed http://hg.mozilla.org/releases/mozilla-1.9.2/rev/db34c710c17e

Here's hoping I'm setting the status flag right... (i.e. NOT matching the approval flag).
Comment 18 Daniel Veditz [:dveditz] 2010-06-12 00:15:47 PDT
Comment on attachment 427251 [details] [diff] [review]
Proposed fix

You did that right, thanks. We're a little off-kilter due to the decision to split Fennec 1.1 off mid-release.

Approved for 1.9.1.11, a=dveditz
Comment 19 Boris Zbarsky [:bz] (still a bit busy) 2010-06-24 21:34:08 PDT
Pushed http://hg.mozilla.org/releases/mozilla-1.9.1/rev/3d53e20b39a5
Comment 20 Al Billings [:abillings] 2010-07-15 18:12:06 PDT
Verified for 1.9.1.11 using testcase in comment 7 (which crashes 1.9.1.10): Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.11) Gecko/20100701 Firefox/3.5.11 (.NET CLR 3.5.30729).

Verified for 1.9.2.7 the same way with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.7) Gecko/20100713 Firefox/3.6.7 (.NET CLR 3.5.30729).

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