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

VERIFIED FIXED

Status

()

Core
Canvas: 2D
VERIFIED FIXED
11 years ago
8 years ago

People

(Reporter: vlad, Assigned: vlad)

Tracking

(4 keywords)

Trunk
x86
All
crash, testcase, verified1.8.0.9, verified1.8.1.1
Points:
---
Bug Flags:
blocking1.9 +
blocking1.8.1.1 +
blocking1.8.0.9 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:critical?])

Attachments

(3 attachments, 1 obsolete attachment)

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.
Created attachment 245245 [details]
testcase (crashes once you click crash button)

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?
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.
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?]
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.
Attachment #245246 - Attachment is obsolete: true
Attachment #246057 - Flags: review?(dveditz)
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.
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+

Updated

11 years ago
Attachment #246057 - Flags: approval1.8.1.1?

Updated

11 years ago
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 1.8.0.9, 1.8.1.1
Keywords: fixed1.8.0.9, fixed1.8.1.1
In on trunk.
Status: ASSIGNED → RESOLVED
Last Resolved: 11 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
Keywords: fixed1.8.0.9, fixed1.8.1.1 → crash, testcase, verified1.8.0.9, verified1.8.1.1

Comment 14

11 years ago
canvasrenderingcontext does not exist on 1.7 branch. Has this code been migrated from somewhere or is this complete new code?
Group: security

Comment 15

8 years ago
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.