Last Comment Bug 360293 - passing in non-numeric values to canvas functions causes crash inside cairo
: passing in non-numeric values to canvas functions causes crash inside cairo
Status: VERIFIED FIXED
[sg:critical?]
: crash, testcase, verified1.8.0.9, verified1.8.1.1
Product: Core
Classification: Components
Component: Canvas: 2D (show other bugs)
: Trunk
: x86 All
: -- normal (vote)
: ---
Assigned To: Vladimir Vukicevic [:vlad] [:vladv]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2006-11-10 11:46 PST by Vladimir Vukicevic [:vlad] [:vladv]
Modified: 2009-04-24 10:57 PDT (History)
5 users (show)
vladimir: blocking1.9+
dveditz: blocking1.8.1.1+
dveditz: blocking1.8.0.9+
bob: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (crashes once you click crash button) (676 bytes, text/html)
2006-11-10 11:47 PST, Vladimir Vukicevic [:vlad] [:vladv]
no flags Details
add float value checks to canvas (8.72 KB, patch)
2006-11-10 11:52 PST, Vladimir Vukicevic [:vlad] [:vladv]
no flags Details | Diff | Splinter Review
fixed patch using JSDOUBLE_IS_FINITE. patch for 1.8.1/1.9. (8.95 KB, patch)
2006-11-20 12:11 PST, Vladimir Vukicevic [:vlad] [:vladv]
dveditz: review+
dveditz: approval1.8.1.1+
Details | Diff | Splinter Review
validate floats, patch for 1.8.0.x (11.00 KB, patch)
2006-11-21 10:53 PST, Vladimir Vukicevic [:vlad] [:vladv]
dveditz: review+
dveditz: approval1.8.0.9+
Details | Diff | Splinter Review

Description Vladimir Vukicevic [:vlad] [:vladv] 2006-11-10 11:46:19 PST
The Canvas API uses floats in the IDL, so the C++ method signatures just accept floats.  xpconnect uses JS_ValueToNumber to do the conversion from jsval to double, and ValueToNumber happily coverts non-numeric values (e.g. "foo") to NaN.  Cairo doesn't handle NaN (or, I assume, +/-Inf) well, and so it leads to a crash.
Comment 1 Vladimir Vukicevic [:vlad] [:vladv] 2006-11-10 11:47:12 PST
Created attachment 245245 [details]
testcase (crashes once you click crash button)

Testcase
Comment 2 Vladimir Vukicevic [:vlad] [:vladv] 2006-11-10 11:51:41 PST
I'm not sure if this is exploitable -- we should backport to 1.8.0.9 if so (trivial backport), otherwise we should just get it in to 1.8.1.x.
Comment 3 Vladimir Vukicevic [:vlad] [:vladv] 2006-11-10 11:52:39 PST
Created attachment 245246 [details] [diff] [review]
add float value checks to canvas

Add checks to make sure each float value passed in is finite; error out otherwise.
Comment 4 Daniel Veditz [:dveditz] 2006-11-10 15:01:34 PST
In a windows debug build it crashes when we pass an invalid pointer to realloc, that's potentially bad.
Comment 5 Vladimir Vukicevic [:vlad] [:vladv] 2006-11-20 12:11:33 PST
Created attachment 246057 [details] [diff] [review]
fixed patch using JSDOUBLE_IS_FINITE.  patch for 1.8.1/1.9.

Better patch, using jsnum.h and JSDOUBLE_IS_FINITE (finitef()/__finitef() are not well supported, as far as I can tell).  Separate patch for 1.8.0 coming soon.
Comment 6 Vladimir Vukicevic [:vlad] [:vladv] 2006-11-21 10:53:15 PST
Created attachment 246173 [details] [diff] [review]
validate floats, patch for 1.8.0.x

Patch for 1.8.0; untested right now (because I can't build 1.8.0 on my mac), but I'll fire up a linux vm for it.  It's pretty straightforward, though.
Comment 7 Daniel Veditz [:dveditz] 2006-11-21 14:31:38 PST
Comment on attachment 246173 [details] [diff] [review]
validate floats, patch for 1.8.0.x

r=dveditz

NS_ERROR_ILLEGAL_VALUE might have been appropriate, too. But I don't feel strongly either way.
Comment 8 Daniel Veditz [:dveditz] 2006-11-21 14:31:49 PST
Comment on attachment 246057 [details] [diff] [review]
fixed patch using JSDOUBLE_IS_FINITE.  patch for 1.8.1/1.9.

r=dveditz
Comment 9 Daniel Veditz [:dveditz] 2006-11-27 11:27:53 PST
Comment on attachment 246057 [details] [diff] [review]
fixed patch using JSDOUBLE_IS_FINITE.  patch for 1.8.1/1.9.

approved for 1.8 branch, a=dveditz for drivers
Comment 10 Daniel Veditz [:dveditz] 2006-11-27 11:28:04 PST
Comment on attachment 246173 [details] [diff] [review]
validate floats, patch for 1.8.0.x

approved for 1.8.0 branch, a=dveditz for drivers
Comment 11 Vladimir Vukicevic [:vlad] [:vladv] 2006-11-27 14:20:53 PST
In on 1.8.0.9, 1.8.1.1
Comment 12 Vladimir Vukicevic [:vlad] [:vladv] 2006-11-27 14:23:22 PST
In on trunk.
Comment 13 Martijn Wargers [:mwargers] (not working for Mozilla) 2006-11-29 14:21:43 PST
Verified fixed on trunk and branches, by using and testing builds before the patch went in and after the patch went in.
Comment 14 Alexander Sack 2006-12-20 08:40:59 PST
canvasrenderingcontext does not exist on 1.7 branch. Has this code been migrated from somewhere or is this complete new code?
Comment 15 Bob Clary [:bc:] 2009-04-24 10:57:11 PDT
crash test landed
http://hg.mozilla.org/mozilla-central/rev/b12e996f5d7a

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