From bda947e17a61b9f7434ea878dff04a409a2ff772 Mon Sep 17 00:00:00 2001 From: Nick Terrell Date: Thu, 26 Jan 2023 12:11:25 -0800 Subject: [PATCH] [huf] Fix bug in fast C decoders The input bounds checks were buggy because they were only breaking from the inner loop, not the outer loop. The fuzzers found this immediately. The fix is to use `goto _out` instead of `break`. This condition can happen on corrupted inputs. I've benchmarked before and after on x86-64 and there were small changes in performance, some positive, and some negative, and they end up about balacing out. Credit to OSS-Fuzz --- lib/decompress/huf_decompress.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/decompress/huf_decompress.c b/lib/decompress/huf_decompress.c index 0a29f03d4..c2d1f633a 100644 --- a/lib/decompress/huf_decompress.c +++ b/lib/decompress/huf_decompress.c @@ -742,7 +742,7 @@ void HUF_decompress4X1_usingDTable_internal_fast_c_loop(HUF_DecompressFastArgs* */ for (stream = 1; stream < 4; ++stream) { if (ip[stream] < ip[stream - 1]) - break; + goto _out; } } @@ -775,6 +775,8 @@ void HUF_decompress4X1_usingDTable_internal_fast_c_loop(HUF_DecompressFastArgs* } while (op[3] < olimit); } +_out: + /* Save the final values of each of the state variables back to args. */ ZSTD_memcpy(&args->bits, &bits, sizeof(bits)); ZSTD_memcpy(&args->ip, &ip, sizeof(ip)); @@ -1535,7 +1537,7 @@ void HUF_decompress4X2_usingDTable_internal_fast_c_loop(HUF_DecompressFastArgs* */ for (stream = 1; stream < 4; ++stream) { if (ip[stream] < ip[stream - 1]) - break; + goto _out; } } @@ -1593,6 +1595,8 @@ void HUF_decompress4X2_usingDTable_internal_fast_c_loop(HUF_DecompressFastArgs* } while (op[3] < olimit); } +_out: + /* Save the final values of each of the state variables back to args. */ ZSTD_memcpy(&args->bits, &bits, sizeof(bits)); ZSTD_memcpy(&args->ip, &ip, sizeof(ip)); -- 2.47.2