Skip to content

Commit

Permalink
src: unify implementations of Utf8Value etc.
Browse files Browse the repository at this point in the history 
Unify the common code of `Utf8Value`, `TwoByteValue`, `BufferValue`
and `StringBytes::InlineDecoder` into one class. Always make the
result zero-terminated for the first three.

This fixes two problems in passing:

* When the conversion of the input value to String fails,
  make the buffer zero-terminated anyway. Previously, this would
  have resulted in possibly reading uninitialized data in multiple
  places in the code. An instance of that problem can be reproduced
  by running e.g.
  `valgrind node -e 'net.isIP({ toString() { throw Error() } })'`.
* Previously, `BufferValue` copied one byte too much from the source,
  possibly resulting in an out-of-bounds memory access.
  This can be reproduced by running e.g.
  `valgrind node -e \
    'fs.openSync(Buffer.from("node".repeat(8192)), "r")'`.

Further minor changes:
* This lifts the `out()` method of `StringBytes::InlineDecoder`
  to the common class so that it can be used when using the
  overloaded `operator*` does not seem appropiate.
* Hopefully clearer variable names.
* Add checks to make sure the length of the data does not exceed
  the allocated storage size, including the possible null terminator.

PR-URL: nodejs#6357
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Trevor Norris <[email protected]>
  • Loading branch information
@addaleax
addaleax committed Apr 29, 2016
1 parent c8e137b commit 44a4032
Show file tree
Hide file tree
Showing 3 changed files with 123 additions and 119 deletions.
44 changes: 15 additions & 29 deletions src/string_bytes.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,22 +7,14 @@
#include "node.h"
#include "env.h"
#include "env-inl.h"
#include "util.h"

