file: rpc_file_exec_run: fix potential memory leak and integer overflow
authorJo-Philipp Wich <jo@mein.io>
Fri, 21 Dec 2018 07:50:36 +0000 (08:50 +0100)
committerJo-Philipp Wich <jo@mein.io>
Fri, 21 Dec 2018 08:00:20 +0000 (09:00 +0100)
 - Store the realloc result in a separate pointer so that we can free
   the original on allocation failure
 - Use an explicit uint8_t for the argument vector length instead of
   "char" which might be signed or unsigned, depending on the arch
 - Bail out with an invalid argument error if the argument vector
   exceeds 255 items

Signed-off-by: Jo-Philipp Wich <jo@mein.io>
file.c

diff --git a/file.c b/file.c
index 488259878bc528f7b26bd9804f3f1cdd1db20c5c..04be6081d8e668947f12569aa06fee7ea20319ff 100644 (file)
--- a/file.c
+++ b/file.c
@@ -20,6 +20,7 @@
 #include <fcntl.h>
 #include <errno.h>
 #include <unistd.h>
+#include <stdint.h>
 #include <stdlib.h>
 #include <string.h>
 #include <limits.h>
@@ -326,7 +327,7 @@ static int
 rpc_file_md5(struct ubus_context *ctx, struct ubus_object *obj,
              struct ubus_request_data *req, const char *method,
              struct blob_attr *msg)
-{ 
+{
        int rv, i;
        char *path;
        struct stat s;
@@ -606,8 +607,8 @@ rpc_file_exec_run(const char *cmd,
        int rem;
        struct blob_attr *cur;
 
-       char arglen;
-       char **args;
+       uint8_t arglen;
+       char **args, **tmp;
 
        struct rpc_file_exec_context *c;
 
@@ -657,11 +658,22 @@ rpc_file_exec_run(const char *cmd,
                                if (blobmsg_type(cur) != BLOBMSG_TYPE_STRING)
                                        continue;
 
+                               if (arglen == 255)
+                               {
+                                       free(args);
+                                       return UBUS_STATUS_INVALID_ARGUMENT;
+                               }
+
                                arglen++;
+                               tmp = realloc(args, sizeof(char *) * arglen);
 
-                               if (!(args = realloc(args, sizeof(char *) * arglen)))
+                               if (!tmp)
+                               {
+                                       free(args);
                                        return UBUS_STATUS_UNKNOWN_ERROR;
+                               }
 
+                               args = tmp;
                                args[arglen-2] = blobmsg_data(cur);
                                args[arglen-1] = NULL;
                        }