Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

ObjectWrap/NODE_SET_PROTOTYPE_METHOD/node::SetPrototypeMethod is unsafe #6690

Closed
cscott opened this issue Dec 12, 2013 · 4 comments
Closed

Comments

@cscott
Copy link

cscott commented Dec 12, 2013

SetPrototypeMethod should set a Signature on the FunctionTemplate, to prevent issues like TryGhost/node-sqlite3#236

When used in conjunction with ObjectWrap, w/o a signature it is possible to pass bogus things as this, which could become a security hole.

@rvagg
Copy link
Member

rvagg commented Mar 6, 2014

@cscott do you think you could provide a basic pull-request for this just to demonstrate what you're suggesting? I think you're going to have to communicate this better to get any action here.

cscott added a commit to cscott/node that referenced this issue Mar 6, 2014
This prevents segfaults when a native method is reassigned to a different
object to corrupt `args.This()`.

Closes nodejs#6690.
@cscott
Copy link
Author

cscott commented Mar 6, 2014

@rvagg done (PR #7261, see above). Note that (1) I haven't even built this patch yet, (2) there are a number of minor issues with the commit message, (3) I'm not in the CLA, and (4) i haven't added a test case. But perhaps it will get the ball rolling?

@rvagg
Copy link
Member

rvagg commented Mar 6, 2014

cool, thanks @cscott, I think the core guys are super busy at the moment so probably don't have a whole lot of bandwidth to absorb this and it's not exactly a straightforward thing to absorb!

cscott added a commit to cscott/node that referenced this issue Mar 7, 2014
This prevents segfaults when a native method is reassigned to a
different object (which corrupts `args.This()`).  When unwrapping,
clients should use `args.Holder()` instead of `args.This()`.

Closes nodejs#6690.
@cscott
Copy link
Author

cscott commented Mar 7, 2014

Fixed the issues above in the latest patch at PR #7261 although there are more test cases that could be written.

cscott added a commit to cscott/node that referenced this issue Mar 14, 2014
This prevents segfaults when a native method is reassigned to a
different object (which corrupts `args.This()`).  When unwrapping,
clients should use `args.Holder()` instead of `args.This()`.

Closes nodejs#6690.
cscott added a commit to cscott/node that referenced this issue Mar 14, 2014
This prevents segfaults when a native method is reassigned to a
different object (which corrupts `args.This()`).  When unwrapping,
clients should use `args.Holder()` instead of `args.This()`.

Closes nodejs#6690.
cscott added a commit to cscott/node that referenced this issue Mar 17, 2014
This prevents segfaults when a native method is reassigned to a
different object (which corrupts `args.This()`).  When unwrapping,
clients should use `args.Holder()` instead of `args.This()`.

Closes nodejs#6690.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants