Add optional per-chain byte cap in chained buffers

Previously, njs_chb_t accumulated bytes unbounded through the memory
pool until the final string boundary (njs_string_create_chb()) enforced
the NJS_STRING_MAX_LENGTH limit.  For string-producing chains this let
the pool grow to tens of GiB before the final alloc was refused, which
on adversarial inputs (such as String.prototype.replace with a regex
that matches the full string and a replacement containing thousands of
$N references) led to process OOM instead of a catchable RangeError.

The fix is to add two fields to njs_chb_t: total_size (O(1) running byte
count) and max_size (optional upper bound, 0 means unlimited).
Exceeding max_size puts the chain into a sticky NJS_CHB_ERR_OVERFLOW
state.  As part of the change njs_chb_drain() now returns early on a
failed chain, in line with njs_chb_drop().

diff --git a/src/njs_chb.c b/src/njs_chb.c
index e7bf7685261c9a23d454c6f18d6e29927fec2000..87e2e2b3ee8c87199a71911448919b0bc57515c9 100644 (file)
--- a/src/njs_chb.c
+++ b/src/njs_chb.c
@@ -15,12 +15,14 @@ void
 njs_chb_init(njs_chb_t *chain, void *pool, njs_chb_alloc_t alloc,
     njs_chb_free_t free)
 {
-    chain->error = 0;
+    chain->error = NJS_CHB_ERR_NONE;
     chain->pool = pool;
     chain->alloc = alloc;
     chain->free = free;
     chain->nodes = NULL;
     chain->last = NULL;
+    chain->total_size = 0;
+    chain->max_size = 0;
 }
 
 
