Unchecked JS_GetPrivate calls in DOMJSClass_toString

VERIFIED FIXED

Status

()

Core
DOM: Core & HTML
--
critical
VERIFIED FIXED
13 years ago
10 years ago

People

(Reporter: shaver, Assigned: sicking)

Tracking

({fixed-aviary1.0.3, fixed1.7.7})

Trunk
fixed-aviary1.0.3, fixed1.7.7
Points:
---
Bug Flags:
blocking-aviary1.5 +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 1 obsolete attachment)

As in bug 290162, we trust too much that we will only get objects of the
appropriate class as |this| for this method.  Crashing test case, due to deref
of bogus pointer:

javascript:a=new Number(0x12345678);a.toString=Node.toString;alert(a)

(Sadly, perhaps, I can't use Node.toString.call, because it doesn't exist,
though Node.toString has a typeof "function".)

Not sure if the obvious fix (JS_GetInstancePrivate against sDOMJSClass) is
correct here, because I don't quite have all of the delegation model wired up in
my head at this hour.
nominating to get on the radar
Flags: blocking1.8b2?
Flags: blocking1.7.7?
Flags: blocking-aviary1.1+
Flags: blocking-aviary1.0.3?
Status: NEW → ASSIGNED
Assignee: jst → bugmail
Status: ASSIGNED → NEW
I changed the assertion to a warning since this can happen without error on our
side.
Status: NEW → ASSIGNED
Attachment #180750 - Flags: superreview?(shaver)
Attachment #180750 - Flags: review?(shaver)
Comment on attachment 180750 [details] [diff] [review]
patch to fix

In a more perfect world, we'd throw something like INVALID_ARG, but this will
more than do for 1.0.3.  r+sr=shaver, requesting approval for branch and trunk.
Attachment #180750 - Flags: superreview?(shaver)
Attachment #180750 - Flags: superreview+
Attachment #180750 - Flags: review?(shaver)
Attachment #180750 - Flags: review+
Attachment #180750 - Flags: approval1.8b2?
Attachment #180750 - Flags: approval1.7.7?
Attachment #180750 - Flags: approval-aviary1.0.3?

Comment 5

13 years ago
Comment on attachment 180750 [details] [diff] [review]
patch to fix

a=asa
Attachment #180750 - Flags: approval1.8b2? → approval1.8b2+

Comment 6

13 years ago
Comment on attachment 180750 [details] [diff] [review]
patch to fix

a=chase for branches
Attachment #180750 - Flags: approval1.7.7?
Attachment #180750 - Flags: approval1.7.7+
Attachment #180750 - Flags: approval-aviary1.0.3?
Attachment #180750 - Flags: approval-aviary1.0.3+
jst thought peterv may have written this native method; cc'ing him so he can
repent and sin no more ;-).

/be
Created attachment 180751 [details] [diff] [review]
fix
Attachment #180750 - Attachment is obsolete: true
Attachment #180751 - Flags: superreview+
Attachment #180751 - Flags: review+
Attachment #180751 - Flags: approval1.8b2+
Attachment #180751 - Flags: approval1.7.7+
Attachment #180751 - Flags: approval-aviary1.0.3+
Fixed everywhere.

/be
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
tested with the 2005041422-1.0.3 ffox build on linux fc3, I no longer crash with
the test case in comment 0 (entering it in the Location field).
looks good on Fx trunk build 2005-04-15-07-trunk

No crash with the following error in js console:

Error: uncaught exception: [Exception... "Could not convert JavaScript argument
arg 0 [nsIDOMWindowInternal.alert]"  nsresult: "0x80570009
(NS_ERROR_XPC_BAD_CONVERT_JS)"  location: "JS frame :: javascript:a=new
Number(0x12345678);a.toString=Node.toString;alert(a) :: <TOP_LEVEL> :: line 1" 
data: no]
Status: RESOLVED → VERIFIED

Updated

13 years ago
Flags: blocking1.8b2?

Updated

13 years ago
Flags: blocking1.7.7?
Flags: blocking-aviary1.0.3?
Keywords: fixed-aviary1.0.3, fixed1.7.7
Group: security

Updated

10 years ago
Component: DOM: Core → DOM: Core & HTML
QA Contact: ian → general
You need to log in before you can comment on or make changes to this bug.