CVE-2022-3647: Improper Resource Shutdown or Release (CWE-404)
value: ** DISPUTED ** A vulnerability, which was classified as problematic, was found in Redis up to 6.2.7/7.0.5. Affected is the function sigsegvHandler of the file debug.c of the component Crash Report. The manipulation leads to denial of service. The complexity of an attack is rather high. The exploitability is told to be difficult. The real existence of this vulnerability is still doubted at the moment. Upgrading to version 6.2.8 and 7.0.6 is able to address this issue. The patch is identified as 0bf90d944313919eb8e63d3588bf63a367f020a3. It is recommended to apply a patch to fix this issue. VDB-211962 is the identifier assigned to this vulnerability. NOTE: The vendor claims that this is not a DoS because it applies to the crash logging mechanism which is triggered after a crash has occurred.
CWE description: The product does not release or incorrectly releases a resource before it is made available for re-use.
CVSS v3 base score: 3.3
Published: 2022-10-21T18:15Z
Repository: https://github.com/redis/redis
Language: C
Files changed: src/debug.c
Commit date: 2022-09-29 08:58:58 +0300
Commit message: Avoid crash on crash report when a bad function pointer was called (#11298)

If Redis crashes due to calling an invalid function pointer,
the `backtrace` function will try to dereference this invalid pointer
which will cause a crash inside the crash report and will kill
the processes without having all the crash report information.

Example:

```
=== REDIS BUG REPORT START: Cut & paste starting from here ===
198672:M 19 Sep 2022 18:06:12.936 # Redis 255.255.255 crashed by signal: 11, si_code: 1
198672:M 19 Sep 2022 18:06:12.936 # Accessing address: 0x1
198672:M 19 Sep 2022 18:06:12.936 # Crashed running the instruction at: 0x1
// here the processes is crashing
```

This PR tries to fix this crash be:
1. Identify the issue when it happened.
2. Replace the invalid pointer with a pointer to some dummy function
   so that `backtrace` will not crash.

I identification is done by comparing `eip` to `info->si_addr`, if they
are the same we know that the crash happened on the same address it tries to
accesses and we can conclude that it tries to call and invalid function pointer.

To replace the invalid pointer we introduce a new function, `setMcontextEip`,
which is very similar to `getMcontextEip` and it knows to set the Eip for the
different supported OS's. After printing the trace we retrieve the old `Eip` value.
Diff: diff --git a/src/debug.c b/src/debug.c
index 8cc811be44..b15ac8780d 100644
--- a/src/debug.c
+++ b/src/debug.c
@@ -1121,77 +1121,92 @@ void bugReportStart(void) {
     }
     pthread_mutex_unlock(&bug_report_start_mutex);
 }
 
 #ifdef HAVE_BACKTRACE
-static void *getMcontextEip(ucontext_t *uc) {
+
+/* Returns the current eip and set it to the given new value (if its not NULL) */
+static void* getAndSetMcontextEip(ucontext_t *uc, void *eip) {
 #define NOT_SUPPORTED() do {\
     UNUSED(uc);\
+    UNUSED(eip);\
     return NULL;\
 } while(0)
+#define GET_SET_RETURN(target_var, new_val) do {\
+    void *old_val = (void*)target_var; \
+    if (new_val) { \
+        void **temp = (void**)&target_var; \
+        *temp = new_val; \
+    } \
+    return old_val; \
+} while(0)
 #if defined(__APPLE__) && !defined(MAC_OS_X_VERSION_10_6)
     /* OSX < 10.6 */
     #if defined(__x86_64__)
-    return (void*) uc->uc_mcontext->__ss.__rip;
+    GET_SET_RETURN(uc->uc_mcontext->__ss.__rip, eip);
     #elif defined(__i386__)
-    return (void*) uc->uc_mcontext->__ss.__eip;
+    GET_SET_RETURN(uc->uc_mcontext->__ss.__eip, eip);
     #else
-    return (void*) uc->uc_mcontext->__ss.__srr0;
+    GET_SET_RETURN(uc->uc_mcontext->__ss.__srr0, eip);
     #endif
 #elif defined(__APPLE__) && defined(MAC_OS_X_VERSION_10_6)
     /* OSX >= 10.6 */
     #if defined(_STRUCT_X86_THREAD_STATE64) && !defined(__i386__)
-    return (void*) uc->uc_mcontext->__ss.__rip;
+    GET_SET_RETURN(uc->uc_mcontext->__ss.__rip, eip);
     #elif defined(__i386__)
-    return (void*) uc->uc_mcontext->__ss.__eip;
+    GET_SET_RETURN(uc->uc_mcontext->__ss.__eip, eip);
     #else
     /* OSX ARM64 */
-    return (void*) arm_thread_state64_get_pc(uc->uc_mcontext->__ss);
+    void *old_val = (void*)arm_thread_state64_get_pc(uc->uc_mcontext->__ss);
+    if (eip) {
+        arm_thread_state64_set_pc_fptr(uc->uc_mcontext->__ss, eip);
+    }
+    return old_val;
     #endif
 #elif defined(__linux__)
     /* Linux */
     #if defined(__i386__) || ((defined(__X86_64__) || defined(__x86_64__)) && defined(__ILP32__))
