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.

Sign up for GitHub

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

Jump to bottom

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
@monsanto monsanto mentioned this issue Apr 21, 2015
Closed
@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
@brendanashworth
buffer: prevent failed assertions
ca408dc 
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
@brendanashworth brendanashworth mentioned this issue Jun 9, 2015
Closed
trevnorris added a commit that referenced this issue Jun 25, 2015
@trevnorris
buffer: prevent abort on bad proto
1cd9eeb 
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 <[email protected]>
@trevnorris
Copy link
Contributor

Fixed by 1cd9eeb.

mscdex pushed a commit to mscdex/io.js that referenced this issue Jul 9, 2015
@trevnorris @mscdex
buffer: prevent abort on bad proto
40c334d 
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 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
buffer Issues and PRs related to the buffer subsystem. confirmed-bug Issues with confirmed bugs.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

buffer: fix segfaults when this is not a Buffer.
6 participants
@tolmasky @mscdex @monsanto @bnoordhuis @trevnorris @Fishrock123