namespace node {

class StringBytes {
public:
class InlineDecoder {
class InlineDecoder : public MaybeStackBuffer<char> {
public:
InlineDecoder() : out_(nullptr) {
}

~InlineDecoder() {
if (out_ != out_st_)
delete[] out_;
out_ = nullptr;
}

inline bool Decode(Environment* env,
v8::Local<v8::String> string,
v8::Local<v8::Value> encoding,
Expand All @@ -33,28 +25,22 @@ class StringBytes {
return false;
}

size_t buflen = StringBytes::StorageSize(env->isolate(), string, enc);
if (buflen > sizeof(out_st_))
out_ = new char[buflen];
else
out_ = out_st_;
size_ = StringBytes::Write(env->isolate(),
out_,
buflen,
string,
enc);
const size_t storage = StringBytes::StorageSize(env->isolate(),
string,
enc);
AllocateSufficientStorage(storage);
const size_t length = StringBytes::Write(env->isolate(),
out(),
storage,
string,
enc);

// No zero terminator is included when using this method.
SetLength(length);
return true;
}

inline const char* out() const { return out_; }
inline size_t size() const { return size_; }

private:
static const int kStorageSize = 1024;

char out_st_[kStorageSize];
char* out_;
size_t size_;
inline size_t size() const { return length(); }
};

// Does the string match the encoding? Quick but non-exhaustive.
Expand Down
75 changes: 34 additions & 41 deletions src/util.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,76 +10,69 @@ using v8::Local;
using v8::String;
using v8::Value;

static int MakeUtf8String(Isolate* isolate,
Local<Value> value,
char** dst,
const size_t size) {
template <typename T>
static void MakeUtf8String(Isolate* isolate,
Local<Value> value,
T* target) {
Local<String> string = value->ToString(isolate);
if (string.IsEmpty())
return 0;
size_t len = StringBytes::StorageSize(isolate, string, UTF8) + 1;
if (len > size) {
*dst = static_cast<char*>(malloc(len));
CHECK_NE(*dst, nullptr);
}
return;

const size_t storage = StringBytes::StorageSize(isolate, string, UTF8) + 1;
target->AllocateSufficientStorage(storage);
const int flags =
String::NO_NULL_TERMINATION | String::REPLACE_INVALID_UTF8;
const int length = string->WriteUtf8(*dst, len, 0, flags);
(*dst)[length] = '\0';
return length;
const int length = string->WriteUtf8(target->out(), storage, 0, flags);
target->SetLengthAndZeroTerminate(length);
}

Utf8Value::Utf8Value(Isolate* isolate, Local<Value> value)
: length_(0), str_(str_st_) {
Utf8Value::Utf8Value(Isolate* isolate, Local<Value> value) {
if (value.IsEmpty())
return;
length_ = MakeUtf8String(isolate, value, &str_, sizeof(str_st_));

MakeUtf8String(isolate, value, this);
}


TwoByteValue::TwoByteValue(Isolate* isolate, Local<Value> value)
: length_(0), str_(str_st_) {
if (value.IsEmpty())
TwoByteValue::TwoByteValue(Isolate* isolate, Local<Value> value) {
if (value.IsEmpty()) {
return;
}

Local<String> string = value->ToString(isolate);
if (string.IsEmpty())
return;

// Allocate enough space to include the null terminator
size_t len =
StringBytes::StorageSize(isolate, string, UCS2) +
sizeof(uint16_t);
if (len > sizeof(str_st_)) {
str_ = static_cast<uint16_t*>(malloc(len));
CHECK_NE(str_, nullptr);
}
const size_t storage = string->Length() + 1;
AllocateSufficientStorage(storage);

const int flags =
String::NO_NULL_TERMINATION | String::REPLACE_INVALID_UTF8;
length_ = string->Write(str_, 0, len, flags);
str_[length_] = '\0';
const int length = string->Write(out(), 0, storage, flags);
SetLengthAndZeroTerminate(length);
}

BufferValue::BufferValue(Isolate* isolate, Local<Value> value)
: str_(str_st_), fail_(true) {
BufferValue::BufferValue(Isolate* isolate, Local<Value> value) {
// Slightly different take on Utf8Value. If value is a String,
// it will return a Utf8 encoded string. If value is a Buffer,
// it will copy the data out of the Buffer as is.
if (value.IsEmpty())
if (value.IsEmpty()) {
// Dereferencing this object will return nullptr.
Invalidate();
return;
}

if (value->IsString()) {
MakeUtf8String(isolate, value, &str_, sizeof(str_st_));
fail_ = false;
MakeUtf8String(isolate, value, this);
} else if (Buffer::HasInstance(value)) {
size_t len = Buffer::Length(value) + 1;
if (len > sizeof(str_st_)) {
str_ = static_cast<char*>(malloc(len));
CHECK_NE(str_, nullptr);
}
memcpy(str_, Buffer::Data(value), len);
str_[len - 1] = '\0';
fail_ = false;
const size_t len = Buffer::Length(value);
// Leave place for the terminating '\0' byte.
AllocateSufficientStorage(len + 1);
memcpy(out(), Buffer::Data(value), len);
SetLengthAndZeroTerminate(len);
} else {
Invalidate();
}
}

Expand Down
123 changes: 74 additions & 49 deletions src/util.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -178,77 +178,102 @@ inline TypeName* Unwrap(v8::Local<v8::Object> object);

inline void SwapBytes(uint16_t* dst, const uint16_t* src, size_t buflen);

class Utf8Value {
// Allocates an array of member type T. For up to kStackStorageSize items,
// the stack is used, otherwise malloc().
template <typename T, size_t kStackStorageSize = 1024>
class MaybeStackBuffer {
public:
explicit Utf8Value(v8::Isolate* isolate, v8::Local<v8::Value> value);
const T* out() const {
return buf_;
}

~Utf8Value() {
if (str_ != str_st_)
free(str_);
T* out() {
return buf_;
}

char* operator*() {
return str_;
};
// operator* for compatibility with `v8::String::(Utf8)Value`
T* operator*() {
return buf_;
}

const char* operator*() const {
return str_;
};
const T* operator*() const {
return buf_;
}

size_t length() const {
return length_;
};

private:
size_t length_;
char* str_;
char str_st_[1024];
};
}

class TwoByteValue {
public:
explicit TwoByteValue(v8::Isolate* isolate, v8::Local<v8::Value> value);
// Call to make sure enough space for `storage` entries is available.
// There can only be 1 call to AllocateSufficientStorage or Invalidate
// per instance.
void AllocateSufficientStorage(size_t storage) {
if (storage <= kStackStorageSize) {
buf_ = buf_st_;
} else {
// Guard against overflow.
CHECK_LE(storage, sizeof(T) * storage);

buf_ = static_cast<T*>(malloc(sizeof(T) * storage));
CHECK_NE(buf_, nullptr);
}

// Remember how much was allocated to check against that in SetLength().
length_ = storage;
}

~TwoByteValue() {
if (str_ != str_st_)
free(str_);
void SetLength(size_t length) {
// length_ stores how much memory was allocated.
CHECK_LE(length, length_);
length_ = length;
}

uint16_t* operator*() {
return str_;
};
void SetLengthAndZeroTerminate(size_t length) {
// length_ stores how much memory was allocated.
CHECK_LE(length + 1, length_);
SetLength(length);

const uint16_t* operator*() const {
return str_;
};
// T() is 0 for integer types, nullptr for pointers, etc.
buf_[length] = T();
}

size_t length() const {
return length_;
};
// Make derefencing this object return nullptr.
// Calling this is mutually exclusive with calling
// AllocateSufficientStorage.
void Invalidate() {
CHECK_EQ(buf_, buf_st_);
length_ = 0;
buf_ = nullptr;
}

MaybeStackBuffer() : length_(0), buf_(buf_st_) {
// Default to a zero-length, null-terminated buffer.
buf_[0] = T();
}

~MaybeStackBuffer() {
if (buf_ != buf_st_)
free(buf_);
}
private:
size_t length_;
uint16_t* str_;
uint16_t str_st_[1024];
T* buf_;
T buf_st_[kStackStorageSize];
};

class BufferValue {
class Utf8Value : public MaybeStackBuffer<char> {
public:
explicit BufferValue(v8::Isolate* isolate, v8::Local<v8::Value> value);

~BufferValue() {
if (str_ != str_st_)
free(str_);
}
explicit Utf8Value(v8::Isolate* isolate, v8::Local<v8::Value> value);
};

const char* operator*() const {
return fail_ ? nullptr : str_;
};
class TwoByteValue : public MaybeStackBuffer<uint16_t> {
public:
explicit TwoByteValue(v8::Isolate* isolate, v8::Local<v8::Value> value);
};

private:
char* str_;
char str_st_[1024];
bool fail_;
class BufferValue : public MaybeStackBuffer<char> {
public:
explicit BufferValue(v8::Isolate* isolate, v8::Local<v8::Value> value);
};

} // namespace node
Expand Down

0 comments on commit 44a4032

Please sign in to comment.