From 16a9e3ad88ccbf85daef7c1e6cf8434ac3420f91 Mon Sep 17 00:00:00 2001
From: Guillem Jover <gjover@sipwise.com>
Date: Fri, 30 Jun 2023 13:23:00 +0200
Subject: [PATCH] MT#57790 Switch medredis_command() to use redisvCommand()

Using redisCommand directly has the problem that it accepts a format
string as its first argument, which means that if we do not escape that
string, it will try parse potentially-looking format arguments, which
can end up accessing garbage on the stack and causing either parse
errors, triggering stack protector checks or injecting parsed garbage
from the stack into the resulting formatted string.

Instead we switch to use redisvCommand() and pass explicitly any
values to be formatted as additional variable arguments. We still
duplicate the formatting to be able to report errors and to have a
minimal fix that can be backported, but that part of the code should
be improved to reduce the useless reformatting.

Analyzed-by: Alessio Garzi <agarzi@sipwise.com>
Change-Id: I7ce3d4450cc93df02778120b9c43ff5984624742
(cherry picked from commit 5f6139aace9d5ed7b89c7d3ed7301f4648253ea2)
---
 medredis.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/medredis.c b/medredis.c
index be0018a..31f198a 100644
--- a/medredis.c
+++ b/medredis.c
@@ -1,5 +1,6 @@
 #include <hiredis/hiredis.h>
 #include <assert.h>
+#include <stdarg.h>
 
 #include "medredis.h"
 #include "medmysql.h"
@@ -135,17 +136,22 @@ err:
 }
 
 /**********************************************************************/
-static redisReply *medredis_command(const char* cmd) {
+static redisReply *medredis_command(const char* cmd, ...) {
+    va_list args;
+
+    va_start(args, cmd);
     L_DEBUG("Performing redis query '%s'\n", cmd);
-    redisReply *reply = redisCommand(con->ctx, cmd);
+    redisReply *reply = redisvCommand(con->ctx, cmd, args);
     if (con->ctx->err == REDIS_ERR_EOF) {
         if (medredis_init() != 0) {
+            va_end(args);
             L_ERROR("Failed to reconnect to redis db\n");
             medredis_cleanup();
             return NULL;
         }
-        reply = redisCommand(con->ctx, cmd);
+        reply = redisvCommand(con->ctx, cmd, args);
     }
+    va_end(args);
     return reply;
 }
 
@@ -547,7 +553,7 @@ gboolean medredis_fetch_callids(GQueue *output) {
 
     do {
         snprintf(buffer, sizeof(buffer), cmd, cursor);
-        reply = medredis_command(buffer);
+        reply = medredis_command(cmd, cursor);
         medredis_check_reply(buffer, reply, err);
 
         if (reply->type != REDIS_REPLY_ARRAY) {
@@ -608,7 +614,7 @@ gboolean medredis_fetch_callids(GQueue *output) {
 
             int truncate_callid = 1;
             snprintf(buffer, sizeof(buffer), cmd_get_dont_clean_suffix, entry->str);
-            redisReply *reply_get_dont_clean_suffix = medredis_command(buffer);
+            redisReply *reply_get_dont_clean_suffix = medredis_command(cmd_get_dont_clean_suffix, entry->str);
             medredis_check_reply(buffer, reply_get_dont_clean_suffix, err);
             if (reply_get_dont_clean_suffix->type == REDIS_REPLY_STRING && reply_get_dont_clean_suffix->str) {
                 if (!strcmp(reply_get_dont_clean_suffix->str,"1")) {