This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a commit to branch 9.2.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/9.2.x by this push:
new 886190ceb Fix `COMPRESSION_ERROR` on valid HPACK input (#8817)
886190ceb is described below
commit 886190ceb74dfba49988b2a9ee5e33d670a3f5cf
Author: Menno de Gier <[email protected]>
AuthorDate: Tue May 17 01:34:59 2022 +0100
Fix `COMPRESSION_ERROR` on valid HPACK input (#8817)
* Check for leaf node after resetting byte counter
* Improve invalid decoding length
* Move error tests to test_Huffmancode
* Reject EOS symbol
(cherry picked from commit d362729d2ec4a9ae4ffdead43e5f0d680c0ea989)
---
proxy/hdrs/HuffmanCodec.cc | 45 ++++++++++++++++++++++++-------------
proxy/hdrs/test_Huffmancode.cc | 26 +++++++++++++++++++++
proxy/hdrs/unit_tests/test_XPACK.cc | 38 ++++++++++++++++++-------------
3 files changed, 78 insertions(+), 31 deletions(-)
diff --git a/proxy/hdrs/HuffmanCodec.cc b/proxy/hdrs/HuffmanCodec.cc
index 546269042..a99e30306 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 24344d7b8..6f5bf7b91 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 91bd8b311..3404e6b67 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")
{
