From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:32964) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZluAo-0008C8-Du for qemu-devel@nongnu.org; Tue, 13 Oct 2015 03:39:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZluAl-0006GP-5W for qemu-devel@nongnu.org; Tue, 13 Oct 2015 03:39:02 -0400 From: Kevin Wolf Date: Tue, 13 Oct 2015 09:38:50 +0200 Message-Id: <1444721930-5121-1-git-send-email-kwolf@redhat.com> Subject: [Qemu-devel] [PATCH] gdbstub: Fix buffer overflows in gdb_handle_packet() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: kwolf@redhat.com, qemu-trivial@nongnu.org, qemu-stable@nongnu.org Some places in gdb_handle_packet() can get an arbitrary length (most times directly from the client) and either didn't check it at all or checked against the wrong value, potentially causing buffer overflows. Cc: qemu-stable@nongnu.org Signed-off-by: Kevin Wolf --- gdbstub.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/gdbstub.c b/gdbstub.c index d2c95b5..9c29aa0 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -956,6 +956,13 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) if (*p == ',') p++; len = strtoull(p, NULL, 16); + + /* memtohex() doubles the required space */ + if (len > MAX_PACKET_LENGTH / 2) { + put_packet (s, "E22"); + break; + } + if (target_memory_rw_debug(s->g_cpu, addr, mem_buf, len, false) != 0) { put_packet (s, "E14"); } else { @@ -970,6 +977,12 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) len = strtoull(p, (char **)&p, 16); if (*p == ':') p++; + + /* hextomem() reads 2*len bytes */ + if (len > strlen(p) / 2) { + put_packet (s, "E22"); + break; + } hextomem(mem_buf, p, len); if (target_memory_rw_debug(s->g_cpu, addr, mem_buf, len, true) != 0) { @@ -1107,7 +1120,8 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) cpu = find_cpu(thread); if (cpu != NULL) { cpu_synchronize_state(cpu); - len = snprintf((char *)mem_buf, sizeof(mem_buf), + /* memtohex() doubles the required space */ + len = snprintf((char *)mem_buf, sizeof(buf) / 2, "CPU#%d [%s]", cpu->cpu_index, cpu->halted ? "halted " : "running"); memtohex(buf, mem_buf, len); @@ -1136,8 +1150,8 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) put_packet(s, "E01"); break; } - hextomem(mem_buf, p + 5, len); len = len / 2; + hextomem(mem_buf, p + 5, len); mem_buf[len++] = 0; qemu_chr_be_write(s->mon_chr, mem_buf, len); put_packet(s, "OK"); -- 1.8.3.1