Fix out-of-bounds read in Buffer.prototype.toString()

Buffer.prototype.toString(encoding, start, end) on the njs engine
clamped start and end independently to the buffer length but never
enforced start <= end. With start > end the unsigned subtraction
end - start underflowed, the zero-length guard did not fire, and the
encoder read past the buffer.

Reject the start >= end case before computing the range, returning an
empty string to match Node.js semantics.

While here, add parallel missing functionality to QuickJS
for start and end arguments for Buffer.prototype.toString().

diff --git a/src/njs_buffer.c b/src/njs_buffer.c
index 886a865736d0d7053971f60be17da9fe45fdedd3..75b7192bbbc428b39473473bec4ea5c500ec112f 100644 (file)
--- a/src/njs_buffer.c
+++ b/src/njs_buffer.c
@@ -1975,14 +1975,14 @@ njs_buffer_prototype_to_string(njs_vm_t *vm, njs_value_t *args,
         return NJS_ERROR;
     }
 
-    str.start = &njs_typed_array_buffer(array)->u.u8[array->offset + start];
-    str.length = end - start;
-
-    if (njs_slow_path(str.length == 0)) {
+    if (njs_slow_path(start >= end)) {
         njs_set_empty_string(vm, retval);
         return NJS_OK;
     }
 
+    str.start = &njs_typed_array_buffer(array)->u.u8[array->offset + start];
+    str.length = end - start;
+
     return encoding->encode(vm, retval, &str);
 }
 

diff --git a/src/qjs_buffer.c b/src/qjs_buffer.c
index b870be91778e00baa1747eee75317138d7e1fc69..f51fc5a4d9214a05178bdea501a04efca07c2b7c 100644 (file)
--- a/src/qjs_buffer.c
+++ b/src/qjs_buffer.c
@@ -198,7 +198,7 @@ static const JSCFunctionListEntry qjs_buffer_proto[] = {
     JS_CFUNC_MAGIC_DEF("swap32", 0, qjs_buffer_prototype_swap, 4),
     JS_CFUNC_MAGIC_DEF("swap64", 0, qjs_buffer_prototype_swap, 8),
     JS_CFUNC_DEF("toJSON", 0, qjs_buffer_prototype_to_json),
-    JS_CFUNC_DEF("toString", 1, qjs_buffer_prototype_to_string),
+    JS_CFUNC_DEF("toString", 3, qjs_buffer_prototype_to_string),
     JS_CFUNC_DEF("write", 4, qjs_buffer_prototype_write),
     JS_CFUNC_MAGIC_DEF("writeInt8", 1, qjs_buffer_prototype_write_int,
                        qjs_buffer_magic(1, 1, 1)),
@@ -1419,6 +1419,7 @@ static JSValue
 qjs_buffer_prototype_to_string(JSContext *ctx, JSValueConst this_val,
     int argc, JSValueConst *argv)
 {
+    uint64_t                     start, end;
     JSValue                      ret;
     njs_str_t                    src, data;
     const qjs_buffer_encoding_t  *encoding;
@@ -1429,6 +1430,33 @@ qjs_buffer_prototype_to_string(JSContext *ctx, JSValueConst this_val,
                                  " object");
     }
 
+    start = 0;
+    end = src.length;
+
+    if (!JS_IsUndefined(argv[1])) {
+        if (JS_ToIndex(ctx, &start, argv[1])) {
+            return JS_EXCEPTION;
+        }
+
+        start = njs_min(start, src.length);
+    }
+
+    if (!JS_IsUndefined(argv[2])) {
+        if (JS_ToIndex(ctx, &end, argv[2])) {
+            return JS_EXCEPTION;
+        }
+
+        end = njs_min(end, src.length);
+    }
+
+    if (start >= end) {
+        src.length = 0;
+
+    } else {
+        src.start += start;
+        src.length = end - start;
+    }
+
     if (JS_IsUndefined(argv[0]) || src.length == 0) {
         return JS_NewStringLen(ctx, (char *) src.start, src.length);
     }

diff --git a/test/buffer.t.js b/test/buffer.t.js
index f4841b052429b1c52cb896d59d211c39edd0f47a..be74083a3ab737ed07790c71b1d8dcc002e86599 100644 (file)
--- a/test/buffer.t.js
+++ b/test/buffer.t.js
@@ -896,7 +896,8 @@ let toString_tsuite = {
     name: "Buffer.toString() tests",
     skip: () => (!has_buffer()),
     T: async (params) => {
-        let r = Buffer.from(params.value).toString(params.fmt);
+        let r = Buffer.from(params.value).toString(params.fmt, params.start,
+                                                   params.end);
 
         if (r.length !== params.expected.length) {
             throw Error(`unexpected "${r}" length ${r.length} != ${params.expected.length}`);
@@ -921,6 +922,11 @@ let toString_tsuite = {
         { value: "ABCD", fmt: "base64", expected: 'QUJDRA==' },
         { value: "ABCD", fmt: "base64url", expected: 'QUJDRA' },
         { value: '', fmt: "utf-128", exception: 'TypeError: "utf-128" encoding is not supported' },
+        { value: "hello", fmt: "utf-8", start: 1, end: 4, expected: 'ell' },
+        { value: "hello", fmt: "utf-8", start: 3, end: 3, expected: '' },
+        { value: "hello", fmt: "utf-8", start: 10, end: 5, expected: '' },
+        { value: "hello", fmt: "utf-8", start: 4, end: 2, expected: '' },
+        { value: "hello", fmt: "hex", start: 4, end: 2, expected: '' },
 ]};