From 266d3b7d519ba3d9c7e2d179f52bca5529cbd0b0 Mon Sep 17 00:00:00 2001 From: Guy Harris Date: Tue, 1 Dec 2015 16:35:00 -0800 Subject: [PATCH] Move the bitrate test against 0 to mp2t_bits_per_second(). As the comment says, that routine "[ensures] there is a valid bitrate", and a bitrate of 0, which comes from truncating a fractional bitrate, is not a valid bitrate (an MPEG-2 Transport Stream with a bitrate less than 1 bit per second is not going to carrry any sensible audio/video stream). Make the "first" argument unsigned; it can never be negative. Restructure the code and change some data types to make it more obvious that it can't. Change-Id: Idd4d073dc558bb31271318e14b2f74292cd16a2b Reviewed-on: https://code.wireshark.org/review/12352 Reviewed-by: Guy Harris --- wiretap/mp2t.c | 44 ++++++++++++++++++++++++++------------------ 1 file changed, 26 insertions(+), 18 deletions(-) diff --git a/wiretap/mp2t.c b/wiretap/mp2t.c index 6004bc6475..a569c99c05 100644 --- a/wiretap/mp2t.c +++ b/wiretap/mp2t.c @@ -54,7 +54,7 @@ typedef struct { - int start_offset; + guint32 start_offset; guint64 bitrate; /* length of trailing data (e.g. FEC) that's appended after each packet */ guint8 trailer_len; @@ -211,7 +211,7 @@ mp2t_find_next_pcr(wtap *wth, guint8 trailer_len, } static wtap_open_return_val -mp2t_bits_per_second(wtap *wth, gint first, guint8 trailer_len, +mp2t_bits_per_second(wtap *wth, guint32 first, guint8 trailer_len, guint64 *bitrate, int *err, gchar **err_info) { guint32 pn1, pn2; @@ -228,10 +228,7 @@ mp2t_bits_per_second(wtap *wth, gint first, guint8 trailer_len, * XXX - is this assuming that the time stamps in the PCRs correspond * to the time scale of the underlying transport stream? */ - - if (first<0) - return WTAP_OPEN_ERROR; - idx = (guint32)first; + idx = first; if (!mp2t_find_next_pcr(wth, trailer_len, err, err_info, &idx, &pcr1, &pid1)) { /* Read error, short read, or EOF */ @@ -281,6 +278,21 @@ mp2t_bits_per_second(wtap *wth, gint first, guint8 trailer_len, bits_passed = MP2T_SIZE * (pn2 - pn1) * 8; *bitrate = ((MP2T_PCR_CLOCK * bits_passed) / pcr_delta); + if (*bitrate == 0) { + /* pcr_delta < MP2T_PCR_CLOCK * bits_passed (pn2 != pn1, + * as that's the test for the loop above, so bits_passed + * is non-zero). + * + * That will produce a fractional bitrate, which turns + * into zero, causing a zero divide later. + * + * XXX - should we report this as "not ours"? A bitrate + * of less than 1 bit per second is not very useful for any + * form of audio/video, so presumably that's unlikely to + * be an MP2T file. + */ + return WTAP_OPEN_ERROR; + } return WTAP_OPEN_MINE; } @@ -290,8 +302,8 @@ mp2t_open(wtap *wth, int *err, gchar **err_info) guint8 buffer[MP2T_SIZE+TRAILER_LEN_MAX]; guint8 trailer_len = 0; guint sync_steps = 0; - int i; - int first; + guint i; + guint32 first = 0; mp2t_filetype_t *mp2t; wtap_open_return_val status; guint64 bitrate; @@ -303,17 +315,18 @@ mp2t_open(wtap *wth, int *err, gchar **err_info) return WTAP_OPEN_NOT_MINE; } - first = -1; for (i = 0; i < MP2T_SIZE; i++) { if (MP2T_SYNC_BYTE == buffer[i]) { first = i; - break; + goto found; } } - if (-1 == first) { - return WTAP_OPEN_NOT_MINE; /* wrong file type - not an mpeg2 ts file */ - } + /* + * No sync bytes found, so not an MPEG-2 Transport Stream file. + */ + return WTAP_OPEN_NOT_MINE; /* wrong file type - not an mpeg2 ts file */ +found: if (-1 == file_seek(wth->fh, first, SEEK_SET, err)) { return WTAP_OPEN_ERROR; } @@ -366,11 +379,6 @@ mp2t_open(wtap *wth, int *err, gchar **err_info) return status; } - if (bitrate == 0) { - /* Prevent an eventual divide by zero */ - return WTAP_OPEN_ERROR; - } - if (-1 == file_seek(wth->fh, first, SEEK_SET, err)) { return WTAP_OPEN_ERROR; }