[!] fix pow2 overflow (#277)

alibaba · Jan 3, 2023 · 917ac4a · 917ac4a
1 parent 6348479
commit 917ac4a
Show file tree

Hide file tree

Showing 7 changed files with 48 additions and 3 deletions.
diff --git a/src/common/utils/2d_hash/xqc_2d_hash_table.c b/src/common/utils/2d_hash/xqc_2d_hash_table.c
@@ -57,6 +57,10 @@ xqc_2d_hash_table_create(size_t bkt_cnt, xqc_2d_hash_table_data_cmp dcmp,
 
     /* make bucket count the least upper power of 2 */
     uint64_t bcnt = xqc_pow2_upper(bkt_cnt);
+    if (bcnt == XQC_POW2_UPPER_ERROR) {
+        xqc_free(ht);
+        return NULL;
+    }
     xqc_list_head_t *list = (xqc_list_head_t *)xqc_malloc(bcnt * sizeof(xqc_list_head_t));
     if (NULL == list) {
         xqc_free(ht);

diff --git a/src/common/utils/ringarray/xqc_ring_array.c b/src/common/utils/ringarray/xqc_ring_array.c
@@ -39,6 +39,10 @@ xqc_rarray_create(size_t cap, size_t esize)
     uint64_t array_cap = 0;
     if (esize != 0) {
         array_cap = xqc_pow2_upper(cap);
+        if (array_cap == XQC_POW2_UPPER_ERROR) {
+            xqc_free(ra);
+            return NULL;
+        }
         ra->buf = xqc_malloc(array_cap * esize);
         if (ra->buf == NULL) {
             xqc_free(ra);
@@ -78,6 +82,9 @@ xqc_rarray_check_range(xqc_rarray_t *ra, uint64_t offset)
          * ra->offset equals to eoffset, only if offset not exceed capacity,
          * it is always in range.
          */
+        if (ra->count == 0) {
+            return XQC_FALSE;
+        }
         return offset >= ra->offset || offset < eoffset;
 
     } else {
@@ -175,6 +182,9 @@ xqc_rarray_resize(xqc_rarray_t *ra, uint64_t cap)
     }
 
     uint64_t array_cap = xqc_pow2_upper(cap);
+    if (array_cap == XQC_POW2_UPPER_ERROR) {
+        return -XQC_EMALLOC;
+    }
     uint8_t *buf = xqc_malloc(array_cap * ra->esize);
     if (buf == NULL) {
         return -XQC_EMALLOC;

diff --git a/src/common/utils/ringmem/xqc_ring_mem.c b/src/common/utils/ringmem/xqc_ring_mem.c
@@ -45,6 +45,10 @@ xqc_ring_mem_create(size_t sz)
     uint64_t msize = 0;
     if (sz != 0) {
         msize = xqc_pow2_upper(sz);
+        if (msize == XQC_POW2_UPPER_ERROR) {
+            xqc_free(rmem);
+            return NULL;
+        }
         rmem->buf = (uint8_t *)xqc_malloc(msize);
         if (rmem->buf == NULL) {
             xqc_free(rmem);
@@ -87,6 +91,9 @@ xqc_ring_mem_resize(xqc_ring_mem_t *rmem, size_t cap)
     }
 
     uint64_t mcap = xqc_pow2_upper(cap);
+    if (mcap == XQC_POW2_UPPER_ERROR) {
+        return -XQC_EPARAM;
+    }
     uint8_t *buf = (uint8_t *)xqc_malloc(mcap);
     if (buf == NULL) {
         return -XQC_EMALLOC;

diff --git a/src/common/xqc_common.h b/src/common/xqc_common.h
@@ -22,10 +22,16 @@
 typedef unsigned char   u_char;
 
 
+#define XQC_POW2_UPPER_ERROR 0
+
 static inline uint64_t
 xqc_pow2_upper(uint64_t n)
 {
-    size_t m = 1;
+    if (n > 0x8000000000000000) {
+        /* return zero mean error */
+        return XQC_POW2_UPPER_ERROR;
+    }
+    uint64_t m = 1;
     for(; m < n; m = m << 1);
     return m;
 }

diff --git a/src/http3/qpack/xqc_rep.c b/src/http3/qpack/xqc_rep.c
@@ -132,12 +132,19 @@ xqc_rep_decode_prefix(xqc_rep_ctx_t *ctx, size_t max_ents, uint64_t icnt, unsign
 
         if (fin) {
             ctx->state = XQC_REP_DECODE_STATE_BASE_SIGN;
+            /* finish parsing the Required Insert Count field */
             xqc_int_t ret = xqc_rep_reconstruct_ric(ctx, max_ents, icnt);
             if (ret < 0) {
                 return ret;
             }
 
+            /* if all bytes are consumed now, return and wait more bytes */
+            if (pos >= end) {
+                break;
+            }
+
         } else {
+            /* the Required Insert Count field need more bytes */
             break;
         }
 

diff --git a/src/http3/xqc_var_buf.c b/src/http3/xqc_var_buf.c
@@ -83,6 +83,9 @@ xqc_var_buf_realloc(xqc_var_buf_t *buf, size_t cap)
     }
 
     uint64_t capacity = xqc_pow2_upper(cap);
+    if (capacity == XQC_POW2_UPPER_ERROR) {
+        return -XQC_EMALLOC;
+    }
     if (capacity > buf->limit) {
         capacity = buf->limit;
     }
@@ -111,6 +114,9 @@ xqc_int_t
 xqc_var_buf_reduce(xqc_var_buf_t *buf)
 {
     uint64_t capacity = xqc_pow2_upper(buf->data_len - buf->consumed_len);
+    if (capacity == XQC_POW2_UPPER_ERROR) {
+        return -XQC_EPARAM;
+    }
     if (capacity > buf->limit) {
         return -XQC_EMALLOC;
     }

diff --git a/tests/unittest/xqc_qpack_test.c b/tests/unittest/xqc_qpack_test.c
@@ -200,8 +200,13 @@ xqc_qpack_test_basic()
     /* decode stream 1, shall not be blocked */
     xqc_bool_t blocked2 = XQC_FALSE;
     void *req_ctx2 = xqc_qpack_create_req_ctx(1);
-    read = xqc_qpack_dec_headers(qpk_server, req_ctx2, efs_buf_server->data + efs_buf_server->consumed_len, efs_buf_server->data_len - efs_buf_server->consumed_len, &hdrs_out2, 1, &blocked);
-    CU_ASSERT(read == efs_buf_server->data_len && blocked2 == XQC_FALSE);
+    while (efs_buf_server->consumed_len < efs_buf_server->data_len) {
+        read = xqc_qpack_dec_headers(qpk_server, req_ctx2, efs_buf_server->data + efs_buf_server->consumed_len, 1, &hdrs_out2, 1, &blocked);
+
+        CU_ASSERT(read == 1);
+        efs_buf_server->consumed_len += read;
+    }
+    CU_ASSERT(efs_buf_server->data_len == efs_buf_server->data_len && blocked2 == XQC_FALSE);
     efs_buf_server->consumed_len += read;
     xqc_qpack_destroy_req_ctx(req_ctx2);