Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

isNaN(Buffer.prototype) crashes #1485

Closed
tolmasky opened this issue Apr 21, 2015 · 4 comments
Closed

isNaN(Buffer.prototype) crashes #1485

tolmasky opened this issue Apr 21, 2015 · 4 comments
Labels
buffer Issues and PRs related to the buffer subsystem. confirmed-bug Issues with confirmed bugs.

Comments

@tolmasky
Copy link
Contributor

isNaN(Buffer.prototype)
Assertion failed: ((obj_data) != (nullptr)), function StringSlice, file ../src/node_buffer.cc, line 237.
Abort trap: 6

@monsanto
Copy link
Contributor

Reproduced, PR in one sec.

@mscdex mscdex added confirmed-bug Issues with confirmed bugs. buffer Issues and PRs related to the buffer subsystem. labels Apr 21, 2015
@bnoordhuis
Copy link
Member

Even simpler test case: +Buffer.prototype. It's because of the custom .toString method on the prototype object...

@Fishrock123
Copy link
Contributor

isNaN(Buffer.prototype)

I am very curious as to how/why you ended up doing this though.

brendanashworth added a commit to brendanashworth/io.js that referenced this issue Jun 9, 2015
This commit adds assert() calls in JS, which prevents non-buffer
instances from triggering failed C++ assertions (killing the process).

They will now die with an AssertionError. Buffer.prototype.copy had to
be wrapped in JS to allow for the assertion, with a modification to the
arguments passed.

Fixes: nodejs#1485
trevnorris added a commit that referenced this issue Jun 25, 2015
If an object's prototype is munged it's possible to bypass the
instanceof check and cause the application to abort. Instead now use
HasInstance() to verify that the object is a Buffer, and throw if not.

This check will not work for JS only methods. So while the application
won't abort, it also won't throw.

In order to properly throw in all cases with toString() the JS
optimization of checking that length is zero has been removed. In its
place the native methods will now return early if a zero length string
is detected.

Ref: #1486
Ref: #1922
Fixes: #1485
PR-URL: #2012
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@trevnorris
Copy link
Contributor

Fixed by 1cd9eeb.

mscdex pushed a commit to mscdex/io.js that referenced this issue Jul 9, 2015
If an object's prototype is munged it's possible to bypass the
instanceof check and cause the application to abort. Instead now use
HasInstance() to verify that the object is a Buffer, and throw if not.

This check will not work for JS only methods. So while the application
won't abort, it also won't throw.

In order to properly throw in all cases with toString() the JS
optimization of checking that length is zero has been removed. In its
place the native methods will now return early if a zero length string
is detected.

Ref: nodejs#1486
Ref: nodejs#1922
Fixes: nodejs#1485
PR-URL: nodejs#2012
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
buffer Issues and PRs related to the buffer subsystem. confirmed-bug Issues with confirmed bugs.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants