-smbios type=11,path=xxx results in buffer overrun due to missing null terminator
In hw/smbios/smbios.c, there's the following code:
if (g_str_equal(name, "path")) {
g_autoptr(GByteArray) data = g_byte_array_new();
g_autofree char *buf = g_new(char, 4096);
ssize_t ret;
int fd = qemu_open(value, O_RDONLY, errp);
if (fd < 0) {
return -1;
}
while (1) {
ret = read(fd, buf, 4096);
if (ret == 0) {
break;
}
if (ret < 0) {
error_setg(errp, "Unable to read from %s: %s",
value, strerror(errno));
qemu_close(fd);
return -1;
}
if (memchr(buf, '\0', ret)) {
error_setg(errp, "NUL in OEM strings value in %s", value);
qemu_close(fd);
return -1;
}
g_byte_array_append(data, (guint8 *)buf, ret);
}
qemu_close(fd);
*opt->dest = g_renew(char *, *opt->dest, (*opt->ndest) + 1);
(*opt->dest)[*opt->ndest] = (char *)g_byte_array_free(data, FALSE);
(*opt->ndest)++;
data = NULL;
}
This leads to the buffer that's assigned to *opt->dest being not null terminated, which leads to a buffer overrun elsewhere in qemu. I believe a null terminator should be added after parsing is finished like follows:
if (g_str_equal(name, "path")) {
g_autoptr(GByteArray) data = g_byte_array_new();
g_autofree char *buf = g_new(char, 4096);
ssize_t ret;
int fd = qemu_open(value, O_RDONLY, errp);
if (fd < 0) {
return -1;
}
while (1) {
ret = read(fd, buf, 4096);
if (ret == 0) {
break;
}
if (ret < 0) {
error_setg(errp, "Unable to read from %s: %s",
value, strerror(errno));
qemu_close(fd);
return -1;
}
if (memchr(buf, '\0', ret)) {
error_setg(errp, "NUL in OEM strings value in %s", value);
qemu_close(fd);
return -1;
}
g_byte_array_append(data, (guint8 *)buf, ret);
}
buf[0] = '\0';
g_byte_array_append(data, (guint8 *)buf, 1);
qemu_close(fd);
*opt->dest = g_renew(char *, *opt->dest, (*opt->ndest) + 1);
(*opt->dest)[*opt->ndest] = (char *)g_byte_array_free(data, FALSE);
(*opt->ndest)++;
data = NULL;
}