Wireshark-bugs: [Wireshark-bugs] [Bug 8912] IxVeriWave 11ac patch
Date: Sun, 16 Mar 2014 20:47:59 +0000
Comment # 17
on bug 8912
from Guy Harris
(In reply to comment #14) > Some notes about the following part of the patch for wiretap/vwr.c in > vwr_get_fpga_version(): > > if ((rec_size > vVW510021_W_STATS_LEN) && (fpga_version == 1000)) { > /* Check the version of the FPGA */ > if (header[8] == 48) > fpga_version = S3_W_FPGA; > } > > if ((rec_size > vVW510021_W_STATS_LEN) && (fpga_version == 1000)) { > /* Check the version of the FPGA */ > if (header[8] == 48) > fpga_version = S3_W_FPGA; > } > > 1. The code is duplicated. Can the duplicated code be removed or was the > second > if (...) intended to be something different ? I've removed the duplication. > 2. More importantly: with the addition of this test, at least 1 capture file > I have is incorrectly identified as a vwr capture file > (and then a crash occurs (see below) when the file is read for > dissection). > > Looking at the code in vwr.c: vwr_get_fpga_version(): this test seems > a bit weak. > Essentially: Test for one of hex 21/31/C1/8B/FE in the 0th byte > and hex 30 in the 8th byte of a 16 byte 'header'. > > Looking a bit more deeply at the code: A search is done through the > file looking for certain patterns and skipping stuff (headers/data) which > don't match the pattern. Would it be possible to do some initial > validation > of each "header" (e.g., checking for valid 'cmd' values) so that there > would > be less likelihood of having to read a lot of data from the file before > deciding that it's not a vwr file. I'll look at strengthening the heuristics. > 3. The crash occurred in 'vwr_read_rec_data_wlan' in the following code: > > if ( rec_size < ((int)common_fields.vw_msdu_length + > (int)vwr->STATS_LEN) ) > /*something's been truncated, DUMP AS-IS*/ > memcpy(&data_ptr[bytes_written], &rec[mpdu_offset], > common_fields.vw_msdu_length); > > I think the problem is copying something greater than 'rec_size' is ng. I've added more sanity checks to the code in the trunk. (I reorganized the code a fair bit to make it a bit clearer what was going on so I could figure out > (I'll see if I can share the capture file I have which causes the crash). If you still have it, see what happens on the trunk.
You are receiving this mail because:
- You are watching all bug changes.
- Prev by Date: [Wireshark-bugs] [Bug 9889] New: Buildbot crash output: fuzz-2014-03-16-26304.pcap
- Next by Date: [Wireshark-bugs] [Bug 8912] IxVeriWave 11ac patch
- Previous by thread: [Wireshark-bugs] [Bug 9889] Buildbot crash output: fuzz-2014-03-16-26304.pcap
- Next by thread: [Wireshark-bugs] [Bug 8912] IxVeriWave 11ac patch
- Index(es):
- Get Wireshark
- Download
- Code of Conduct