From 8f8a0f72b442efe66c7ee26417a92508a1546289 Mon Sep 17 00:00:00 2001 From: Peter Wu Date: Thu, 29 Jun 2017 22:44:08 +0200 Subject: dumpcap: fix buffer overflow on packets larger than 2048 bytes When the current capture buffer is too small, it must be increased before attempting to read the next data packet. Fix developed by Mikael Kanstrup (and Guy), I added comments such that the next reader does not have to guess whether "incl_len" is accidentally used for reading from the buffer (it is not). Change-Id: I980bd21ac79601a34d57ffc99a34bfb54c297ac0 Fixes: v2.5.0rc0-28-gd0865fd619 ("Allow bigger snapshot lengths for D-Bus captures.") Bug: 13852 Reviewed-on: https://code.wireshark.org/review/22464 Petri-Dish: Peter Wu Tested-by: Petri Dish Buildbot Reviewed-by: Guy Harris --- dumpcap.c | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/dumpcap.c b/dumpcap.c index 6c7e26c22c..a1d20722db 100644 --- a/dumpcap.c +++ b/dumpcap.c @@ -2049,16 +2049,17 @@ cap_pipe_dispatch(loop_data *ld, capture_src *pcap_src, char *errmsg, int errmsg cap_pipe_adjust_header(pcap_src->cap_pipe_byte_swapped, &pcap_src->cap_pipe_hdr, &pcap_src->cap_pipe_rechdr.hdr); if (pcap_src->cap_pipe_rechdr.hdr.incl_len > pcap_src->cap_pipe_max_pkt_size) { + /* + * The record contains more data than the advertised/allowed in the + * pcap header, do not try to read more data (do not change to + * STATE_EXPECT_DATA) as that would not fit in the buffer and + * instead stop with an error. + */ g_snprintf(errmsg, errmsgl, "Frame %u too long (%d bytes)", ld->packet_count+1, pcap_src->cap_pipe_rechdr.hdr.incl_len); break; } - if (pcap_src->cap_pipe_rechdr.hdr.incl_len) { - pcap_src->cap_pipe_state = STATE_EXPECT_DATA; - return 0; - } - if (pcap_src->cap_pipe_rechdr.hdr.incl_len > pcap_src->cap_pipe_databuf_size) { /* * Grow the buffer to the packet size, rounded up to a power of @@ -2078,7 +2079,20 @@ cap_pipe_dispatch(loop_data *ld, capture_src *pcap_src, char *errmsg, int errmsg pcap_src->cap_pipe_databuf = (guchar*)g_realloc(pcap_src->cap_pipe_databuf, new_bufsize); pcap_src->cap_pipe_databuf_size = new_bufsize; } - /* no data to read? */ + + /* + * The record has some data following the header, try to read it next + * time. + */ + if (pcap_src->cap_pipe_rechdr.hdr.incl_len) { + pcap_src->cap_pipe_state = STATE_EXPECT_DATA; + return 0; + } + + /* + * No data following the record header? Then no more data needs to be + * read and we will fallthrough and emit an empty packet. + */ /* FALLTHROUGH */ case PD_DATA_READ: /* Fill in a "struct pcap_pkthdr", and process the packet. */ -- cgit v1.2.1