file: rpc_file_exec_run: fix potential memory leak and integer overflow

- 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>
This commit is contained in:
Jo-Philipp Wich 2018-12-21 08:50:36 +01:00
parent 3aa81d0dfa
commit e5243c16eb

20
file.c
View file

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