From 313941e853ac5c3e49e95d3b3e61c1f0ec041695 Mon Sep 17 00:00:00 2001 From: Tianhua Sun Date: Tue, 30 Oct 2018 15:54:06 +0800 Subject: [PATCH] tools: acrn-manager: remove unsafe api sscanf function sscanf is banned according to the security requirements. So remove sscanf api. Tracked-On: #1254 Signed-off-by: Tianhua Sun Reviewed-by: Yan, Like Reviewed-by: Tao, Yuhong --- tools/acrn-manager/acrn_mngr.c | 13 ++--- tools/acrn-manager/acrn_vm_ops.c | 88 +++++++++++++++++++++++++------- tools/acrn-manager/acrnctl.c | 46 ++++++++++++++--- tools/acrn-manager/acrnd.c | 28 +++++----- 4 files changed, 129 insertions(+), 46 deletions(-) diff --git a/tools/acrn-manager/acrn_mngr.c b/tools/acrn-manager/acrn_mngr.c index d05b6ebfa..1a7364a0a 100644 --- a/tools/acrn-manager/acrn_mngr.c +++ b/tools/acrn-manager/acrn_mngr.c @@ -408,8 +408,7 @@ static int connect_to_server(const char *name) struct mngr_fd *mfd; int ret; DIR *dir; - char buf[128] = { }; - char *s_name = NULL; + char *s_name = NULL, *p = NULL; struct dirent *entry; dir = opendir("/run/acrn/mngr"); @@ -419,11 +418,13 @@ static int connect_to_server(const char *name) } while ((entry = readdir(dir))) { - memset(buf, 0, sizeof(buf)); - ret = sscanf(entry->d_name, "%[^.]", buf); - if (ret != 1) + p = strchr(entry->d_name, '.'); + if (!p || p == entry->d_name) continue; - if (!strncmp(buf, name, sizeof(buf))) { + else + ret = p - entry->d_name; + + if (!strncmp(entry->d_name, name, ret)) { s_name = entry->d_name; break; } diff --git a/tools/acrn-manager/acrn_vm_ops.c b/tools/acrn-manager/acrn_vm_ops.c index 867ed45d1..3625735b2 100644 --- a/tools/acrn-manager/acrn_vm_ops.c +++ b/tools/acrn-manager/acrn_vm_ops.c @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -82,6 +83,39 @@ static int query_state(const char *name) return ack.data.state; } +/* + * get vmname and pid from /run/acrn/mngr/[vmname].monitor.[pid].socket + */ +static inline int _get_vmname_pid(const char *src, char *p_vmname, + int max_len_vmname, int *pid) +{ + char *p = NULL; + + p = strchr(src, '.'); + /* p - src: length of the substring "vmname" in the sting "src" */ + if (!p || p - src == 0 || p - src >= max_len_vmname) + return -1; + else + strncpy(p_vmname, src, p - src); + + /* move the pointer to the "pid" in the string "src" */ + if (strncmp(".monitor.", p, strlen(".monitor."))) + return -1; + else + p = p + strlen(".monitor."); + + *pid = strtol(p, NULL, 10); + if ((errno == ERANGE && (*pid == LONG_MAX || *pid == LONG_MIN)) + || (errno != 0 && *pid == 0)) + return -1; + + p = strchr(p, '.'); + if (!p || strncmp(".socket", p, strlen(".socket"))) + return -1; + + return 0; +} + /* find all the running DM process, which has */ /* /run/acrn/mngr/[vmname].monitor.[pid].socket */ static void _scan_alive_vm(void) @@ -107,10 +141,8 @@ static void _scan_alive_vm(void) while ((entry = readdir(dir))) { memset(name, 0, sizeof(name)); - ret = - sscanf(entry->d_name, "%[^.].monitor.%d.socket", name, - &pid); - if (ret != 2) + ret = _get_vmname_pid(entry->d_name, name, sizeof(name), &pid); + if (ret < 0) continue; if (name[sizeof(name) - 1]) { @@ -153,6 +185,34 @@ static void _scan_alive_vm(void) closedir(dir); } +/* + * get vmname and suffix from src, + * which has [vmname].[suffix] + */ +static inline int _get_vmname_suffix(const char *src, + char *name, int max_len_name, char *suffix, int max_len_suffix) +{ + char *p = NULL; + + p = strchr(src, '.'); + /* p - src: length of the substring vmname in the string src*/ + if (!p || p - src == 0) + return -1; + + strncpy(name, src, p - src); + if (p - src >= max_len_name) { + pdebug(); + /* truncate name and go a head */ + name[max_len_name - 1] = '\0'; + } + + strncpy(suffix, p + 1, max_len_suffix); + if (strncmp(suffix, "sh", strlen("sh"))) + return -1; + + return 0; +} + static void _scan_added_vm(void) { DIR *dir; @@ -181,27 +241,17 @@ static void _scan_added_vm(void) } while ((entry = readdir(dir))) { - memset(name, 0, sizeof(name)); - memset(suffix, 0, sizeof(suffix)); - ret = strnlen(entry->d_name, sizeof(entry->d_name)); if (ret >= sizeof(name)) { pdebug(); continue; } - ret = sscanf(entry->d_name, "%[^.].%s", name, suffix); - - if (ret != 2) - continue; - - if (name[sizeof(name) - 1]) { - pdebug(); - /* truncate name and go a head */ - name[sizeof(name) - 1] = 0; - } - - if (strncmp(suffix, "sh", sizeof("sh"))) + memset(name, 0, sizeof(name)); + memset(suffix, 0, sizeof(suffix)); + ret = _get_vmname_suffix(entry->d_name, + name, sizeof(name), suffix, sizeof(suffix)); + if (ret < 0) continue; vm = vmmngr_find(name); diff --git a/tools/acrn-manager/acrnctl.c b/tools/acrn-manager/acrnctl.c index 6e2125c8a..ac73ed615 100644 --- a/tools/acrn-manager/acrnctl.c +++ b/tools/acrn-manager/acrnctl.c @@ -134,6 +134,33 @@ static int write_tmp_file(int fd, int n, char *word[]) return 0; } +/* + * get vmname from the string src, and src + * format is "acrnctl: [vmname]" + */ +static inline int _get_vmname(const char *src, char *vmname, int max_len_vmname) +{ + const char *vmname_p = NULL; + + if (!strncmp("acrnctl: ", src, strlen("acrnctl: "))) { + vmname_p = src + strlen("acrnctl: "); + + memset(vmname, 0, max_len_vmname); + strncpy(vmname, vmname_p, max_len_vmname); + if(vmname[max_len_vmname - 1]) { + /* vmname is truncated */ + printf("get vmname failed, vmname is truncated\n"); + return -1; + } + } else { + /* the prefix of the string "src" isn't "acrnctl: " */ + printf("can't found prefix 'acrnctl: '\n"); + return -1; + } + + return 0; +} + #define MAX_FILE_SIZE (4096 * 4) #define FILE_NAME_LENGTH 128 @@ -150,7 +177,7 @@ static int acrnctl_do_add(int argc, char *argv[]) char fname[FILE_NAME_LENGTH + sizeof(TMP_FILE_SUFFIX)]; char cmd[128]; char args[128]; - int p, i; + int p, i, len_cmd_out = 0; char cmd_out[256]; char vmname[128]; size_t len = sizeof(cmd_out); @@ -253,7 +280,6 @@ static int acrnctl_do_add(int argc, char *argv[]) } system(cmd); - memset(vmname, 0, sizeof(vmname)); if (snprintf(cmd, sizeof(cmd), "bash %s%s >./%s.result", argv[1], args, argv[1]) >= sizeof(cmd)) { printf("ERROR: cmd is truncated\n"); @@ -270,14 +296,18 @@ static int acrnctl_do_add(int argc, char *argv[]) ret = -1; goto get_vmname; } - ret = shell_cmd(cmd, cmd_out, sizeof(cmd_out)); - if (ret < 0) + len_cmd_out = shell_cmd(cmd, cmd_out, sizeof(cmd_out)); + if (len_cmd_out < 0) { + ret = len_cmd_out; goto get_vmname; + } - ret = sscanf(cmd_out, "acrnctl: %s", vmname); - if (ret != 1) { - ret = -1; + if(cmd_out[len_cmd_out - 1] == '\n') + cmd_out[len_cmd_out - 1] = '\0'; + ret = _get_vmname(cmd_out, vmname, sizeof(vmname)); + if (ret < 0) { + /* failed to get vmname */ if (snprintf(cmd, sizeof(cmd), "cat ./%s.result", argv[1]) >= sizeof(cmd)) { printf("ERROR: cmd is truncated\n"); goto get_vmname; @@ -538,7 +568,7 @@ static int acrnctl_do_resume(int argc, char *argv[]) } if (argc == 3) { - sscanf(argv[2], "%x", &reason); + reason = strtoul(argv[2], NULL, 16); reason = (reason & (0xff << 24)) ? 0 : reason; } else printf("No wake up reason, use 0x%x\n", reason); diff --git a/tools/acrn-manager/acrnd.c b/tools/acrn-manager/acrnd.c index bd60ae411..de4a9db53 100644 --- a/tools/acrn-manager/acrnd.c +++ b/tools/acrn-manager/acrnd.c @@ -153,8 +153,7 @@ static int load_timer_list(void) struct work_arg arg = {}; time_t expire, current; char l[256]; - char s1[16], s2[64]; /* vmname & expire */ - int ret = 0; + int i, ret = 0; pthread_mutex_lock(&timer_file_mutex); @@ -173,20 +172,23 @@ static int load_timer_list(void) continue; } - memset(s1, 0, 16); - memset(s2, 0, 64); + /* get vmname from the string "l", which has "[vmname]\t[expire]" */ + for (i = 0; i < sizeof(arg.name); i++) { + if (l[i] == '\t') { + arg.name[i] = '\0'; + break; + } + arg.name[i] = l[i]; + } - sscanf(l, "%s\t%s", s1, s2); - - if (strlen(s1) == 0 || strlen(s1) > 16) { - fprintf(stderr, "Invalid vmname %s from timer list file\n", s1); + /* can't found vmname in the string "l" or vmname is truncated */ + if (i == 0 || i == sizeof(arg.name)) { + fprintf(stderr, "Invalid vmname %s from timer list file\n", arg.name); continue; } - memset(arg.name, 0, sizeof(arg.name)); - strncpy(arg.name, s1, sizeof(arg.name)); - - expire = strtoul(s2, NULL, 10); + /* get expire from the string "l" */ + expire = strtoul(&l[i + 1], NULL, 10); if (expire == 0 || errno == ERANGE) { perror("Invalid expire from timer list file"); continue; @@ -202,7 +204,7 @@ static int load_timer_list(void) if (ret != 0) { fprintf(stderr, "Failed to add vm timer, errno %d", ret); } else { - printf("vm %s will be activated at %ld seconds later\n", s1, expire); + printf("vm %s will be activated at %ld seconds later\n", arg.name, expire); } }