@@ -47,6 +49,17 @@ njs_chb_reserve(njs_chb_t *chain, size_t size)
 {
     njs_chb_node_t  *n;
 
+    if (njs_slow_path(chain->error)) {
+        return NULL;
+    }
+
+    if (njs_slow_path(chain->max_size != 0
+                      && chain->total_size + size > chain->max_size))
+    {
+        chain->error = NJS_CHB_ERR_OVERFLOW;
+        return NULL;
+    }
+
     n = chain->last;
 
     if (njs_fast_path(n != NULL && njs_chb_node_room(n) >= size)) {
@@ -59,7 +72,7 @@ njs_chb_reserve(njs_chb_t *chain, size_t size)
 
     n = chain->alloc(chain->pool, sizeof(njs_chb_node_t) + size);
     if (njs_slow_path(n == NULL)) {
-        chain->error = 1;
+        chain->error = NJS_CHB_ERR_NOMEM;
         return NULL;
     }
 
@@ -116,17 +129,26 @@ njs_chb_sprintf(njs_chb_t *chain, size_t size, const char* fmt, ...)
 void
 njs_chb_drain(njs_chb_t *chain, size_t drain)
 {
+    size_t          size;
     njs_chb_node_t  *n;
 
+    if (njs_slow_path(chain->error)) {
+        return;
+    }
+
     n = chain->nodes;
 
     while (n != NULL) {
-        if (njs_chb_node_size(n) > drain) {
+        size = njs_chb_node_size(n);
+
+        if (size > drain) {
             n->start += drain;
+            chain->total_size -= drain;
             return;
         }
 
-        drain -= njs_chb_node_size(n);
+        drain -= size;
+        chain->total_size -= size;
         chain->nodes = n->next;
 
         if (chain->free != NULL) {
@@ -137,6 +159,7 @@ njs_chb_drain(njs_chb_t *chain, size_t drain)
     }
 
     chain->last = NULL;
+    chain->total_size = 0;
 }
 
 
@@ -146,7 +169,7 @@ njs_chb_drain(njs_chb_t *chain, size_t drain)
 void
 njs_chb_drop(njs_chb_t *chain, size_t drop)
 {
-    uint64_t        size;
+    uint64_t        keep, kept;
     njs_chb_node_t  *n, *next;
 
     if (njs_slow_path(chain->error)) {
@@ -157,27 +180,34 @@ njs_chb_drop(njs_chb_t *chain, size_t drop)
 
     if (njs_fast_path(n != NULL && (njs_chb_node_size(n) > drop))) {
         n->pos -= drop;
+        chain->total_size -= drop;
         return;
     }
 
-    n = chain->nodes;
-    size = (uint64_t) njs_chb_size(chain);
-
-    if (drop >= size) {
+    if (drop >= chain->total_size) {
         njs_chb_destroy(chain);
-        njs_chb_init(chain, chain->pool, chain->alloc, chain->free);
+
+        chain->error = NJS_CHB_ERR_NONE;
+        chain->nodes = NULL;
+        chain->last = NULL;
+        chain->total_size = 0;
         return;
     }
 
+    kept = 0;
+    keep = chain->total_size - drop;
+    n = chain->nodes;
+
     while (n != NULL) {
-        size -= njs_chb_node_size(n);
+        kept += njs_chb_node_size(n);
 
-        if (size <= drop) {
+        if (kept >= keep) {
             chain->last = n;
-            chain->last->pos -= drop - size;
+            n->pos -= kept - keep;
 
-            n = chain->last->next;
-            chain->last->next = NULL;
+            next = n->next;
+            n->next = NULL;
+            n = next;
 
             break;
         }
@@ -194,6 +224,8 @@ njs_chb_drop(njs_chb_t *chain, size_t drop)
 
         n = next;
     }
+
+    chain->total_size -= drop;
 }
 
 

diff --git a/src/njs_chb.h b/src/njs_chb.h
index 4bd1cb3f60fa12eb375ee5f9e36591c0a43ddff2..4d24a0ebf2ec2f5d3b25ff13681371491206bef0 100644 (file)
--- a/src/njs_chb.h
+++ b/src/njs_chb.h
@@ -20,8 +20,12 @@ struct njs_chb_node_s {
 typedef void *(*njs_chb_alloc_t)(void *pool, size_t size);
 typedef void (*njs_chb_free_t)(void *pool, void *p);
 
+
 typedef struct {
-    njs_bool_t              error;
+#define NJS_CHB_ERR_NONE      0
+#define NJS_CHB_ERR_NOMEM     1
+#define NJS_CHB_ERR_OVERFLOW  2
+    uint8_t                 error;
 
     void                    *pool;
     njs_chb_alloc_t         alloc;
@@ -29,6 +33,9 @@ typedef struct {
 
     njs_chb_node_t          *nodes;
     njs_chb_node_t          *last;
+
+    uint64_t                total_size;
+    uint64_t                max_size;
 } njs_chb_t;
 
 
@@ -40,6 +47,10 @@ void njs_chb_init(njs_chb_t *chain, void *pool, njs_chb_alloc_t alloc,
 #define NJS_CHB_CTX_INIT(chain, ctx)                                         \
     njs_chb_init(chain, ctx, (njs_chb_alloc_t) js_malloc,                    \
     (njs_chb_free_t) js_free)
+#define NJS_CHB_MP_INIT_MAX(chain, mp, max)                                  \
+    (NJS_CHB_MP_INIT(chain, mp), (chain)->max_size = (max))
+#define NJS_CHB_CTX_INIT_MAX(chain, ctx, max)                                \
+    (NJS_CHB_CTX_INIT(chain, ctx), (chain)->max_size = (max))
 void njs_chb_append0(njs_chb_t *chain, const char *msg, size_t len);
 void njs_chb_vsprintf(njs_chb_t *chain, size_t size, const char *fmt,
     va_list args);
@@ -73,23 +84,11 @@ njs_chb_append_str(njs_chb_t *chain, njs_str_t *str)
 njs_inline int64_t
 njs_chb_size(njs_chb_t *chain)
 {
-    uint64_t        size;
-    njs_chb_node_t  *n;
-
     if (njs_slow_path(chain->error)) {
         return -1;
     }
 
-    n = chain->nodes;
-
-    size = 0;
-
-    while (n != NULL) {
-        size += njs_chb_node_size(n);
-        n = n->next;
-    }
-
-    return size;
+    return chain->total_size;
 }
 
 
@@ -151,6 +150,7 @@ njs_inline void
 njs_chb_written(njs_chb_t *chain, size_t bytes)
 {
     chain->last->pos += bytes;
+    chain->total_size += bytes;
 }
 
 

diff --git a/src/njs_string.c b/src/njs_string.c
index ba781727c1c8c8bfa3f3818e32a5497965d0b7a8..d49f58720705785428ad6a741bff0fa89d8e6e6d 100644 (file)
--- a/src/njs_string.c
+++ b/src/njs_string.c
@@ -107,7 +107,13 @@ njs_string_create_chb(njs_vm_t *vm, njs_value_t *value, njs_chb_t *chain)
 
     size = njs_chb_size(chain);
     if (njs_slow_path(size < 0)) {
-        njs_memory_error(vm);
+        if (chain->error == NJS_CHB_ERR_OVERFLOW) {
+            njs_range_error(vm, "invalid string length");
+
+        } else {
+            njs_memory_error(vm);
+        }
+
         return NJS_ERROR;
     }
 

diff --git a/src/qjs.c b/src/qjs.c
index aa083e819e66ef7bf98d554488391d178ab02e5f..26cdabb407795c33da1cba22165b5fca82afe2b4 100644 (file)
--- a/src/qjs.c
+++ b/src/qjs.c
@@ -1192,12 +1192,22 @@ qjs_string_create_chb(JSContext *cx, njs_chb_t *chain)
     njs_str_t  str;
 
     ret = njs_chb_join(chain, &str);
-    njs_chb_destroy(chain);
 
     if (ret != NJS_OK) {
-        return JS_ThrowInternalError(cx, "failed to create string");
+        if (chain->error == NJS_CHB_ERR_OVERFLOW) {
+            val = JS_ThrowRangeError(cx, "invalid string length");
+
+        } else {
+            val = JS_ThrowInternalError(cx, "failed to create string");
+        }
+
+        njs_chb_destroy(chain);
+
+        return val;
     }
 
+    njs_chb_destroy(chain);
+
     val = JS_NewStringLen(cx, (const char *) str.start, str.length);
 
     chain->free(cx, str.start);

diff --git a/src/test/njs_unit_test.c b/src/test/njs_unit_test.c
index 02ce2c5a9717374e1d7df19f2d7517d26c32bd51..92635708d4130fb6f617b2c316058a9ad8dd5b0f 100644 (file)
--- a/src/test/njs_unit_test.c
+++ b/src/test/njs_unit_test.c
@@ -23692,6 +23692,71 @@ njs_chb_test(njs_vm_t *vm, njs_opts_t *opts, njs_stat_t *stat)
     njs_chb_destroy(&chain);
     njs_mp_free(njs_vm_memory_pool(vm), string.start);
 
+    /*
+     * Overflow: capped chain refuses appends beyond max_size and
+     * reports NJS_CHB_ERR_OVERFLOW.
+     */
+    NJS_CHB_MP_INIT_MAX(&chain, njs_vm_memory_pool(vm), 100);
+
+    njs_chb_append_literal(&chain, "0123456789");
+
+    if (chain.error != NJS_CHB_ERR_NONE || njs_chb_size(&chain) != 10) {
+        ret = NJS_ERROR;
+        njs_printf("chb cap: pre-overflow state wrong, "
+                   "error:%d size:%z\n",
+                   (int) chain.error, (size_t) njs_chb_size(&chain));
+        goto done;
+    }
+
+    for (i = 0; i < 20; i++) {
+        njs_chb_append(&chain, "0123456789", 10);
+    }
+
+    if (chain.error != NJS_CHB_ERR_OVERFLOW) {
+        ret = NJS_ERROR;
+        njs_printf("chb cap: expected NJS_CHB_ERR_OVERFLOW, got %d\n",
+                   (int) chain.error);
+        goto done;
+    }
+
+    if (njs_chb_size(&chain) != -1) {
+        ret = NJS_ERROR;
+        njs_printf("chb cap: size after overflow should be -1, got %z\n",
+                   (size_t) njs_chb_size(&chain));
+        goto done;
+    }
+
+    njs_chb_destroy(&chain);
+
+    /*
+     * Full-chain drop preserves max_size.
+     */
+    NJS_CHB_MP_INIT_MAX(&chain, njs_vm_memory_pool(vm), 20);
+
+    njs_chb_append_literal(&chain, "0123456789");
+    njs_chb_drop(&chain, 100);
+
+    if (chain.max_size != 20 || njs_chb_size(&chain) != 0) {
+        ret = NJS_ERROR;
+        njs_printf("chb drop: full reset lost cap or size, "
+                   "max_size:%z size:%z\n",
+                   (size_t) chain.max_size, (size_t) njs_chb_size(&chain));
+        goto done;
+    }
+
+    /* Chain still enforces the cap after the reset. */
+    for (i = 0; i < 5; i++) {
+        njs_chb_append_literal(&chain, "0123456789");
+    }
+
+    if (chain.error != NJS_CHB_ERR_OVERFLOW) {
+        ret = NJS_ERROR;
+        njs_printf("chb drop: cap not enforced after full reset\n");
+        goto done;
+    }
+
+    njs_chb_destroy(&chain);
+
 done:
 
     if (ret != NJS_OK) {