Closed Bug 353365 Opened 15 years ago Closed 9 years ago

JSGetMethodOp signature is GC-hazard friendly

Categories

(Core :: JavaScript Engine, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED INVALID

People

(Reporter: igor, Unassigned)

References

(Depends on 1 open bug)

Details

[I mark the bug as restricted since it exposes the way to exploit GC hazards.]   

[This is a spin-off bug 353165 comment 3.]

The signature of JSGetMethodOp which is the type of JSXMLObjectOps.getMethod field, is asking for GC hazards as bug 352846 and bug 353165 show. Currently it is defined as:

typedef JSObject *
(* JS_DLL_CALLBACK JSGetMethodOp)(JSContext *cx, JSObject *obj, jsid id,
                                  jsval *vp);

In this form it is too easy for the caller to forget to root the result. In addition, each and every implementation of JSGetMethodOp has to take extra, often complicated measures to provide a temporary root for an object that will be returned to the caller just to be placed into another root. 

It would be nice to change the signature to something that is less GC-hazard friendly and that gives the callee an access to rooted storage of the caller.
No longer depends on: 353165
Depends on: 353165
I am not working on the bug.
Assignee: igor → general
This bugs this refers to have been fixed and made public.  Is there any reason to still have this one marked security sensitive?
The signature for this function is still the same as when this bug was opened per Brendan's comment:
http://mxr.mozilla.org/mozilla/source/js/src/jspubtd.h#574

I'm assuming it's still a GC hazard.  Brendan correct me if I'm wrong.  Also, any idea on what the severity rating should be?
There's no point in keeping this bug confidential.

I think this is low priority because we've fixed the one exploitable bug based on the botched API design. We should fix the API, but that can be done better in a bug such as bug 408416.

/be
Depends on: 408416
Thanks, Brendan.  Unhiding per comment 4.
Group: core-security
Seems to have been removed.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.