Repository navigation
Crash when trying to create buffers with invalid base64. #3496
Description
Activity
/cc @trevnorris
I can reproduce this.
I can confirm that this also crashes on Amazon Linux AMI:
$ nvm use node Now using node v4.1.2 (npm v2.14.4) $ node > new Buffer("=" + new Array(10000).join("A"), "base64"); node: ../src/node_buffer.cc:224: v8::MaybeLocal<v8::Object> node::Buffer::New(v8::Isolate*, v8::Local<v8::String>, node::encoding): Assertion `(data) != (nullptr)' failed. Aborted $ cat /etc/issue Amazon Linux AMI release 2014.09 Kernel \r on an \mIt did not crash on OS X for me.
Just FYI, this also crashes on FreeBSD 10.1-RELEASE:
ggreer@calcium:~% node > new Buffer("=" + new Array(10000).join("A"), "base64"); Assertion failed: ((data) != (nullptr)), function New, file ../src/node_buffer.cc, line 225. zsh: abort (core dumped) node ggreer@calcium:~%That's using node.js v4.2.1 from ports.
@bnoordhuis - your PR seem to solve the crash which happens when failing to parse a string with output > 4096 (base64 input > 5641).
Just wondering - what is the desired behavior when the parsing of base64 string fails? The Buffer constructor currently returns an empty buffer which is indistinguishable from successful parsing of an empty input buffer.
Shouldn't it somehow indicate that the parsing has failed?It'd be nice to throw an exception if invalid base64 was passed into
new Buffer(), but I think that'd change the Buffer API. Currently, the Buffer constructor only throws RangeError, and only if you try to make a huge (2GB?) buffer.Right now though, I just want node to not crash.
- added a commit that references this issue
on Oct 23, 2015 Just wondering - what is the desired behavior when the parsing of base64 string fails?
@jhamhader The base64 decoder's behavior is backwards compatible (going back all the way to v0.1.x, IIRC.) It's allowed to pass in base64 data with trailing gunk and (some) interior gunk.
To wit, one of my first contributions to node was a better base64 decoder. One of my first bug fixes was for the regression it introduced because it was too strict. :-)
If you want to ensure that all input has been decoded, you would have to validate it yourself. Assuming valid base64 without whitespace, the decoded size should be
(size / 4) * 3, wheresizeis a multiple of 4.
I've been having this problem intermittently in production, and (with the help of @kans) managed to create a reproducible test case. This is on Ubuntu 15.04 using node.js v4.2.1 (built from source):
In the Buffer constructor (https://github.com/nodejs/node/blob/master/src/node_buffer.cc#L224), it looks like
StringBytes::Write()fails and returns zero. Thenrealloc()is called with a length of zero. On linux, this frees the memory and returns a null pointer. Then the null assertion fails and node crashes.realloc()behaves differently on OS X, so this won't crash on a mac.