Closed Bug 360293 Opened 14 years ago Closed 14 years ago

passing in non-numeric values to canvas functions causes crash inside cairo

Categories

(Core :: Canvas: 2D, defect)

x86
All
defect
Not set
normal

Tracking

()

VERIFIED FIXED

People

(Reporter: vlad, Assigned: vlad)

Details

(4 keywords, Whiteboard: [sg:critical?])

Attachments

(3 files, 1 obsolete file)

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.
Testcase
Assignee: nobody → vladimir
Status: NEW → ASSIGNED
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.
Flags: blocking1.9+
Flags: blocking1.8.1.1?
Flags: blocking1.8.0.9?
Attached patch add float value checks to canvas (obsolete) — Splinter Review
Add checks to make sure each float value passed in is finite; error out otherwise.
In a windows debug build it crashes when we pass an invalid pointer to realloc, that's potentially bad.
Flags: blocking1.8.1.1?
Flags: blocking1.8.1.1+
Flags: blocking1.8.0.9?
Flags: blocking1.8.0.9+
Whiteboard: [sg:critical?]
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.
Attachment #245246 - Attachment is obsolete: true
Attachment #246057 - Flags: review?(dveditz)
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.
Attachment #246173 - Flags: review?(dveditz)
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.
Attachment #246173 - Flags: review?(dveditz) → review+
Comment on attachment 246057 [details] [diff] [review]
fixed patch using JSDOUBLE_IS_FINITE.  patch for 1.8.1/1.9.

r=dveditz
Attachment #246057 - Flags: review?(dveditz) → review+
Attachment #246057 - Flags: approval1.8.1.1?
Attachment #246173 - Flags: approval1.8.0.9?
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
Attachment #246057 - Flags: approval1.8.1.1? → approval1.8.1.1+
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
Attachment #246173 - Flags: approval1.8.0.9? → approval1.8.0.9+
In on trunk.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Verified fixed on trunk and branches, by using and testing builds before the patch went in and after the patch went in.
Status: RESOLVED → VERIFIED
canvasrenderingcontext does not exist on 1.7 branch. Has this code been migrated from somewhere or is this complete new code?
Group: security
crash test landed
http://hg.mozilla.org/mozilla-central/rev/b12e996f5d7a
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.