diff --git a/proxy/hdrs/HuffmanCodec.cc b/proxy/hdrs/HuffmanCodec.cc index 546269042c6..a99e30306c8 100644 --- a/proxy/hdrs/HuffmanCodec.cc +++ b/proxy/hdrs/HuffmanCodec.cc @@ -149,46 +149,61 @@ hpack_huffman_fin() } } +#define MAX_HUFFMAN_CODE_LEN 30 +#define EOS 0x3fffffff + int64_t huffman_decode(char *dst_start, const uint8_t *src, uint32_t src_len) { - char *dst_end = dst_start; - uint8_t shift = 7; - Node *current = HUFFMAN_TREE_ROOT; - int byte_boundary_crossed = 0; - bool includes_zero = false; + char *dst_end = dst_start; + uint8_t shift = 7; + Node *current = HUFFMAN_TREE_ROOT; + int nbits = 0; + uint32_t curr_bits = 0; while (src_len) { + if (nbits > 0) { + curr_bits <<= 1; + } if (*src & (1 << shift)) { + curr_bits |= 1; current = current->right; } else { - current = current->left; - includes_zero = true; + current = current->left; } + ++nbits; if (current->leaf_node == true) { - *dst_end = current->ascii_code; + if (curr_bits == EOS) { + return -1; + } + nbits = 0; + curr_bits = 0; + *dst_end = current->ascii_code; ++dst_end; - current = HUFFMAN_TREE_ROOT; - byte_boundary_crossed = 0; - includes_zero = false; + current = HUFFMAN_TREE_ROOT; } + if (shift) { --shift; } else { shift = 7; ++src; --src_len; - ++byte_boundary_crossed; } - if (byte_boundary_crossed > 3) { + + if (nbits > MAX_HUFFMAN_CODE_LEN) { return -1; } } - if (byte_boundary_crossed > 1) { + + if (nbits > 7) { return -1; } - if (includes_zero) { + + // Padding bits must be a prefix of EOS + uint8_t mask = (1 << nbits) - 1; + if ((mask & curr_bits) != mask) { return -1; } diff --git a/proxy/hdrs/test_Huffmancode.cc b/proxy/hdrs/test_Huffmancode.cc index 24344d7b8c1..6f5bf7b915c 100644 --- a/proxy/hdrs/test_Huffmancode.cc +++ b/proxy/hdrs/test_Huffmancode.cc @@ -136,6 +136,12 @@ values_test() int bytes = huffman_decode(dst_start, encoded_mapped.y, encoded_size); char ascii_value = i / 2; + + // EOS is treated as invalid so check for an error + if (value == 0x3fffffff) { + assert(bytes == -1); + continue; + } assert(dst_start[0] == ascii_value); assert(bytes == 1); } @@ -171,6 +177,25 @@ encode_test() } } +void +decode_errors_test() +{ + const static struct { + char *input; + int input_len; + } test_cases[] = { + {(char *)"\x00", 1}, {(char *)"\xff", 1}, {(char *)"\x1f\xff", 2}, {(char *)"\xff\xae", 2}, {(char *)"\xff\x9f\xff\xff\xff", 5}, + }; + + for (unsigned int i = 0; i < sizeof(test_cases) / sizeof(test_cases[0]); i++) { + int dst_len = 2 * test_cases[i].input_len; + char *dst = (char *)malloc(dst_len); + int len = huffman_decode(dst, (uint8_t *)test_cases[i].input, test_cases[i].input_len); + assert(len == -1); + free(dst); + } +} + int main() { @@ -180,6 +205,7 @@ main() random_test(); } values_test(); + decode_errors_test(); hpack_huffman_fin(); diff --git a/proxy/hdrs/unit_tests/test_XPACK.cc b/proxy/hdrs/unit_tests/test_XPACK.cc index 91bd8b31161..3404e6b6786 100644 --- a/proxy/hdrs/unit_tests/test_XPACK.cc +++ b/proxy/hdrs/unit_tests/test_XPACK.cc @@ -73,22 +73,28 @@ TEST_CASE("XPACK_String", "[xpack]") uint32_t raw_string_len; uint8_t *encoded_field; int encoded_field_len; - } string_test_case[] = {{(char *)"", 0, - (uint8_t *)"\x0" - "", - 1}, - {(char *)"custom-key", 10, - (uint8_t *)"\xA" - "custom-key", - 11}, - {(char *)"", 0, - (uint8_t *)"\x80" - "", - 1}, - {(char *)"custom-key", 10, - (uint8_t *)"\x88" - "\x25\xa8\x49\xe9\x5b\xa9\x7d\x7f", - 9}}; + } string_test_case[] = { + {(char *)"", 0, + (uint8_t *)"\x0" + "", + 1}, + {(char *)"custom-key", 10, + (uint8_t *)"\xA" + "custom-key", + 11}, + {(char *)"", 0, + (uint8_t *)"\x80" + "", + 1}, + {(char *)"custom-key", 10, + (uint8_t *)"\x88" + "\x25\xa8\x49\xe9\x5b\xa9\x7d\x7f", + 9}, + {(char *)"cw Times New Roman_σ=1", 23, + (uint8_t *)"\x95" + "\x27\x85\x37\x9a\x92\xa1\x4d\x25\xf0\xa6\xd3\xd2\x3a\xa2\xff\xff\xf6\xff\xff\x44\x01", + 22}, + }; SECTION("Encoding") {