-    return (void*) uc->uc_mcontext.gregs[14]; /* Linux 32 */
+    GET_SET_RETURN(uc->uc_mcontext.gregs[14], eip);
     #elif defined(__X86_64__) || defined(__x86_64__)
-    return (void*) uc->uc_mcontext.gregs[16]; /* Linux 64 */
+    GET_SET_RETURN(uc->uc_mcontext.gregs[16], eip);
     #elif defined(__ia64__) /* Linux IA64 */
-    return (void*) uc->uc_mcontext.sc_ip;
+    GET_SET_RETURN(uc->uc_mcontext.sc_ip, eip);
     #elif defined(__arm__) /* Linux ARM */
-    return (void*) uc->uc_mcontext.arm_pc;
+    GET_SET_RETURN(uc->uc_mcontext.arm_pc, eip);
     #elif defined(__aarch64__) /* Linux AArch64 */
-    return (void*) uc->uc_mcontext.pc;
+    GET_SET_RETURN(uc->uc_mcontext.pc, eip);
     #else
     NOT_SUPPORTED();
     #endif
 #elif defined(__FreeBSD__)
     /* FreeBSD */
     #if defined(__i386__)
-    return (void*) uc->uc_mcontext.mc_eip;
+    GET_SET_RETURN(uc->uc_mcontext.mc_eip, eip);
     #elif defined(__x86_64__)
-    return (void*) uc->uc_mcontext.mc_rip;
+    GET_SET_RETURN(uc->uc_mcontext.mc_rip, eip);
     #else
     NOT_SUPPORTED();
     #endif
 #elif defined(__OpenBSD__)
     /* OpenBSD */
     #if defined(__i386__)
-    return (void*) uc->sc_eip;
+    GET_SET_RETURN(uc->sc_eip, eip);
     #elif defined(__x86_64__)
-    return (void*) uc->sc_rip;
+    GET_SET_RETURN(uc->sc_rip, eip);
     #else
     NOT_SUPPORTED();
     #endif
 #elif defined(__NetBSD__)
     #if defined(__i386__)
-    return (void*) uc->uc_mcontext.__gregs[_REG_EIP];
+    GET_SET_RETURN(uc->uc_mcontext.__gregs[_REG_EIP], eip);
     #elif defined(__x86_64__)
-    return (void*) uc->uc_mcontext.__gregs[_REG_RIP];
+    GET_SET_RETURN(uc->uc_mcontext.__gregs[_REG_RIP], eip);
     #else
     NOT_SUPPORTED();
     #endif
 #elif defined(__DragonFly__)
-    return (void*) uc->uc_mcontext.mc_rip;
+    GET_SET_RETURN(uc->uc_mcontext.mc_rip, eip);
 #else
     NOT_SUPPORTED();
 #endif
 #undef NOT_SUPPORTED
 }
@@ -1949,10 +1964,14 @@ void dumpCodeAroundEIP(void *eip) {
             dumpX86Calls(base, len);
         }
     }
 }
 
