From 80a4970f3817294b6cba8731c02cca23e3bc5907 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Svensson?= Date: Thu, 1 Sep 2022 13:47:54 +0200 Subject: [PATCH 1/2] Fix heap-buffer-overflow issue in redisvFormatCommad A command with a faulty formatting string that lacks the conversion specifier results in a ASAN heap-buffer-overflow. This was due to that strchr() matches on null-termination, which triggers a continuation of the string parsing. --- hiredis.c | 5 +++++ test.c | 6 +++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/hiredis.c b/hiredis.c index 91086f6f6..b2b4d1f58 100644 --- a/hiredis.c +++ b/hiredis.c @@ -399,6 +399,11 @@ int redisvFormatCommand(char **target, const char *format, va_list ap) { /* Copy va_list before consuming with va_arg */ va_copy(_cpy,ap); + /* Make sure we have more characters otherwise strchr() accepts + * '\0' as an integer specifier. This is checked after above + * va_copy() to avoid UB in fmt_invalid's call to va_end(). */ + if (*_p == '\0') goto fmt_invalid; + /* Integer conversion (without modifiers) */ if (strchr(intfmts,*_p) != NULL) { va_arg(ap,int); diff --git a/test.c b/test.c index b90175214..3b98dda96 100644 --- a/test.c +++ b/test.c @@ -330,10 +330,14 @@ static void test_format_commands(void) { FLOAT_WIDTH_TEST(float); FLOAT_WIDTH_TEST(double); - test("Format command with invalid printf format: "); + test("Format command with unhandled printf format (specifier 'p' not supported): "); len = redisFormatCommand(&cmd,"key:%08p %b",(void*)1234,"foo",(size_t)3); test_cond(len == -1); + test("Format command with invalid printf format (specifier missing): "); + len = redisFormatCommand(&cmd,"%-"); + test_cond(len == -1); + const char *argv[3]; argv[0] = "SET"; argv[1] = "foo\0xxx"; From b8f0f4888d020c0681d5ce65acefb84e2258b8b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Svensson?= Date: Thu, 1 Sep 2022 15:32:53 +0200 Subject: [PATCH 2/2] fuzzer: No alloc in redisFormatCommand() when fail --- fuzzing/format_command_fuzzer.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/fuzzing/format_command_fuzzer.c b/fuzzing/format_command_fuzzer.c index 91adeac58..de125e08d 100644 --- a/fuzzing/format_command_fuzzer.c +++ b/fuzzing/format_command_fuzzer.c @@ -48,10 +48,9 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { memcpy(new_str, data, size); new_str[size] = '\0'; - redisFormatCommand(&cmd, new_str); - - if (cmd != NULL) + if (redisFormatCommand(&cmd, new_str) != -1) hi_free(cmd); + free(new_str); return 0; }