* [Qemu-devel] [PATCH] security_20040626
@ 2004-06-27 0:16 Tim
0 siblings, 0 replies; only message in thread
From: Tim @ 2004-06-27 0:16 UTC (permalink / raw)
To: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 462 bytes --]
I have been pretty busy lately, so it took some time to get this version
out. I took Charlie's advice and created a version of strdup which uses
qemu_malloc() instead of malloc().
Since Fabrice already applied a number of fixes a while back, this patch
doesn't have a whole lot of real code changes, outside of stuff in slirp
and the pstrdup() function. I'll be turning my attention to the tmp
file handling issues next.
Ok, feel free to fire away...
tim
[-- Attachment #2: security.patch --]
[-- Type: text/plain, Size: 14520 bytes --]
--- qemu-current/hw/cirrus_vga.c 2004-06-26 16:56:25.000000000 -0700
+++ qemu-dev/hw/cirrus_vga.c 2004-06-26 11:30:07.000000000 -0700
@@ -2861,7 +2861,7 @@
CirrusVGAState *s;
s = qemu_mallocz(sizeof(CirrusVGAState));
-
+ /* TODO: check for s == NULL */
vga_common_init((VGAState *)s,
ds, vga_ram_base, vga_ram_offset, vga_ram_size);
cirrus_init_common(s, CIRRUS_ID_CLGD5430, 0);
--- qemu-current/hw/vga.c 2004-06-26 16:56:25.000000000 -0700
+++ qemu-dev/hw/vga.c 2004-06-26 11:30:07.000000000 -0700
@@ -1850,6 +1850,7 @@
{
s->linesize = w * 4;
s->data = qemu_malloc(h * s->linesize);
+ /* TODO: add check for s->data == NULL ?? */
vga_save_w = w;
vga_save_h = h;
}
--- qemu-current/dyngen.c 2004-04-04 05:56:28.000000000 -0700
+++ qemu-dev/dyngen.c 2004-06-18 08:33:58.000000000 -0700
@@ -458,6 +458,7 @@
/* read all section data */
sdata = malloc(sizeof(void *) * ehdr.e_shnum);
+ /* TODO: check for sdata == NULL */
memset(sdata, 0, sizeof(void *) * ehdr.e_shnum);
for(i = 0;i < ehdr.e_shnum; i++) {
@@ -653,6 +654,7 @@
/* read all section data */
sdata = malloc(sizeof(void *) * fhdr.f_nscns);
+ /* TODO: check for sdata == NULL */
memset(sdata, 0, sizeof(void *) * fhdr.f_nscns);
const char *p;
@@ -698,6 +700,7 @@
/* set coff symbol */
symtab = malloc(sizeof(struct coff_sym) * nb_syms);
+ /* TODO: check for symtab == NULL */
int aux_size, j;
for (i = 0, ext_sym = coff_symtab, sym = symtab; i < nb_syms; i++, ext_sym++, sym++) {
@@ -746,6 +749,7 @@
/* set coff relocation */
relocs = malloc(sizeof(struct coff_rel) * nb_relocs);
+ /* TODO: check for relocs == NULL */
for (i = 0, ext_rel = coff_relocs, rel = relocs; i < nb_relocs;
i++, ext_rel++, rel++) {
memset(rel, 0, sizeof(*rel));
--- qemu-current/exec.c 2004-06-26 16:56:25.000000000 -0700
+++ qemu-dev/exec.c 2004-06-23 20:09:25.000000000 -0700
@@ -156,6 +156,7 @@
if (!p) {
/* allocate if not found */
p = qemu_malloc(sizeof(PageDesc) * L2_SIZE);
+ /* TODO: check for p == NULL */
memset(p, 0, sizeof(PageDesc) * L2_SIZE);
*lp = p;
}
@@ -181,6 +182,7 @@
if (!p) {
/* allocate if not found */
p = qemu_malloc(sizeof(PhysPageDesc) * L2_SIZE);
+ /* TODO: check for p == NULL */
memset(p, 0, sizeof(PhysPageDesc) * L2_SIZE);
*lp = p;
}
@@ -210,6 +212,7 @@
if (!p) {
/* allocate if not found */
p = qemu_malloc(sizeof(VirtPageDesc) * L2_SIZE);
+ /* TODO: check for p == NULL */
memset(p, 0, sizeof(VirtPageDesc) * L2_SIZE);
*lp = p;
}
@@ -1914,6 +1917,7 @@
/* alloc dirty bits array */
phys_ram_dirty = qemu_malloc(phys_ram_size >> TARGET_PAGE_BITS);
+ /* TODO: check for phys_ram_dirty == NULL */
}
/* mem_read and mem_write are arrays of functions containing the
--- qemu-current/monitor.c 2004-06-26 16:56:25.000000000 -0700
+++ qemu-dev/monitor.c 2004-06-20 18:39:09.000000000 -0700
@@ -1179,8 +1179,7 @@
len = p - pstart;
if (len > sizeof(cmdname) - 1)
len = sizeof(cmdname) - 1;
- memcpy(cmdname, pstart, len);
- cmdname[len] = '\0';
+ pstrcpy(cmdname, len, pstart);
/* find the command */
for(cmd = term_cmds; cmd->name != NULL; cmd++) {
@@ -1227,8 +1226,9 @@
term_printf("%s: string expected\n", cmdname);
goto fail;
}
- str = qemu_malloc(strlen(buf) + 1);
- strcpy(str, buf);
+ str = pstrdup(buf);
+ if (!str)
+ goto fail;
str_allocated[nb_args] = str;
add_str:
if (nb_args >= MAX_ARGS) {
--- qemu-current/sparc-dis.c 2003-06-09 08:23:31.000000000 -0700
+++ qemu-dev/sparc-dis.c 2004-06-18 09:05:50.000000000 -0700
@@ -2468,6 +2468,7 @@
if (!opcodes_initialized)
sorted_opcodes = (const struct sparc_opcode **)
malloc (sparc_num_opcodes * sizeof (struct sparc_opcode *));
+ /* TODO: check for sorted_opcodes == NULL */
/* Reset the sorted table so we can resort it. */
for (i = 0; i < sparc_num_opcodes; ++i)
sorted_opcodes[i] = &sparc_opcodes[i];
@@ -3183,6 +3184,7 @@
if (hash_buf != NULL)
free (hash_buf);
hash_buf = (struct opcode_hash *) malloc (sizeof (struct opcode_hash) * num_opcodes);
+ /* TODO: check for hash_buf == NULL */
for (i = num_opcodes - 1; i >= 0; --i)
{
register int hash = HASH_INSN (opcode_table[i]->match);
--- qemu-current/thunk.c 2003-06-15 12:52:54.000000000 -0700
+++ qemu-dev/thunk.c 2004-06-18 08:48:05.000000000 -0700
@@ -85,6 +85,7 @@
offset = 0;
max_align = 1;
se->field_offsets[i] = malloc(nb_fields * sizeof(int));
+ /* TODO: check for NULL from malloc() */
type_ptr = se->field_types;
for(j = 0;j < nb_fields; j++) {
size = thunk_type_size(type_ptr, i);
--- qemu-current/vl.c 2004-06-26 16:56:25.000000000 -0700
+++ qemu-dev/vl.c 2004-06-26 11:30:06.000000000 -0700
@@ -293,6 +293,16 @@
return buf;
}
+char *pstrdup(char *buf)
+{
+ unsigned int size = strlen(buf) + 1;
+ char* ret_val = (char*)qemu_malloc(size);
+ if (ret_val == NULL)
+ return NULL;
+ pstrcpy(ret_val, size, buf);
+ return ret_val;
+}
+
/* return the size or -1 if error */
int get_image_size(const char *filename)
{
@@ -618,6 +628,7 @@
QEMUTimer *ts;
ts = qemu_mallocz(sizeof(QEMUTimer));
+ /* TODO: check for ts == NULL */
ts->clock = clock;
ts->cb = cb;
ts->opaque = opaque;
--- qemu-current/vl.h 2004-06-26 16:56:25.000000000 -0700
+++ qemu-dev/vl.h 2004-06-26 13:41:16.000000000 -0700
@@ -215,6 +215,7 @@
void pstrcpy(char *buf, int buf_size, const char *str);
char *pstrcat(char *buf, int buf_size, const char *s);
+char *pstrdup(char *buf);
int serial_open_device(void);
--- qemu-current/linux-user/path.c 2003-04-10 17:15:13.000000000 -0700
+++ qemu-dev/linux-user/path.c 2004-06-18 11:02:32.000000000 -0700
@@ -43,7 +43,9 @@
struct pathelem *parent,
const char *name)
{
+ /* TODO: is this line safe? */
struct pathelem *new = malloc(sizeof(*new));
+ /* TODO: check for new == NULL */
new->name = strdup(name);
asprintf(&new->pathname, "%s/%s", root, name);
new->num_entries = 0;
@@ -75,7 +77,7 @@
root = realloc(root, sizeof(*root)
+ sizeof(root->entries[0])*root->num_entries);
-
+ /* TODO: check for root == NULL */
root->entries[root->num_entries-1] = new_entry(root->pathname, root, name);
root->entries[root->num_entries-1]
= add_dir_maybe(root->entries[root->num_entries-1]);
--- qemu-current/linux-user/syscall.c 2004-06-26 16:56:26.000000000 -0700
+++ qemu-dev/linux-user/syscall.c 2004-06-19 13:12:14.000000000 -0700
@@ -1410,6 +1410,7 @@
if (flags & CLONE_VM) {
ts = malloc(sizeof(TaskState) + NEW_STACK_SIZE);
+ /* TODO: check for ts == NULL */
memset(ts, 0, sizeof(TaskState));
new_stack = ts->stack;
ts->used = 1;
@@ -2473,7 +2474,7 @@
tnamelen = treclen - (2 * sizeof(target_long) + 2);
if (tnamelen > 256)
tnamelen = 256;
- strncpy(tde->d_name, de->d_name, tnamelen);
+ pstrcpy(tde->d_name, tnamelen, de->d_name);
de = (struct dirent *)((char *)de + reclen);
len -= reclen;
tde = (struct dirent *)((char *)tde + treclen);
--- qemu-current/slirp/debug.c 2004-06-16 20:43:10.000000000 -0700
+++ qemu-dev/slirp/debug.c 2004-06-15 19:40:42.000000000 -0700
@@ -305,7 +305,7 @@
for (so = tcb.so_next; so != &tcb; so = so->so_next) {
- n = sprintf(buff, "tcp[%s]", so->so_tcpcb?tcpstates[so->so_tcpcb->t_state]:"NONE");
+ n = snprintf(buff, sizeof(buff),"tcp[%s]", so->so_tcpcb?tcpstates[so->so_tcpcb->t_state]:"NONE");
while (n < 17)
buff[n++] = ' ';
buff[17] = 0;
@@ -319,7 +319,7 @@
for (so = udb.so_next; so != &udb; so = so->so_next) {
- n = sprintf(buff, "udp[%d sec]", (so->so_expire - curtime) / 1000);
+ n = snprintf(buff, sizeof(buff), "udp[%d sec]", (so->so_expire - curtime) / 1000);
while (n < 17)
buff[n++] = ' ';
buff[17] = 0;
--- qemu-current/slirp/mbuf.c 2004-04-21 17:10:47.000000000 -0700
+++ qemu-dev/slirp/mbuf.c 2004-06-18 11:04:03.000000000 -0700
@@ -152,6 +152,7 @@
if (m->m_flags & M_EXT) {
/* datasize = m->m_data - m->m_ext; */
m->m_ext = (char *)realloc(m->m_ext,size);
+ /* TODO: fix? and re-add check for m->m_ext == NULL */
/* if (m->m_ext == NULL)
* return (struct mbuf *)NULL;
*/
@@ -161,6 +162,7 @@
char *dat;
datasize = m->m_data - m->m_dat;
dat = (char *)malloc(size);
+ /* TODO: fix? and re-add the dat==NULL check */
/* if (dat == NULL)
* return (struct mbuf *)NULL;
*/
--- qemu-current/slirp/misc.c 2004-06-16 20:43:10.000000000 -0700
+++ qemu-dev/slirp/misc.c 2004-06-20 18:06:22.000000000 -0700
@@ -182,6 +182,7 @@
tmp_ptr = *ex_ptr;
*ex_ptr = (struct ex_list *)malloc(sizeof(struct ex_list));
+ /* TODO: check for *ex_ptr == NULL */
(*ex_ptr)->ex_fport = port;
(*ex_ptr)->ex_addr = addr;
(*ex_ptr)->ex_pty = do_pty;
@@ -362,10 +363,10 @@
if (x_port >= 0) {
#ifdef HAVE_SETENV
- sprintf(buff, "%s:%d.%d", inet_ntoa(our_addr), x_port, x_screen);
+ snprintf(buff, sizeof(buff), "%s:%d.%d", inet_ntoa(our_addr), x_port, x_screen);
setenv("DISPLAY", buff, 1);
#else
- sprintf(buff, "DISPLAY=%s:%d.%d", inet_ntoa(our_addr), x_port, x_screen);
+ snprintf(buff, sizeof(buff), "DISPLAY=%s:%d.%d", inet_ntoa(our_addr), x_port, x_screen);
putenv(buff);
#endif
}
@@ -401,7 +402,7 @@
{
char buff[256];
- sprintf(buff, "Error: execvp of %s failed: %s\n",
+ snprintf(buff, sizeof(buff), "Error: execvp of %s failed: %s\n",
argv[0], strerror(errno));
write(2, buff, strlen(buff)+1);
}
@@ -448,6 +449,7 @@
char *bptr;
bptr = (char *)malloc(strlen(str)+1);
+ /* TODO: check for bptr == NULL */
strcpy(bptr, str);
return bptr;
@@ -476,7 +478,7 @@
sock_in.sin_port = htons(slirp_socket_port);
if (connect(s, (struct sockaddr *)&sock_in, sizeof(sock_in)) != 0)
slirp_exit(1); /* just exit...*/
- sprintf(buff, "kill %s:%d", slirp_socket_passwd, slirp_socket_unit);
+ snprintf(buff, sizeof(buff), "kill %s:%d", slirp_socket_passwd, slirp_socket_unit);
write(s, buff, strlen(buff)+1);
}
#ifndef NO_UNIX_SOCKETS
@@ -485,11 +487,11 @@
if (s < 0)
slirp_exit(1);
sock_un.sun_family = AF_UNIX;
- strcpy(sock_un.sun_path, socket_path);
+ snprintf(sock_un.sun_path, sizeof(sock_un.sun_path), "%s", socket_path);
if (connect(s, (struct sockaddr *)&sock_un,
sizeof(sock_un.sun_family) + sizeof(sock_un.sun_path)) != 0)
slirp_exit(1);
- sprintf(buff, "kill none:%d", slirp_socket_unit);
+ snprintf(buff, sizeof(buff), "kill none:%d", slirp_socket_unit);
write(s, buff, strlen(buff)+1);
}
#endif
@@ -615,7 +617,7 @@
lprint_sb->sb_data = (char *)realloc(lprint_sb->sb_data,
lprint_sb->sb_datalen + TCP_SNDSPACE);
-
+ /* TODO: check for lprint_sb->sb_data == NULL */
/* Adjust all values */
lprint_sb->sb_wptr = lprint_sb->sb_data + deltaw;
lprint_sb->sb_rptr = lprint_sb->sb_data + deltar;
@@ -715,6 +717,7 @@
/* link it */
emup = (struct emu_t *)malloc(sizeof (struct emu_t));
+ /* TODO: check for emup == NULL */
emup->lport = (u_int16_t)lport;
emup->fport = (u_int16_t)fport;
emup->tos = tos;
@@ -889,10 +892,10 @@
/* Set the DISPLAY */
if (x_port >= 0) {
#ifdef HAVE_SETENV
- sprintf(buff, "%s:%d.%d", inet_ntoa(our_addr), x_port, x_screen);
+ snprintf(buff, sizeof(buff), "%s:%d.%d", inet_ntoa(our_addr), x_port, x_screen);
setenv("DISPLAY", buff, 1);
#else
- sprintf(buff, "DISPLAY=%s:%d.%d", inet_ntoa(our_addr), x_port, x_screen);
+ snprintf(buff, sizeof(buff), "DISPLAY=%s:%d.%d", inet_ntoa(our_addr), x_port, x_screen);
putenv(buff);
#endif
}
@@ -907,7 +910,7 @@
/* Ooops, failed, let's tell the user why */
- sprintf(buff, "Error: execlp of %s failed: %s\n",
+ snprintf(buff, sizeof(buff), "Error: execlp of %s failed: %s\n",
"rsh", strerror(errno));
write(2, buff, strlen(buff)+1);
close(0); close(1); close(2); /* XXX */
--- qemu-current/slirp/tcp_subr.c 2004-06-16 20:43:10.000000000 -0700
+++ qemu-dev/slirp/tcp_subr.c 2004-06-15 19:51:31.000000000 -0700
@@ -731,7 +731,7 @@
if (*ptr++ == 0) {
n++;
if (n == 2) {
- sprintf(args, "rlogin -l %s %s",
+ snprintf(args, sizeof(args), "rlogin -l %s %s",
ptr, inet_ntoa(so->so_faddr));
} else if (n == 3) {
i2 = so_rcv->sb_wptr - ptr;
@@ -739,9 +739,9 @@
if (ptr[i] == '/') {
ptr[i] = 0;
#ifdef HAVE_SETENV
- sprintf(term, "%s", ptr);
+ snprintf(term, sizeof(term), "%s", ptr);
#else
- sprintf(term, "TERM=%s", ptr);
+ snprintf(term, sizeof(term), "TERM=%s", ptr);
#endif
ptr[i] = '/';
break;
--- qemu-current/slirp/udp.c 2004-04-21 17:10:48.000000000 -0700
+++ qemu-dev/slirp/udp.c 2004-06-20 17:45:46.000000000 -0700
@@ -449,14 +449,14 @@
type = omsg->type;
OTOSIN(omsg, ctl_addr)->sin_port = addr.sin_port;
OTOSIN(omsg, ctl_addr)->sin_addr = our_addr;
- strncpy(omsg->l_name, getlogin(), NAME_SIZE_OLD);
+ snprintf(omsg->l_name, NAME_SIZE_OLD, "%s", getlogin());
} else { /* new talk */
omsg = (CTL_MSG_OLD *) buff;
nmsg = mtod(m, CTL_MSG *);
type = nmsg->type;
OTOSIN(nmsg, ctl_addr)->sin_port = addr.sin_port;
OTOSIN(nmsg, ctl_addr)->sin_addr = our_addr;
- strncpy(nmsg->l_name, getlogin(), NAME_SIZE_OLD);
+ snprintf(nmsg->l_name, NAME_SIZE_OLD, "%s", getlogin());
}
if (type == LOOK_UP)
@@ -493,6 +493,7 @@
if (!req) { /* no entry for so, create new */
req = (struct talk_request *)
malloc(sizeof(struct talk_request));
+ /* TODO: check for req == NULL */
req->udp_so = so;
req->tcp_so = solisten(0,
OTOSIN(omsg, addr)->sin_addr.s_addr,
[-- Attachment #3: README --]
[-- Type: text/plain, Size: 1023 bytes --]
This patch contains fixes for calls to potentially unsafe string
manipulation functions. Other changes may avert future
vulnerabilities. In addition, TODO comments were added to certain
sections of code where further checks should be implemented. This
version of the patch makes changes to the following files in the most
current CVS (as of today):
dyngen.c
exec.c
monitor.c
sparc-dis.c
thunk.c
vl.c
vl.h
hw/cirrus_vga.c
hw/vga.c
linux-user/path.c
linux-user/syscall.c
slirp/debug.c
slirp/mbuf.c
slirp/misc.c
slirp/tcp_subr.c
slirp/udp.c
Any original code contained within this patch is hereby released into
the public domain. All derivative portions of code contained within,
for the purposes of documentation, are a part of QEMU, and are therefore
protected by the copyright of their respective authors.
While care was taken in making these changes, it is possible that this
patch will break something. Use your own risk. Please report any
success/failure to:
tim {HYPHEN} qemu {AT} sentinelchicken {DOT} org.
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2004-06-27 0:18 UTC | newest]
Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-06-27 0:16 [Qemu-devel] [PATCH] security_20040626 Tim
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).