+void invalidFunctionWasCalled() {}
+
+typedef void (*invalidFunctionWasCalledType)();
+
 void sigsegvHandler(int sig, siginfo_t *info, void *secret) {
     UNUSED(secret);
     UNUSED(info);
 
     bugReportStart();
@@ -1966,17 +1985,34 @@ void sigsegvHandler(int sig, siginfo_t *info, void *secret) {
         serverLog(LL_WARNING, "Killed by PID: %ld, UID: %d", (long) info->si_pid, info->si_uid);
     }
 
 #ifdef HAVE_BACKTRACE
     ucontext_t *uc = (ucontext_t*) secret;
-    void *eip = getMcontextEip(uc);
+    void *eip = getAndSetMcontextEip(uc, NULL);
     if (eip != NULL) {
         serverLog(LL_WARNING,
         "Crashed running the instruction at: %p", eip);
     }
 
-    logStackTrace(getMcontextEip(uc), 1);
+    if (eip == info->si_addr) {
+        /* When eip matches the bad address, it's an indication that we crashed when calling a non-mapped
+         * function pointer. In that case the call to backtrace will crash trying to access that address and we
+         * won't get a crash report logged. Set it to a valid point to avoid that crash. */
+
+        /* This trick allow to avoid compiler warning */
+        void *ptr;
+        invalidFunctionWasCalledType *ptr_ptr = (invalidFunctionWasCalledType*)&ptr;
+        *ptr_ptr = invalidFunctionWasCalled;
+        getAndSetMcontextEip(uc, ptr);
+    }
+
+    logStackTrace(eip, 1);
+
+    if (eip == info->si_addr) {
+        /* Restore old eip */
+        getAndSetMcontextEip(uc, eip);
+    }
 
     logRegisters(uc);
 #endif
 
     printCrashReport();
@@ -2077,11 +2113,11 @@ void watchdogSignalHandler(int sig, siginfo_t *info, void *secret) {
     UNUSED(info);
     UNUSED(sig);
 
     serverLogFromHandler(LL_WARNING,"\n--- WATCHDOG TIMER EXPIRED ---");
 #ifdef HAVE_BACKTRACE
-    logStackTrace(getMcontextEip(uc), 1);
+    logStackTrace(getAndSetMcontextEip(uc, NULL), 1);
 #else
     serverLogFromHandler(LL_WARNING,"Sorry: no support for backtrace().");
 #endif
     serverLogFromHandler(LL_WARNING,"--------\n");
 }

Vulnerable code: static void *getMcontextEip(ucontext_t *uc) {
    return (void*) uc->uc_mcontext->__ss.__rip;
    return (void*) uc->uc_mcontext->__ss.__eip;
    return (void*) uc->uc_mcontext->__ss.__srr0;
    return (void*) uc->uc_mcontext->__ss.__rip;
    return (void*) uc->uc_mcontext->__ss.__eip;
    return (void*) arm_thread_state64_get_pc(uc->uc_mcontext->__ss);
    return (void*) uc->uc_mcontext.gregs[14]; /* Linux 32 */
    return (void*) uc->uc_mcontext.gregs[16]; /* Linux 64 */
    return (void*) uc->uc_mcontext.sc_ip;
    return (void*) uc->uc_mcontext.arm_pc;
    return (void*) uc->uc_mcontext.pc;
    return (void*) uc->uc_mcontext.mc_eip;
    return (void*) uc->uc_mcontext.mc_rip;
    return (void*) uc->sc_eip;
    return (void*) uc->sc_rip;
    return (void*) uc->uc_mcontext.__gregs[_REG_EIP];
    return (void*) uc->uc_mcontext.__gregs[_REG_RIP];
    return (void*) uc->uc_mcontext.mc_rip;
    void *eip = getMcontextEip(uc);
    logStackTrace(getMcontextEip(uc), 1);
    logStackTrace(getMcontextEip(uc), 1);
Fixed code: 
/* Returns the current eip and set it to the given new value (if its not NULL) */
static void* getAndSetMcontextEip(ucontext_t *uc, void *eip) {
    UNUSED(eip);\
#define GET_SET_RETURN(target_var, new_val) do {\
    void *old_val = (void*)target_var; \
    if (new_val) { \
        void **temp = (void**)&target_var; \
        *temp = new_val; \
    } \
    return old_val; \
} while(0)
    GET_SET_RETURN(uc->uc_mcontext->__ss.__rip, eip);
    GET_SET_RETURN(uc->uc_mcontext->__ss.__eip, eip);
    GET_SET_RETURN(uc->uc_mcontext->__ss.__srr0, eip);
    GET_SET_RETURN(uc->uc_mcontext->__ss.__rip, eip);
    GET_SET_RETURN(uc->uc_mcontext->__ss.__eip, eip);
    void *old_val = (void*)arm_thread_state64_get_pc(uc->uc_mcontext->__ss);
    if (eip) {
        arm_thread_state64_set_pc_fptr(uc->uc_mcontext->__ss, eip);
    }
    return old_val;
    GET_SET_RETURN(uc->uc_mcontext.gregs[14], eip);
    GET_SET_RETURN(uc->uc_mcontext.gregs[16], eip);
    GET_SET_RETURN(uc->uc_mcontext.sc_ip, eip);
    GET_SET_RETURN(uc->uc_mcontext.arm_pc, eip);
    GET_SET_RETURN(uc->uc_mcontext.pc, eip);
    GET_SET_RETURN(uc->uc_mcontext.mc_eip, eip);
    GET_SET_RETURN(uc->uc_mcontext.mc_rip, eip);
    GET_SET_RETURN(uc->sc_eip, eip);
    GET_SET_RETURN(uc->sc_rip, eip);
    GET_SET_RETURN(uc->uc_mcontext.__gregs[_REG_EIP], eip);
    GET_SET_RETURN(uc->uc_mcontext.__gregs[_REG_RIP], eip);
    GET_SET_RETURN(uc->uc_mcontext.mc_rip, eip);
void invalidFunctionWasCalled() {}

typedef void (*invalidFunctionWasCalledType)();

    void *eip = getAndSetMcontextEip(uc, NULL);
    if (eip == info->si_addr) {
        /* When eip matches the bad address, it's an indication that we crashed when calling a non-mapped
         * function pointer. In that case the call to backtrace will crash trying to access that address and we
         * won't get a crash report logged. Set it to a valid point to avoid that crash. */

        /* This trick allow to avoid compiler warning */
        void *ptr;
        invalidFunctionWasCalledType *ptr_ptr = (invalidFunctionWasCalledType*)&ptr;
        *ptr_ptr = invalidFunctionWasCalled;
        getAndSetMcontextEip(uc, ptr);
    }

    logStackTrace(eip, 1);

    if (eip == info->si_addr) {
        /* Restore old eip */
        getAndSetMcontextEip(uc, eip);
    }
    logStackTrace(getAndSetMcontextEip(uc, NULL), 1);