From 2a189709e098139b3158575ee6cb6d7ff266ed45 Mon Sep 17 00:00:00 2001 From: Oran Agra Date: Mon, 24 Mar 2025 12:24:52 +0200 Subject: [PATCH] avoid possible use-after-free with module KSN changes (#13875) in #13505, we changed the code to use the string value of the key rather than the integer value on the stack, but we have a test in unit/moduleapi/keyspace_events that uses keyspace notification hook to modify the value with RM_StringDMA, which can cause this value to be released before used. the reason it didn't happen so far is because we were using shared integers, so releasing the object doesn't free it. --- src/t_string.c | 2 +- tests/unit/moduleapi/keyspace_events.tcl | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/t_string.c b/src/t_string.c index 04eb794123..67b362d1fa 100644 --- a/src/t_string.c +++ b/src/t_string.c @@ -612,10 +612,10 @@ void incrDecrCommand(client *c, long long incr) { dbAdd(c->db,c->argv[1],new); } } + addReplyLongLongFromStr(c,new); signalModifiedKey(c,c->db,c->argv[1]); notifyKeyspaceEvent(NOTIFY_STRING,"incrby",c->argv[1],c->db->id); server.dirty++; - addReplyLongLongFromStr(c,new); } void incrCommand(client *c) { diff --git a/tests/unit/moduleapi/keyspace_events.tcl b/tests/unit/moduleapi/keyspace_events.tcl index 1323b12966..30fdc1e2b6 100644 --- a/tests/unit/moduleapi/keyspace_events.tcl +++ b/tests/unit/moduleapi/keyspace_events.tcl @@ -3,6 +3,10 @@ set testmodule [file normalize tests/modules/keyspace_events.so] tags "modules" { start_server [list overrides [list loadmodule "$testmodule"]] { + # avoid using shared integers, to increase the chance of detection heap issues + r config set maxmemory-policy allkeys-lru + r config set maxmemory 1gb + test {Test loaded key space event} { r set x 1 r hset y f v