diff --git a/lib/nghttp2_hd.c b/lib/nghttp2_hd.c index 3f1340e7..f331f057 100644 --- a/lib/nghttp2_hd.c +++ b/lib/nghttp2_hd.c @@ -485,10 +485,10 @@ static size_t encode_length(uint8_t *buf, size_t n, size_t prefix) } /* - * Decodes |prefx| prefixed integer stored from |in|. The |last| + * Decodes |prefx| prefixed integer stored from |in|. The |last| * represents the 1 beyond the last of the valid contiguous memory - * region from |in|. The decoded integer must be strictly less than 1 - * << 16. + * region from |in|. The decoded integer must be less than or equal + * to UINT32_MAX. * * If the |initial| is nonzero, it is used as a initial value, this * function assumes the |in| starts with intermediate data. @@ -496,40 +496,54 @@ static size_t encode_length(uint8_t *buf, size_t n, size_t prefix) * An entire integer is decoded successfully, decoded, the |*final| is * set to nonzero. * - * This function returns the next byte of read byte. This function - * stores the decoded integer in |*res| and number of shift to make in - * the next decoding in |*shift_ptr| if it succeed, including partial - * decoding, or stores -1 in |*res|, indicating decoding error. + * This function stores the decoded integer in |*res| if it succeed, + * including partial decoding (in this case, number of shift to make + * in the next call will be stored in |*shift_ptr|) and returns number + * of bytes processed, or returns -1, indicating decoding error. */ -static uint8_t* decode_length(ssize_t *res, size_t *shift_ptr, int *final, - ssize_t initial, size_t shift, - uint8_t *in, uint8_t *last, size_t prefix) +static ssize_t decode_length(uint32_t *res, size_t *shift_ptr, int *final, + uint32_t initial, size_t shift, + uint8_t *in, uint8_t *last, size_t prefix) { - int k = (1 << prefix) - 1; - ssize_t n = initial; + uint32_t k = (1 << prefix) - 1; + uint32_t n = initial; + uint8_t *start = in; *shift_ptr = 0; *final = 0; if(n == 0) { - if((*in & k) == k) { - n = k; - } else { + if((*in & k) != k) { *res = (*in) & k; *final = 1; - return in + 1; + return 1; } + + n = k; + if(++in == last) { *res = n; - return in; + return in - start; } } + for(; in != last; ++in, shift += 7) { - n += (*in & 0x7f) << shift; - if(n >= (1 << 16)) { - *res = -1; - return in + 1; + uint32_t add = *in & 0x7f; + + if((UINT32_MAX >> shift) < add) { + DEBUGF(fprintf(stderr, "inflate: integer overflow on shift\n")); + return -1; } + + add <<= shift; + + if(UINT32_MAX - add < n) { + DEBUGF(fprintf(stderr, "inflate: integer overflow on addition\n")); + return -1; + } + + n += add; + if((*in & (1 << 7)) == 0) { break; } @@ -539,12 +553,12 @@ static uint8_t* decode_length(ssize_t *res, size_t *shift_ptr, int *final, if(in == last) { *res = n; - return in; + return in - start; } *res = n; *final = 1; - return in + 1; + return in + 1 - start; } static int emit_clear_refset(nghttp2_bufs *bufs) @@ -1353,21 +1367,29 @@ static ssize_t hd_inflate_read_len(nghttp2_hd_inflater *inflater, uint8_t *in, uint8_t *last, size_t prefix, size_t maxlen) { - uint8_t *nin; + ssize_t rv; + uint32_t out; + *rfin = 0; - nin = decode_length(&inflater->left, &inflater->shift, rfin, inflater->left, - inflater->shift, in, last, prefix); - if(inflater->left == -1) { - DEBUGF(fprintf(stderr, "inflatehd: invalid integer\n")); + + rv = decode_length(&out, &inflater->shift, rfin, (uint32_t)inflater->left, + inflater->shift, in, last, prefix); + + if(rv == -1) { + DEBUGF(fprintf(stderr, "inflatehd: integer decoding failed\n")); return NGHTTP2_ERR_HEADER_COMP; } - if((size_t)inflater->left > maxlen) { + + if(out > maxlen) { DEBUGF(fprintf(stderr, "inflatehd: integer exceeded the maximum value %zu\n", maxlen)); return NGHTTP2_ERR_HEADER_COMP; } - return (ssize_t)(nin - in); + + inflater->left = out; + + return rv; } /* @@ -1391,7 +1413,7 @@ static ssize_t hd_inflate_read_huff(nghttp2_hd_inflater *inflater, { ssize_t readlen; int final = 0; - if(last - in >= inflater->left) { + if((size_t)(last - in) >= inflater->left) { last = in + inflater->left; final = 1; } @@ -1402,7 +1424,7 @@ static ssize_t hd_inflate_read_huff(nghttp2_hd_inflater *inflater, DEBUGF(fprintf(stderr, "inflatehd: huffman decoding failed\n")); return readlen; } - inflater->left -= readlen; + inflater->left -= (size_t)readlen; return readlen; } @@ -1425,12 +1447,12 @@ static ssize_t hd_inflate_read(nghttp2_hd_inflater *inflater, uint8_t *in, uint8_t *last) { int rv; - size_t len = nghttp2_min(last - in, inflater->left); + size_t len = nghttp2_min((size_t)(last - in), inflater->left); rv = nghttp2_bufs_add(bufs, in, len); if(rv != 0) { return rv; } - inflater->left -= (ssize_t)len; + inflater->left -= len; return (ssize_t)len; } @@ -1715,7 +1737,7 @@ ssize_t nghttp2_hd_inflate_hd(nghttp2_hd_inflater *inflater, if(!rfin) { goto almost_ok; } - DEBUGF(fprintf(stderr, "inflatehd: table_size=%zd\n", inflater->left)); + DEBUGF(fprintf(stderr, "inflatehd: table_size=%zu\n", inflater->left)); inflater->ctx.hd_table_bufsize_max = inflater->left; hd_context_shrink_table_size(&inflater->ctx); inflater->state = NGHTTP2_HD_STATE_OPCODE; @@ -1748,7 +1770,7 @@ ssize_t nghttp2_hd_inflate_hd(nghttp2_hd_inflater *inflater, if(!rfin) { goto almost_ok; } - DEBUGF(fprintf(stderr, "inflatehd: index=%zd\n", inflater->left)); + DEBUGF(fprintf(stderr, "inflatehd: index=%zu\n", inflater->left)); if(inflater->opcode == NGHTTP2_HD_OPCODE_INDEXED) { inflater->index = inflater->left; assert(inflater->index > 0); @@ -1791,7 +1813,7 @@ ssize_t nghttp2_hd_inflate_hd(nghttp2_hd_inflater *inflater, in += rv; if(!rfin) { DEBUGF(fprintf(stderr, - "inflatehd: integer not fully decoded. current=%zd\n", + "inflatehd: integer not fully decoded. current=%zu\n", inflater->left)); goto almost_ok; @@ -1817,7 +1839,7 @@ ssize_t nghttp2_hd_inflate_hd(nghttp2_hd_inflater *inflater, if(inflater->left) { DEBUGF(fprintf(stderr, - "inflatehd: still %zd bytes to go\n", inflater->left)); + "inflatehd: still %zu bytes to go\n", inflater->left)); goto almost_ok; } @@ -1838,7 +1860,7 @@ ssize_t nghttp2_hd_inflate_hd(nghttp2_hd_inflater *inflater, DEBUGF(fprintf(stderr, "inflatehd: %zd bytes read\n", rv)); if(inflater->left) { DEBUGF(fprintf(stderr, - "inflatehd: still %zd bytes to go\n", inflater->left)); + "inflatehd: still %zu bytes to go\n", inflater->left)); goto almost_ok; } @@ -1870,7 +1892,7 @@ ssize_t nghttp2_hd_inflate_hd(nghttp2_hd_inflater *inflater, goto almost_ok; } - DEBUGF(fprintf(stderr, "inflatehd: valuelen=%zd\n", inflater->left)); + DEBUGF(fprintf(stderr, "inflatehd: valuelen=%zu\n", inflater->left)); if(inflater->left == 0) { if(inflater->opcode == NGHTTP2_HD_OPCODE_NEWNAME) { rv = hd_inflate_commit_newname(inflater, nv_out); @@ -1905,7 +1927,7 @@ ssize_t nghttp2_hd_inflate_hd(nghttp2_hd_inflater *inflater, if(inflater->left) { DEBUGF(fprintf(stderr, - "inflatehd: still %zd bytes to go\n", inflater->left)); + "inflatehd: still %zu bytes to go\n", inflater->left)); goto almost_ok; } @@ -1938,7 +1960,7 @@ ssize_t nghttp2_hd_inflate_hd(nghttp2_hd_inflater *inflater, if(inflater->left) { DEBUGF(fprintf(stderr, - "inflatehd: still %zd bytes to go\n", inflater->left)); + "inflatehd: still %zu bytes to go\n", inflater->left)); goto almost_ok; } @@ -2063,3 +2085,11 @@ int nghttp2_hd_emit_table_size(nghttp2_bufs *bufs, size_t table_size) { return emit_table_size(bufs, table_size); } + +ssize_t nghttp2_hd_decode_length(uint32_t *res, size_t *shift_ptr, int *final, + uint32_t initial, size_t shift, + uint8_t *in, uint8_t *last, size_t prefix) +{ + return decode_length(res, shift_ptr, final, initial, shift, in, last, + prefix); +} diff --git a/lib/nghttp2_hd.h b/lib/nghttp2_hd.h index 4bcfcb25..59affd49 100644 --- a/lib/nghttp2_hd.h +++ b/lib/nghttp2_hd.h @@ -156,7 +156,7 @@ struct nghttp2_hd_inflater { name. This entry must be in header table. */ nghttp2_hd_entry *ent_name; /* The number of bytes to read */ - ssize_t left; + size_t left; /* The index in indexed repr or indexed name */ size_t index; /* The index of header table to toggle off the entry from reference @@ -292,6 +292,11 @@ int nghttp2_hd_emit_table_size(nghttp2_bufs *bufs, size_t table_size); nghttp2_hd_entry* nghttp2_hd_table_get(nghttp2_hd_context *context, size_t index); +/* For unittesting purpose */ +ssize_t nghttp2_hd_decode_length(uint32_t *res, size_t *shift_ptr, int *final, + uint32_t initial, size_t shift, + uint8_t *in, uint8_t *last, size_t prefix); + /* Huffman encoding/decoding functions */ /* diff --git a/tests/main.c b/tests/main.c index bb26c08f..a44affec 100644 --- a/tests/main.c +++ b/tests/main.c @@ -285,6 +285,8 @@ int main(int argc, char* argv[]) !CU_add_test(pSuite, "hd_deflate_bound", test_nghttp2_hd_deflate_bound) || !CU_add_test(pSuite, "hd_public_api", test_nghttp2_hd_public_api) || + !CU_add_test(pSuite, "hd_decode_length", + test_nghttp2_hd_decode_length) || !CU_add_test(pSuite, "adjust_local_window_size", test_nghttp2_adjust_local_window_size) || !CU_add_test(pSuite, "check_header_name", diff --git a/tests/nghttp2_hd_test.c b/tests/nghttp2_hd_test.c index f2309d31..0ceb5bb6 100644 --- a/tests/nghttp2_hd_test.c +++ b/tests/nghttp2_hd_test.c @@ -883,9 +883,33 @@ void test_nghttp2_hd_change_table_size(void) nva_out_reset(&out); nghttp2_bufs_reset(&bufs); - nghttp2_bufs_free(&bufs); nghttp2_hd_inflate_free(&inflater); nghttp2_hd_deflate_free(&deflater); + + /* Check that table size UINT32_MAX can be received */ + nghttp2_hd_deflate_init2(&deflater, UINT32_MAX); + nghttp2_hd_inflate_init(&inflater); + + CU_ASSERT(0 == nghttp2_hd_inflate_change_table_size(&inflater, UINT32_MAX)); + CU_ASSERT(0 == nghttp2_hd_deflate_change_table_size(&deflater, UINT32_MAX)); + + rv = nghttp2_hd_deflate_hd_bufs(&deflater, &bufs, nva, 2); + blocklen = nghttp2_bufs_len(&bufs); + + CU_ASSERT(0 == rv); + CU_ASSERT(UINT32_MAX == deflater.ctx.hd_table_bufsize_max); + + CU_ASSERT(blocklen == inflate_hd(&inflater, &out, &bufs, 0)); + CU_ASSERT(UINT32_MAX == inflater.ctx.hd_table_bufsize_max); + CU_ASSERT(UINT32_MAX == inflater.settings_hd_table_bufsize_max); + + nva_out_reset(&out); + nghttp2_bufs_reset(&bufs); + + nghttp2_hd_inflate_free(&inflater); + nghttp2_hd_deflate_free(&deflater); + + nghttp2_bufs_free(&bufs); } static void check_deflate_inflate(nghttp2_hd_deflater *deflater, @@ -1199,3 +1223,79 @@ void test_nghttp2_hd_public_api(void) nghttp2_hd_deflate_del(deflater); } +static size_t encode_length(uint8_t *buf, uint64_t n, size_t prefix) +{ + size_t k = (1 << prefix) - 1; + size_t len = 0; + *buf &= ~k; + if(n >= k) { + *buf++ |= k; + n -= k; + ++len; + } else { + *buf++ |= n; + return 1; + } + do { + ++len; + if(n >= 128) { + *buf++ = (1 << 7) | (n & 0x7f); + n >>= 7; + } else { + *buf++ = (uint8_t)n; + break; + } + } while(n); + return len; +} + +void test_nghttp2_hd_decode_length(void) +{ + uint32_t out; + size_t shift; + int final; + uint8_t buf[16]; + uint8_t *bufp; + size_t len; + ssize_t rv; + size_t i; + + memset(buf, 0, sizeof(buf)); + len = encode_length(buf, UINT32_MAX, 7); + + rv = nghttp2_hd_decode_length(&out, &shift, &final, 0, 0, buf, buf + len, 7); + + CU_ASSERT((ssize_t)len == rv); + CU_ASSERT(0 != final); + CU_ASSERT(UINT32_MAX == out); + + /* Make sure that we can decode integer if we feed 1 byte at a + time */ + out = 0; + shift = 0; + final = 0; + bufp = buf; + + for(i = 0; i < len; ++i, ++bufp) { + rv = nghttp2_hd_decode_length(&out, &shift, &final, out, shift, + bufp, bufp + 1, 7); + + CU_ASSERT(rv == 1); + + if(final) { + break; + } + } + + CU_ASSERT(i == len - 1); + CU_ASSERT(0 != final); + CU_ASSERT(UINT32_MAX == out); + + /* Check overflow case */ + memset(buf, 0, sizeof(buf)); + len = encode_length(buf, 1ll << 32, 7); + + rv = nghttp2_hd_decode_length(&out, &shift, &final, 0, 0, buf, buf + len, 7); + + CU_ASSERT(-1 == rv); +} diff --git a/tests/nghttp2_hd_test.h b/tests/nghttp2_hd_test.h index 51bf4ed6..21dc5294 100644 --- a/tests/nghttp2_hd_test.h +++ b/tests/nghttp2_hd_test.h @@ -43,5 +43,6 @@ void test_nghttp2_hd_deflate_inflate(void); void test_nghttp2_hd_no_index(void); void test_nghttp2_hd_deflate_bound(void); void test_nghttp2_hd_public_api(void); +void test_nghttp2_hd_decode_length(void); #endif /* NGHTTP2_HD_TEST_H */