From cdb942944aa91202fe85372c8f13c70db0ce3141 Mon Sep 17 00:00:00 2001 From: Guy Harris Date: Mon, 2 Sep 2019 14:23:33 -0700 Subject: [PATCH] Strengthen the I4B heuristics. Check some more field values, and fix some tests to check against the maximum possible value given in the i4b_trace.h file rather than against that value + 1. (> max, or >= max+1, are both reasonable, but > max+1 isn't.) Check the first few packets, not just the first packet. Make some header fields unsigned, as that's how we treat them in most cases; that way we treat them that way by default. Change-Id: I8c2d28af048c676a3dbae367bbb49c886e0dc566 Ping-Bug: 16031 Reviewed-on: https://code.wireshark.org/review/34432 Petri-Dish: Guy Harris Tested-by: Petri Dish Buildbot Reviewed-by: Guy Harris --- wiretap/i4b_trace.h | 6 +-- wiretap/i4btrace.c | 102 +++++++++++++++++++++++++++++++++++++------- 2 files changed, 89 insertions(+), 19 deletions(-) diff --git a/wiretap/i4b_trace.h b/wiretap/i4b_trace.h index 174278f3a1..ffbb25c2ab 100644 --- a/wiretap/i4b_trace.h +++ b/wiretap/i4b_trace.h @@ -15,8 +15,8 @@ *---------------------------------------------------------------------------*/ typedef struct { guint32 length; /* length of the following mbuf */ - gint32 unit; /* controller unit number */ - gint32 type; /* type of channel */ + guint32 unit; /* controller unit number */ + guint32 type; /* type of channel */ #define TRC_CH_I 0 /* Layer 1 INFO's */ #define TRC_CH_D 1 /* D channel */ #define TRC_CH_B1 2 /* B1 channel */ @@ -24,7 +24,7 @@ typedef struct { gint32 dir; /* direction */ #define FROM_TE 0 /* user -> network */ #define FROM_NT 1 /* network -> user */ - gint32 trunc; /* # of truncated bytes (frame > MCLBYTES) */ + guint32 trunc; /* # of truncated bytes (frame > MCLBYTES) */ guint32 count; /* frame count for this unit/type */ guint32 ts_sec; /* timestamp seconds */ guint32 ts_usec; /* timestamp microseconds */ diff --git a/wiretap/i4btrace.c b/wiretap/i4btrace.c index 35cb3f30cb..69de8859bd 100644 --- a/wiretap/i4btrace.c +++ b/wiretap/i4btrace.c @@ -27,13 +27,37 @@ static gboolean i4btrace_seek_read(wtap *wth, gint64 seek_off, static int i4b_read_rec(wtap *wth, FILE_T fh, wtap_rec *rec, Buffer *buf, int *err, gchar **err_info); +/* + * Byte-swap the header. + */ +#define I4B_BYTESWAP_HEADER(hdr) \ + { \ + hdr.length = GUINT32_SWAP_LE_BE(hdr.length); \ + hdr.unit = GUINT32_SWAP_LE_BE(hdr.unit); \ + hdr.type = GUINT32_SWAP_LE_BE(hdr.type); \ + hdr.dir = GUINT32_SWAP_LE_BE(hdr.dir); \ + hdr.trunc = GUINT32_SWAP_LE_BE(hdr.trunc); \ + hdr.count = GUINT32_SWAP_LE_BE(hdr.count); \ + hdr.ts_sec = GUINT32_SWAP_LE_BE(hdr.ts_sec); \ + hdr.ts_usec = GUINT32_SWAP_LE_BE(hdr.ts_usec); \ + } + /* * Test some fields in the header to see if they make sense. */ #define I4B_HDR_IS_OK(hdr) \ - (!((unsigned int)hdr.length < 3 || (unsigned int)hdr.length > 16384 || \ - (unsigned int)hdr.unit > 4 || (unsigned int)hdr.type > 4 || \ - (unsigned int)hdr.dir > 2 || (unsigned int)hdr.trunc > 2048)) + (!(hdr.length < sizeof(hdr) || \ + hdr.length > 16384 || \ + hdr.unit > 4 || \ + hdr.type > TRC_CH_B2 || \ + hdr.dir > FROM_NT || \ + hdr.trunc > 2048 || \ + hdr.ts_usec >= 1000000)) + +/* + * Number of packets to try reading. + */ +#define PACKETS_TO_CHECK 5 wtap_open_return_val i4btrace_open(wtap *wth, int *err, gchar **err_info) { @@ -53,11 +77,7 @@ wtap_open_return_val i4btrace_open(wtap *wth, int *err, gchar **err_info) /* * OK, try byte-swapping the header fields. */ - hdr.length = GUINT32_SWAP_LE_BE(hdr.length); - hdr.unit = GUINT32_SWAP_LE_BE(hdr.unit); - hdr.type = GUINT32_SWAP_LE_BE(hdr.type); - hdr.dir = GUINT32_SWAP_LE_BE(hdr.dir); - hdr.trunc = GUINT32_SWAP_LE_BE(hdr.trunc); + I4B_BYTESWAP_HEADER(hdr); if (!I4B_HDR_IS_OK(hdr)) { /* * It doesn't look valid in either byte order. @@ -72,6 +92,63 @@ wtap_open_return_val i4btrace_open(wtap *wth, int *err, gchar **err_info) byte_swapped = TRUE; } + /* + * Now try to read past the packet bytes; if that fails with + * a short read, we don't fail, so that we can report + * the file as a truncated I4B file. + */ + if (!wtap_read_bytes(wth->fh, NULL, hdr.length - (guint32)sizeof(hdr), + err, err_info)) { + if (*err != WTAP_ERR_SHORT_READ) + return WTAP_OPEN_ERROR; + } + + /* + * Now try reading a few more packets. + */ + for (int i = 1; i < PACKETS_TO_CHECK; i++) { + /* + * Read and check the file header; we've already + * decided whether this would be a byte-swapped file + * or not, so we swap iff we decided it was. + */ + if (!wtap_read_bytes_or_eof(wth->fh, &hdr, sizeof(hdr), err, + err_info)) { + if (*err == 0) { + /* EOF; no more packets to try. */ + break; + } + if (*err != WTAP_ERR_SHORT_READ) + return WTAP_OPEN_ERROR; + return WTAP_OPEN_NOT_MINE; + } + + if (byte_swapped) + I4B_BYTESWAP_HEADER(hdr); + if (!I4B_HDR_IS_OK(hdr)) { + /* + * It doesn't look valid in either byte order. + */ + return WTAP_OPEN_NOT_MINE; + } + + /* + * Now try to read past the packet bytes; if that fails with + * a short read, we don't fail, so that we can report + * the file as a truncated I4B file. + */ + if (!wtap_read_bytes(wth->fh, NULL, hdr.length - (guint32)sizeof(hdr), + err, err_info)) { + if (*err != WTAP_ERR_SHORT_READ) + return WTAP_OPEN_ERROR; + + /* + * Probably a truncated file, so just quit. + */ + break; + } + } + if (file_seek(wth->fh, 0, SEEK_SET, err) == -1) return WTAP_OPEN_ERROR; @@ -134,14 +211,7 @@ i4b_read_rec(wtap *wth, FILE_T fh, wtap_rec *rec, Buffer *buf, /* * Byte-swap the header. */ - hdr.length = GUINT32_SWAP_LE_BE(hdr.length); - hdr.unit = GUINT32_SWAP_LE_BE(hdr.unit); - hdr.type = GUINT32_SWAP_LE_BE(hdr.type); - hdr.dir = GUINT32_SWAP_LE_BE(hdr.dir); - hdr.trunc = GUINT32_SWAP_LE_BE(hdr.trunc); - hdr.count = GUINT32_SWAP_LE_BE(hdr.count); - hdr.ts_sec = GUINT32_SWAP_LE_BE(hdr.ts_sec); - hdr.ts_usec = GUINT32_SWAP_LE_BE(hdr.ts_usec); + I4B_BYTESWAP_HEADER(hdr); } if (hdr.length < sizeof(hdr)) {