Skip to content

Commit 19f3ac9

Browse files
addaleaxMylesBorins
authored andcommitted
src: add Malloc() size param + overflow detection
Adds an optional second parameter to `node::Malloc()` and an optional third parameter to `node::Realloc()` giving the size/number of items to be allocated, in the style of `calloc(3)`. Use a proper overflow check using division; the previous `CHECK_GE(n * size, n);` would not detect all cases of overflow (e.g. `size == SIZE_MAX / 2 && n == 3`). Backport-PR-URL: #16587 PR-URL: #8482 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
1 parent 92b13e4 commit 19f3ac9

File tree

4 files changed

+25
-17
lines changed

4 files changed

+25
-17
lines changed

src/node_crypto.cc

+2-3
Original file line numberDiff line numberDiff line change
@@ -5826,11 +5826,10 @@ void GetCurves(const FunctionCallbackInfo<Value>& args) {
58265826
const size_t num_curves = EC_get_builtin_curves(nullptr, 0);
58275827
Local<Array> arr = Array::New(env->isolate(), num_curves);
58285828
EC_builtin_curve* curves;
5829-
size_t alloc_size;
58305829

58315830
if (num_curves) {
5832-
alloc_size = sizeof(*curves) * num_curves;
5833-
curves = static_cast<EC_builtin_curve*>(node::Malloc(alloc_size));
5831+
curves = static_cast<EC_builtin_curve*>(node::Malloc(sizeof(*curves),
5832+
num_curves));
58345833

58355834
CHECK_NE(curves, nullptr);
58365835

src/string_bytes.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ class ExternString: public ResourceType {
5454
return scope.Escape(String::Empty(isolate));
5555

5656
TypeName* new_data =
57-
static_cast<TypeName*>(node::Malloc(length * sizeof(*new_data)));
57+
static_cast<TypeName*>(node::Malloc(length, sizeof(*new_data)));
5858
if (new_data == nullptr) {
5959
return Local<String>();
6060
}

src/util-inl.h

+18-6
Original file line numberDiff line numberDiff line change
@@ -320,31 +320,43 @@ bool StringEqualNoCaseN(const char* a, const char* b, size_t length) {
320320
return true;
321321
}
322322

323+
inline size_t MultiplyWithOverflowCheck(size_t a, size_t b) {
324+
size_t ret = a * b;
325+
if (a != 0)
326+
CHECK_EQ(b, ret / a);
327+
328+
return ret;
329+
}
330+
323331
// These should be used in our code as opposed to the native
324332
// versions as they abstract out some platform and or
325333
// compiler version specific functionality.
326334
// malloc(0) and realloc(ptr, 0) have implementation-defined behavior in
327335
// that the standard allows them to either return a unique pointer or a
328336
// nullptr for zero-sized allocation requests. Normalize by always using
329337
// a nullptr.
330-
void* Realloc(void* pointer, size_t size) {
331-
if (size == 0) {
338+
void* Realloc(void* pointer, size_t n, size_t size) {
339+
size_t full_size = MultiplyWithOverflowCheck(size, n);
340+
341+
if (full_size == 0) {
332342
free(pointer);
333343
return nullptr;
334344
}
335-
return realloc(pointer, size);
345+
346+
return realloc(pointer, full_size);
336347
}
337348

338349
// As per spec realloc behaves like malloc if passed nullptr.
339-
void* Malloc(size_t size) {
350+
void* Malloc(size_t n, size_t size) {
351+
if (n == 0) n = 1;
340352
if (size == 0) size = 1;
341-
return Realloc(nullptr, size);
353+
return Realloc(nullptr, n, size);
342354
}
343355

344356
void* Calloc(size_t n, size_t size) {
345357
if (n == 0) n = 1;
346358
if (size == 0) size = 1;
347-
CHECK_GE(n * size, n); // Overflow guard.
359+
MultiplyWithOverflowCheck(size, n);
348360
return calloc(n, size);
349361
}
350362

src/util.h

+4-7
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,9 @@ namespace node {
3131
// that the standard allows them to either return a unique pointer or a
3232
// nullptr for zero-sized allocation requests. Normalize by always using
3333
// a nullptr.
34-
inline void* Realloc(void* pointer, size_t size);
35-
inline void* Malloc(size_t size);
36-
inline void* Calloc(size_t n, size_t size);
34+
inline void* Realloc(void* pointer, size_t n, size_t size = 1);
35+
inline void* Malloc(size_t n, size_t size = 1);
36+
inline void* Calloc(size_t n, size_t size = 1);
3737

3838
#ifdef __GNUC__
3939
#define NO_RETURN __attribute__((noreturn))
@@ -294,10 +294,7 @@ class MaybeStackBuffer {
294294
if (storage <= kStackStorageSize) {
295295
buf_ = buf_st_;
296296
} else {
297-
// Guard against overflow.
298-
CHECK_LE(storage, sizeof(T) * storage);
299-
300-
buf_ = static_cast<T*>(Malloc(sizeof(T) * storage));
297+
buf_ = static_cast<T*>(Malloc(sizeof(T), storage));
301298
CHECK_NE(buf_, nullptr);
302299
}
303300

0 commit comments

Comments
 (0)