From 14176484697dc96df62c76630eef2b97a89bb8d0 Mon Sep 17 00:00:00 2001 From: Oran Agra Date: Sun, 21 Nov 2021 14:30:20 +0200 Subject: [PATCH] Prevent LCS from allocating temp memory over proto-max-bulk-len (#9817) LCS can allocate immense amount of memory (sizes of two inputs multiplied by each other). In the past this caused some possible security issues due to overflows, which we solved and also added use of `trymalloc` to return "Insufficient memory" instead of OOM panic zmalloc. But in case overcommit is enabled, it could be that we won't get the OOM panic, and zmalloc will succeed, and then we can get OOM killed by the kernel. The solution here is to prevent LCS from allocating transient memory that's bigger than `proto-max-bulk-len` config. This config is not directly related to transient memory, but using a hard coded value ad well as introducing a specific config seems wrong. This comes to solve an error in the corrupt-dump-fuzzer test that started in the daily CI see #9799 --- src/t_string.c | 9 +++++++-- tests/integration/corrupt-dump.tcl | 9 +++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/t_string.c b/src/t_string.c index d4334b1eb6..7298fda6fc 100644 --- a/src/t_string.c +++ b/src/t_string.c @@ -800,10 +800,15 @@ void lcsCommand(client *c) { unsigned long long lcssize = (unsigned long long)(alen+1)*(blen+1); /* Can't overflow due to the size limits above. */ unsigned long long lcsalloc = lcssize * sizeof(uint32_t); uint32_t *lcs = NULL; - if (lcsalloc < SIZE_MAX && lcsalloc / lcssize == sizeof(uint32_t)) + if (lcsalloc < SIZE_MAX && lcsalloc / lcssize == sizeof(uint32_t)) { + if (lcsalloc > (size_t)server.proto_max_bulk_len) { + addReplyError(c, "Insufficient memory, transient memory for LCS exceeds proto-max-bulk-len"); + goto cleanup; + } lcs = ztrymalloc(lcsalloc); + } if (!lcs) { - addReplyError(c, "Insufficient memory"); + addReplyError(c, "Insufficient memory, failed allocating transient memory for LCS"); goto cleanup; } diff --git a/tests/integration/corrupt-dump.tcl b/tests/integration/corrupt-dump.tcl index cc811a6687..fe2b0e4654 100644 --- a/tests/integration/corrupt-dump.tcl +++ b/tests/integration/corrupt-dump.tcl @@ -732,5 +732,14 @@ test {corrupt payload: fuzzer findings - stream double free listpack when insert } } +test {corrupt payload: fuzzer findings - LCS OOM} { + start_server [list overrides [list loglevel verbose use-exit-on-panic yes crash-memcheck-enabled no] ] { + r SETRANGE _int 423324 1450173551 + catch {r LCS _int _int} err + assert_match "*Insufficient memory*" $err + r ping + } +} + } ;# tags