Skip to content

Commit 313941e

Browse files
tianhuaslijinxia
authored andcommitted
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 <tianhuax.s.sun@intel.com> Reviewed-by: Yan, Like <like.yan@intel.com> Reviewed-by: Tao, Yuhong <yuhong.tao@intel.com>
1 parent e24464a commit 313941e

File tree

4 files changed

+129
-46
lines changed

4 files changed

+129
-46
lines changed

tools/acrn-manager/acrn_mngr.c

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -408,8 +408,7 @@ static int connect_to_server(const char *name)
408408
struct mngr_fd *mfd;
409409
int ret;
410410
DIR *dir;
411-
char buf[128] = { };
412-
char *s_name = NULL;
411+
char *s_name = NULL, *p = NULL;
413412
struct dirent *entry;
414413

415414
dir = opendir("/run/acrn/mngr");
@@ -419,11 +418,13 @@ static int connect_to_server(const char *name)
419418
}
420419

421420
while ((entry = readdir(dir))) {
422-
memset(buf, 0, sizeof(buf));
423-
ret = sscanf(entry->d_name, "%[^.]", buf);
424-
if (ret != 1)
421+
p = strchr(entry->d_name, '.');
422+
if (!p || p == entry->d_name)
425423
continue;
426-
if (!strncmp(buf, name, sizeof(buf))) {
424+
else
425+
ret = p - entry->d_name;
426+
427+
if (!strncmp(entry->d_name, name, ret)) {
427428
s_name = entry->d_name;
428429
break;
429430
}

tools/acrn-manager/acrn_vm_ops.c

Lines changed: 69 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include <fcntl.h>
1010
#include <string.h>
1111
#include <errno.h>
12+
#include <limits.h>
1213
#include <pthread.h>
1314
#include <dirent.h>
1415
#include <sys/stat.h>
@@ -82,6 +83,39 @@ static int query_state(const char *name)
8283
return ack.data.state;
8384
}
8485

86+
/*
87+
* get vmname and pid from /run/acrn/mngr/[vmname].monitor.[pid].socket
88+
*/
89+
static inline int _get_vmname_pid(const char *src, char *p_vmname,
90+
int max_len_vmname, int *pid)
91+
{
92+
char *p = NULL;
93+
94+
p = strchr(src, '.');
95+
/* p - src: length of the substring "vmname" in the sting "src" */
96+
if (!p || p - src == 0 || p - src >= max_len_vmname)
97+
return -1;
98+
else
99+
strncpy(p_vmname, src, p - src);
100+
101+
/* move the pointer to the "pid" in the string "src" */
102+
if (strncmp(".monitor.", p, strlen(".monitor.")))
103+
return -1;
104+
else
105+
p = p + strlen(".monitor.");
106+
107+
*pid = strtol(p, NULL, 10);
108+
if ((errno == ERANGE && (*pid == LONG_MAX || *pid == LONG_MIN))
109+
|| (errno != 0 && *pid == 0))
110+
return -1;
111+
112+
p = strchr(p, '.');
113+
if (!p || strncmp(".socket", p, strlen(".socket")))
114+
return -1;
115+
116+
return 0;
117+
}
118+
85119
/* find all the running DM process, which has */
86120
/* /run/acrn/mngr/[vmname].monitor.[pid].socket */
87121
static void _scan_alive_vm(void)
@@ -107,10 +141,8 @@ static void _scan_alive_vm(void)
107141

108142
while ((entry = readdir(dir))) {
109143
memset(name, 0, sizeof(name));
110-
ret =
111-
sscanf(entry->d_name, "%[^.].monitor.%d.socket", name,
112-
&pid);
113-
if (ret != 2)
144+
ret = _get_vmname_pid(entry->d_name, name, sizeof(name), &pid);
145+
if (ret < 0)
114146
continue;
115147

116148
if (name[sizeof(name) - 1]) {
@@ -153,6 +185,34 @@ static void _scan_alive_vm(void)
153185
closedir(dir);
154186
}
155187

188+
/*
189+
* get vmname and suffix from src,
190+
* which has [vmname].[suffix]
191+
*/
192+
static inline int _get_vmname_suffix(const char *src,
193+
char *name, int max_len_name, char *suffix, int max_len_suffix)
194+
{
195+
char *p = NULL;
196+
197+
p = strchr(src, '.');
198+
/* p - src: length of the substring vmname in the string src*/
199+
if (!p || p - src == 0)
200+
return -1;
201+
202+
strncpy(name, src, p - src);
203+
if (p - src >= max_len_name) {
204+
pdebug();
205+
/* truncate name and go a head */
206+
name[max_len_name - 1] = '\0';
207+
}
208+
209+
strncpy(suffix, p + 1, max_len_suffix);
210+
if (strncmp(suffix, "sh", strlen("sh")))
211+
return -1;
212+
213+
return 0;
214+
}
215+
156216
static void _scan_added_vm(void)
157217
{
158218
DIR *dir;
@@ -181,27 +241,17 @@ static void _scan_added_vm(void)
181241
}
182242

183243
while ((entry = readdir(dir))) {
184-
memset(name, 0, sizeof(name));
185-
memset(suffix, 0, sizeof(suffix));
186-
187244
ret = strnlen(entry->d_name, sizeof(entry->d_name));
188245
if (ret >= sizeof(name)) {
189246
pdebug();
190247
continue;
191248
}
192249

193-
ret = sscanf(entry->d_name, "%[^.].%s", name, suffix);
194-
195-
if (ret != 2)
196-
continue;
197-
198-
if (name[sizeof(name) - 1]) {
199-
pdebug();
200-
/* truncate name and go a head */
201-
name[sizeof(name) - 1] = 0;
202-
}
203-
204-
if (strncmp(suffix, "sh", sizeof("sh")))
250+
memset(name, 0, sizeof(name));
251+
memset(suffix, 0, sizeof(suffix));
252+
ret = _get_vmname_suffix(entry->d_name,
253+
name, sizeof(name), suffix, sizeof(suffix));
254+
if (ret < 0)
205255
continue;
206256

207257
vm = vmmngr_find(name);

tools/acrn-manager/acrnctl.c

Lines changed: 38 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,33 @@ static int write_tmp_file(int fd, int n, char *word[])
134134
return 0;
135135
}
136136

137+
/*
138+
* get vmname from the string src, and src
139+
* format is "acrnctl: [vmname]"
140+
*/
141+
static inline int _get_vmname(const char *src, char *vmname, int max_len_vmname)
142+
{
143+
const char *vmname_p = NULL;
144+
145+
if (!strncmp("acrnctl: ", src, strlen("acrnctl: "))) {
146+
vmname_p = src + strlen("acrnctl: ");
147+
148+
memset(vmname, 0, max_len_vmname);
149+
strncpy(vmname, vmname_p, max_len_vmname);
150+
if(vmname[max_len_vmname - 1]) {
151+
/* vmname is truncated */
152+
printf("get vmname failed, vmname is truncated\n");
153+
return -1;
154+
}
155+
} else {
156+
/* the prefix of the string "src" isn't "acrnctl: " */
157+
printf("can't found prefix 'acrnctl: '\n");
158+
return -1;
159+
}
160+
161+
return 0;
162+
}
163+
137164
#define MAX_FILE_SIZE (4096 * 4)
138165
#define FILE_NAME_LENGTH 128
139166

@@ -150,7 +177,7 @@ static int acrnctl_do_add(int argc, char *argv[])
150177
char fname[FILE_NAME_LENGTH + sizeof(TMP_FILE_SUFFIX)];
151178
char cmd[128];
152179
char args[128];
153-
int p, i;
180+
int p, i, len_cmd_out = 0;
154181
char cmd_out[256];
155182
char vmname[128];
156183
size_t len = sizeof(cmd_out);
@@ -253,7 +280,6 @@ static int acrnctl_do_add(int argc, char *argv[])
253280
}
254281
system(cmd);
255282

256-
memset(vmname, 0, sizeof(vmname));
257283
if (snprintf(cmd, sizeof(cmd), "bash %s%s >./%s.result", argv[1],
258284
args, argv[1]) >= sizeof(cmd)) {
259285
printf("ERROR: cmd is truncated\n");
@@ -270,14 +296,18 @@ static int acrnctl_do_add(int argc, char *argv[])
270296
ret = -1;
271297
goto get_vmname;
272298
}
273-
ret = shell_cmd(cmd, cmd_out, sizeof(cmd_out));
274-
if (ret < 0)
299+
len_cmd_out = shell_cmd(cmd, cmd_out, sizeof(cmd_out));
300+
if (len_cmd_out < 0) {
301+
ret = len_cmd_out;
275302
goto get_vmname;
303+
}
276304

277-
ret = sscanf(cmd_out, "acrnctl: %s", vmname);
278-
if (ret != 1) {
279-
ret = -1;
305+
if(cmd_out[len_cmd_out - 1] == '\n')
306+
cmd_out[len_cmd_out - 1] = '\0';
280307

308+
ret = _get_vmname(cmd_out, vmname, sizeof(vmname));
309+
if (ret < 0) {
310+
/* failed to get vmname */
281311
if (snprintf(cmd, sizeof(cmd), "cat ./%s.result", argv[1]) >= sizeof(cmd)) {
282312
printf("ERROR: cmd is truncated\n");
283313
goto get_vmname;
@@ -538,7 +568,7 @@ static int acrnctl_do_resume(int argc, char *argv[])
538568
}
539569

540570
if (argc == 3) {
541-
sscanf(argv[2], "%x", &reason);
571+
reason = strtoul(argv[2], NULL, 16);
542572
reason = (reason & (0xff << 24)) ? 0 : reason;
543573
} else
544574
printf("No wake up reason, use 0x%x\n", reason);

tools/acrn-manager/acrnd.c

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -153,8 +153,7 @@ static int load_timer_list(void)
153153
struct work_arg arg = {};
154154
time_t expire, current;
155155
char l[256];
156-
char s1[16], s2[64]; /* vmname & expire */
157-
int ret = 0;
156+
int i, ret = 0;
158157

159158
pthread_mutex_lock(&timer_file_mutex);
160159

@@ -173,20 +172,23 @@ static int load_timer_list(void)
173172
continue;
174173
}
175174

176-
memset(s1, 0, 16);
177-
memset(s2, 0, 64);
178-
179-
sscanf(l, "%s\t%s", s1, s2);
175+
/* get vmname from the string "l", which has "[vmname]\t[expire]" */
176+
for (i = 0; i < sizeof(arg.name); i++) {
177+
if (l[i] == '\t') {
178+
arg.name[i] = '\0';
179+
break;
180+
}
181+
arg.name[i] = l[i];
182+
}
180183

181-
if (strlen(s1) == 0 || strlen(s1) > 16) {
182-
fprintf(stderr, "Invalid vmname %s from timer list file\n", s1);
184+
/* can't found vmname in the string "l" or vmname is truncated */
185+
if (i == 0 || i == sizeof(arg.name)) {
186+
fprintf(stderr, "Invalid vmname %s from timer list file\n", arg.name);
183187
continue;
184188
}
185189

186-
memset(arg.name, 0, sizeof(arg.name));
187-
strncpy(arg.name, s1, sizeof(arg.name));
188-
189-
expire = strtoul(s2, NULL, 10);
190+
/* get expire from the string "l" */
191+
expire = strtoul(&l[i + 1], NULL, 10);
190192
if (expire == 0 || errno == ERANGE) {
191193
perror("Invalid expire from timer list file");
192194
continue;
@@ -202,7 +204,7 @@ static int load_timer_list(void)
202204
if (ret != 0) {
203205
fprintf(stderr, "Failed to add vm timer, errno %d", ret);
204206
} else {
205-
printf("vm %s will be activated at %ld seconds later\n", s1, expire);
207+
printf("vm %s will be activated at %ld seconds later\n", arg.name, expire);
206208
}
207209
}
208210

0 commit comments

Comments
 (0)