From 06c100c9bfeef0d0c1d138153321491f7884cef3 Mon Sep 17 00:00:00 2001 From: Charlie Gordon Date: Sun, 3 Mar 2024 14:05:40 +0100 Subject: [PATCH] Prevent UB on memcpy and floating point conversions - add `memcpy_no_ub` that accepts null pointers for 0 count - prevent 0 length allocation in `js_worker_postMessage` - use safer test for `int` value in `JS_NewFloat64`, `JS_ToArrayLengthFree` and `js_typed_array_indexOf` --- cutils.c | 2 +- cutils.h | 7 +++++++ libbf.c | 2 +- quickjs.c | 13 ++++++++----- quickjs.h | 20 +++++++++----------- 5 files changed, 26 insertions(+), 18 deletions(-) diff --git a/cutils.c b/cutils.c index b4960f9..c0aacef 100644 --- a/cutils.c +++ b/cutils.c @@ -140,7 +140,7 @@ int dbuf_put(DynBuf *s, const uint8_t *data, size_t len) if (dbuf_realloc(s, s->size + len)) return -1; } - memcpy(s->buf + s->size, data, len); + memcpy_no_ub(s->buf + s->size, data, len); s->size += len; return 0; } diff --git a/cutils.h b/cutils.h index ff2d3fb..11246e3 100644 --- a/cutils.h +++ b/cutils.h @@ -26,6 +26,7 @@ #define CUTILS_H #include +#include #include #define likely(x) __builtin_expect(!!(x), 1) @@ -64,6 +65,12 @@ char *pstrcat(char *buf, int buf_size, const char *s); int strstart(const char *str, const char *val, const char **ptr); int has_suffix(const char *str, const char *suffix); +/* Prevent UB when n == 0 and (src == NULL or dest == NULL) */ +static inline void memcpy_no_ub(void *dest, const void *src, size_t n) { + if (n) + memcpy(dest, src, n); +} + static inline int max_int(int a, int b) { if (a > b) diff --git a/libbf.c b/libbf.c index b3ed209..dee394a 100644 --- a/libbf.c +++ b/libbf.c @@ -309,7 +309,7 @@ int bf_set(bf_t *r, const bf_t *a) } r->sign = a->sign; r->expn = a->expn; - memcpy(r->tab, a->tab, a->len * sizeof(limb_t)); + memcpy_no_ub(r->tab, a->tab, a->len * sizeof(limb_t)); return 0; } diff --git a/quickjs.c b/quickjs.c index 6e1bb36..ebf45a9 100644 --- a/quickjs.c +++ b/quickjs.c @@ -11078,6 +11078,8 @@ static __exception int JS_ToArrayLengthFree(JSContext *ctx, uint32_t *plen, if (JS_TAG_IS_FLOAT64(tag)) { double d; d = JS_VALUE_GET_FLOAT64(val); + if (!(d >= 0 && d <= UINT32_MAX)) + goto fail; len = (uint32_t)d; if (len != d) goto fail; @@ -33388,8 +33390,8 @@ static JSValue js_create_function(JSContext *ctx, JSFunctionDef *fd) } } else { b->vardefs = (void *)((uint8_t*)b + vardefs_offset); - memcpy(b->vardefs, fd->args, fd->arg_count * sizeof(fd->args[0])); - memcpy(b->vardefs + fd->arg_count, fd->vars, fd->var_count * sizeof(fd->vars[0])); + memcpy_no_ub(b->vardefs, fd->args, fd->arg_count * sizeof(fd->args[0])); + memcpy_no_ub(b->vardefs + fd->arg_count, fd->vars, fd->var_count * sizeof(fd->vars[0])); } b->var_count = fd->var_count; b->arg_count = fd->arg_count; @@ -53997,9 +53999,10 @@ static JSValue js_typed_array_indexOf(JSContext *ctx, JSValueConst this_val, } else if (tag == JS_TAG_FLOAT64) { d = JS_VALUE_GET_FLOAT64(argv[0]); - // XXX: should fix UB - v64 = d; - is_int = (v64 == d); + if (d >= INT64_MIN && d < 0x1p63) { + v64 = d; + is_int = (v64 == d); + } } else if (tag == JS_TAG_BIG_INT) { JSBigFloat *p1 = JS_VALUE_GET_PTR(argv[0]); diff --git a/quickjs.h b/quickjs.h index 003af2f..a951e67 100644 --- a/quickjs.h +++ b/quickjs.h @@ -550,23 +550,21 @@ JSValue JS_NewBigUint64(JSContext *ctx, uint64_t v); static js_force_inline JSValue JS_NewFloat64(JSContext *ctx, double d) { - JSValue v; int32_t val; union { double d; uint64_t u; } u, t; - u.d = d; - val = (int32_t)d; - t.d = val; - /* -0 cannot be represented as integer, so we compare the bit - representation */ - if (u.u == t.u) { - v = JS_MKVAL(JS_TAG_INT, val); - } else { - v = __JS_NewFloat64(ctx, d); + if (d >= INT32_MIN && d <= INT32_MAX) { + u.d = d; + val = (int32_t)d; + t.d = val; + /* -0 cannot be represented as integer, so we compare the bit + representation */ + if (u.u == t.u) + return JS_MKVAL(JS_TAG_INT, val); } - return v; + return __JS_NewFloat64(ctx, d); } static inline JS_BOOL JS_IsNumber(JSValueConst v)