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

Fix uninitialized memory in buffers #4706

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 1 addition & 13 deletions lib/buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

const binding = process.binding('buffer');
const internalUtil = require('internal/util');
const bindingObj = {};

exports.Buffer = Buffer;
exports.SlowBuffer = SlowBuffer;
Expand All @@ -15,9 +14,7 @@ Buffer.poolSize = 8 * 1024;
var poolSize, poolOffset, allocPool;


binding.setupBufferJS(Buffer.prototype, bindingObj);
const flags = bindingObj.flags;
const kNoZeroFill = 0;
binding.setupBufferJS(Buffer.prototype);

function createBuffer(size) {
const ui8 = new Uint8Array(size);
Expand All @@ -27,8 +24,6 @@ function createBuffer(size) {

function createPool() {
poolSize = Buffer.poolSize;
if (poolSize > 0)
flags[kNoZeroFill] = 1;
allocPool = createBuffer(poolSize);
poolOffset = 0;
}
Expand Down Expand Up @@ -74,8 +69,6 @@ Object.setPrototypeOf(Buffer, Uint8Array);
function SlowBuffer(length) {
if (+length != length)
length = 0;
if (length > 0)
flags[kNoZeroFill] = 1;
return createBuffer(+length);
}

Expand All @@ -95,11 +88,6 @@ function allocate(size) {
alignPool();
return b;
} else {
// Even though this is checked above, the conditional is a safety net and
// sanity check to prevent any subsequent typed array allocation from not
// being zero filled.
if (size > 0)
flags[kNoZeroFill] = 1;
return createBuffer(size);
}
}
Expand Down
26 changes: 0 additions & 26 deletions src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -145,27 +145,6 @@ inline void Environment::TickInfo::set_last_threw(bool value) {
last_threw_ = value;
}

inline Environment::ArrayBufferAllocatorInfo::ArrayBufferAllocatorInfo() {
for (int i = 0; i < kFieldsCount; ++i)
fields_[i] = 0;
}

inline uint32_t* Environment::ArrayBufferAllocatorInfo::fields() {
return fields_;
}

inline int Environment::ArrayBufferAllocatorInfo::fields_count() const {
return kFieldsCount;
}

inline bool Environment::ArrayBufferAllocatorInfo::no_zero_fill() const {
return fields_[kNoZeroFill] != 0;
}

inline void Environment::ArrayBufferAllocatorInfo::reset_fill_flag() {
fields_[kNoZeroFill] = 0;
}

inline Environment* Environment::New(v8::Local<v8::Context> context,
uv_loop_t* loop) {
Environment* env = new Environment(context, loop);
Expand Down Expand Up @@ -331,11 +310,6 @@ inline Environment::TickInfo* Environment::tick_info() {
return &tick_info_;
}

inline Environment::ArrayBufferAllocatorInfo*
Environment::array_buffer_allocator_info() {
return &array_buffer_allocator_info_;
}

inline uint64_t Environment::timer_base() const {
return timer_base_;
}
Expand Down
23 changes: 0 additions & 23 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -354,27 +354,6 @@ class Environment {
DISALLOW_COPY_AND_ASSIGN(TickInfo);
};

class ArrayBufferAllocatorInfo {
public:
inline uint32_t* fields();
inline int fields_count() const;
inline bool no_zero_fill() const;
inline void reset_fill_flag();

private:
friend class Environment; // So we can call the constructor.
inline ArrayBufferAllocatorInfo();

enum Fields {
kNoZeroFill,
kFieldsCount
};

uint32_t fields_[kFieldsCount];

DISALLOW_COPY_AND_ASSIGN(ArrayBufferAllocatorInfo);
};

typedef void (*HandleCleanupCb)(Environment* env,
uv_handle_t* handle,
void* arg);
Expand Down Expand Up @@ -437,7 +416,6 @@ class Environment {
inline AsyncHooks* async_hooks();
inline DomainFlag* domain_flag();
inline TickInfo* tick_info();
inline ArrayBufferAllocatorInfo* array_buffer_allocator_info();
inline uint64_t timer_base() const;

static inline Environment* from_cares_timer_handle(uv_timer_t* handle);
Expand Down Expand Up @@ -544,7 +522,6 @@ class Environment {
AsyncHooks async_hooks_;
DomainFlag domain_flag_;
TickInfo tick_info_;
ArrayBufferAllocatorInfo array_buffer_allocator_info_;
const uint64_t timer_base_;
uv_timer_t cares_timer_handle_;
ares_channel cares_channel_;
Expand Down
8 changes: 0 additions & 8 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -943,14 +943,6 @@ Local<Value> WinapiErrnoException(Isolate* isolate,
}
#endif


void* ArrayBufferAllocator::Allocate(size_t size) {
if (env_ == nullptr || !env_->array_buffer_allocator_info()->no_zero_fill())
return calloc(size, 1);
env_->array_buffer_allocator_info()->reset_fill_flag();
return malloc(size);
}

static bool DomainHasErrorHandler(const Environment* env,
const Local<Object>& domain) {
HandleScope scope(env->isolate());
Expand Down
13 changes: 0 additions & 13 deletions src/node_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1055,19 +1055,6 @@ void SetupBufferJS(const FunctionCallbackInfo<Value>& args) {
env->SetMethod(proto, "utf8Write", Utf8Write);

env->SetMethod(proto, "copy", Copy);

CHECK(args[1]->IsObject());
Local<Object> bObj = args[1].As<Object>();

uint32_t* const fields = env->array_buffer_allocator_info()->fields();
uint32_t const fields_count =
env->array_buffer_allocator_info()->fields_count();

Local<ArrayBuffer> array_buffer =
ArrayBuffer::New(env->isolate(), fields, sizeof(*fields) * fields_count);

bObj->Set(String::NewFromUtf8(env->isolate(), "flags"),
Uint32Array::New(array_buffer, 0, fields_count));
}


Expand Down
2 changes: 1 addition & 1 deletion src/node_internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ class ArrayBufferAllocator : public v8::ArrayBuffer::Allocator {

inline void set_env(Environment* env) { env_ = env; }

virtual void* Allocate(size_t size); // Defined in src/node.cc
virtual void* Allocate(size_t size) { return calloc(size, 1); }
virtual void* AllocateUninitialized(size_t size) { return malloc(size); }
virtual void Free(void* data, size_t) { free(data); }

Expand Down
11 changes: 11 additions & 0 deletions test/parallel/test-buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,17 @@ var cntr = 0;

var b = Buffer(1024); // safe constructor

function isZeroFilled(b) {
for (var i = 0; i < b.length; i++) {
if (b[i] != 0) {
return false;
}
}
return true;
}

assert.ok(isZeroFilled(b), 'Buffer is not zero-filled.');

console.log('b.length == %d', b.length);
assert.strictEqual(1024, b.length);

Expand Down