Skip to content

Commit 6a140ec

Browse files
committed
Fix packet length handling.
Treat the packet length as unsigned - it shouldn't be negative in the file. If it is, that'll probably cause the sscanf to fail, so we'll report the file as bad. Check it against WTAP_MAX_PACKET_SIZE to make sure we don't try to allocate a huge amount of memory, just as we do in other file readers. Use the now-validated packet size as the length in ws_buffer_assure_space(), so we are certain to have enough space, and don't allocate too much space. Merge the header and packet data parsing routines while we're at it. Bug: 12396 Change-Id: I7f981f9cdcbea7ecdeb88bfff2f12d875de2244f Reviewed-on: https://code.wireshark.org/review/15176 Reviewed-by: Guy Harris <guy@alum.mit.edu>
1 parent c245be9 commit 6a140ec

File tree

2 files changed

+40
-64
lines changed

2 files changed

+40
-64
lines changed

Diff for: wiretap/netscreen.c

+40-61
Original file line numberDiff line numberDiff line change
@@ -69,12 +69,8 @@ static gboolean netscreen_read(wtap *wth, int *err, gchar **err_info,
6969
static gboolean netscreen_seek_read(wtap *wth, gint64 seek_off,
7070
struct wtap_pkthdr *phdr, Buffer *buf,
7171
int *err, gchar **err_info);
72-
static int parse_netscreen_rec_hdr(struct wtap_pkthdr *phdr, const char *line,
73-
char *cap_int, gboolean *cap_dir, char *cap_dst,
74-
int *err, gchar **err_info);
75-
static gboolean parse_netscreen_hex_dump(FILE_T fh, int pkt_len,
76-
const char *cap_int, const char *cap_dst, struct wtap_pkthdr *phdr,
77-
Buffer* buf, int *err, gchar **err_info);
72+
static gboolean parse_netscreen_packet(FILE_T fh, struct wtap_pkthdr *phdr,
73+
Buffer* buf, char *line, int *err, gchar **err_info);
7874
static int parse_single_hex_dump_line(char* rec, guint8 *buf,
7975
guint byte_offset);
8076

@@ -191,27 +187,16 @@ static gboolean netscreen_read(wtap *wth, int *err, gchar **err_info,
191187
gint64 *data_offset)
192188
{
193189
gint64 offset;
194-
int pkt_len;
195190
char line[NETSCREEN_LINE_LENGTH];
196-
char cap_int[NETSCREEN_MAX_INT_NAME_LENGTH];
197-
gboolean cap_dir;
198-
char cap_dst[13];
199191

200192
/* Find the next packet */
201193
offset = netscreen_seek_next_packet(wth, err, err_info, line);
202194
if (offset < 0)
203195
return FALSE;
204196

205-
/* Parse the header */
206-
pkt_len = parse_netscreen_rec_hdr(&wth->phdr, line, cap_int, &cap_dir,
207-
cap_dst, err, err_info);
208-
if (pkt_len == -1)
209-
return FALSE;
210-
211-
/* Convert the ASCII hex dump to binary data, and fill in some
212-
struct wtap_pkthdr fields */
213-
if (!parse_netscreen_hex_dump(wth->fh, pkt_len, cap_int,
214-
cap_dst, &wth->phdr, wth->frame_buffer, err, err_info))
197+
/* Parse the header and convert the ASCII hex dump to binary data */
198+
if (!parse_netscreen_packet(wth->fh, &wth->phdr,
199+
wth->frame_buffer, line, err, err_info))
215200
return FALSE;
216201

217202
/*
@@ -239,11 +224,7 @@ netscreen_seek_read(wtap *wth, gint64 seek_off,
239224
struct wtap_pkthdr *phdr, Buffer *buf,
240225
int *err, gchar **err_info)
241226
{
242-
int pkt_len;
243227
char line[NETSCREEN_LINE_LENGTH];
244-
char cap_int[NETSCREEN_MAX_INT_NAME_LENGTH];
245-
gboolean cap_dir;
246-
char cap_dst[13];
247228

248229
if (file_seek(wth->random_fh, seek_off, SEEK_SET, err) == -1) {
249230
return FALSE;
@@ -257,15 +238,8 @@ netscreen_seek_read(wtap *wth, gint64 seek_off,
257238
return FALSE;
258239
}
259240

260-
pkt_len = parse_netscreen_rec_hdr(phdr, line, cap_int, &cap_dir,
261-
cap_dst, err, err_info);
262-
if (pkt_len == -1)
263-
return FALSE;
264-
265-
if (!parse_netscreen_hex_dump(wth->random_fh, pkt_len, cap_int,
266-
cap_dst, phdr, buf, err, err_info))
267-
return FALSE;
268-
return TRUE;
241+
return parse_netscreen_packet(wth->random_fh, phdr, buf, line,
242+
err, err_info);
269243
}
270244

271245
/* Parses a packet record header. There are a few possible formats:
@@ -285,49 +259,54 @@ netscreen_seek_read(wtap *wth, gint64 seek_off,
285259
286260
287261
*/
288-
static int
289-
parse_netscreen_rec_hdr(struct wtap_pkthdr *phdr, const char *line, char *cap_int,
290-
gboolean *cap_dir, char *cap_dst, int *err, gchar **err_info)
262+
static gboolean
263+
parse_netscreen_packet(FILE_T fh, struct wtap_pkthdr *phdr, Buffer* buf,
264+
char *line, int *err, gchar **err_info)
291265
{
292-
int sec;
293-
int dsec, pkt_len;
294-
char direction[2];
295-
char cap_src[13];
266+
int sec;
267+
int dsec;
268+
char cap_int[NETSCREEN_MAX_INT_NAME_LENGTH];
269+
char direction[2];
270+
guint pkt_len;
271+
char cap_src[13];
272+
char cap_dst[13];
273+
guint8 *pd;
274+
gchar *p;
275+
int n, i = 0;
276+
guint offset = 0;
277+
gchar dststr[13];
296278

297279
phdr->rec_type = REC_TYPE_PACKET;
298280
phdr->presence_flags = WTAP_HAS_TS|WTAP_HAS_CAP_LEN;
299281

300-
if (sscanf(line, "%9d.%9d: %15[a-z0-9/:.-](%1[io]) len=%9d:%12s->%12s/",
282+
if (sscanf(line, "%9d.%9d: %15[a-z0-9/:.-](%1[io]) len=%9u:%12s->%12s/",
301283
&sec, &dsec, cap_int, direction, &pkt_len, cap_src, cap_dst) < 5) {
302284
*err = WTAP_ERR_BAD_FILE;
303285
*err_info = g_strdup("netscreen: Can't parse packet-header");
304286
return -1;
305287
}
288+
if (pkt_len > WTAP_MAX_PACKET_SIZE) {
289+
/*
290+
* Probably a corrupt capture file; don't blow up trying
291+
* to allocate space for an immensely-large packet.
292+
*/
293+
*err = WTAP_ERR_BAD_FILE;
294+
*err_info = g_strdup_printf("netscreen: File has %u-byte packet, bigger than maximum of %u",
295+
pkt_len, WTAP_MAX_PACKET_SIZE);
296+
return FALSE;
297+
}
306298

307-
*cap_dir = (direction[0] == 'o' ? NETSCREEN_EGRESS : NETSCREEN_INGRESS);
299+
/*
300+
* If direction[0] is 'o', the direction is NETSCREEN_EGRESS,
301+
* otherwise it's NETSCREEN_INGRESS.
302+
*/
308303

309304
phdr->ts.secs = sec;
310305
phdr->ts.nsecs = dsec * 100000000;
311306
phdr->len = pkt_len;
312307

313-
return pkt_len;
314-
}
315-
316-
/* Converts ASCII hex dump to binary data, and fills in some struct
317-
wtap_pkthdr fields. Returns TRUE on success and FALSE on any error. */
318-
static gboolean
319-
parse_netscreen_hex_dump(FILE_T fh, int pkt_len, const char *cap_int,
320-
const char *cap_dst, struct wtap_pkthdr *phdr, Buffer* buf,
321-
int *err, gchar **err_info)
322-
{
323-
guint8 *pd;
324-
gchar line[NETSCREEN_LINE_LENGTH];
325-
gchar *p;
326-
int n, i = 0, offset = 0;
327-
gchar dststr[13];
328-
329308
/* Make sure we have enough room for the packet */
330-
ws_buffer_assure_space(buf, NETSCREEN_MAX_PACKET_LEN);
309+
ws_buffer_assure_space(buf, pkt_len);
331310
pd = ws_buffer_start_ptr(buf);
332311

333312
while(1) {
@@ -373,7 +352,7 @@ parse_netscreen_hex_dump(FILE_T fh, int pkt_len, const char *cap_int,
373352
/* If there is no more data and the line was not empty,
374353
* then there must be an error in the file
375354
*/
376-
if(n == -1) {
355+
if (n == -1) {
377356
*err = WTAP_ERR_BAD_FILE;
378357
*err_info = g_strdup("netscreen: cannot parse hex-data");
379358
return FALSE;
@@ -385,7 +364,7 @@ parse_netscreen_hex_dump(FILE_T fh, int pkt_len, const char *cap_int,
385364
/* If there was more hex-data than was announced in the len=x
386365
* header, then then there must be an error in the file
387366
*/
388-
if(offset > pkt_len) {
367+
if (offset > pkt_len) {
389368
*err = WTAP_ERR_BAD_FILE;
390369
*err_info = g_strdup("netscreen: too much hex-data");
391370
return FALSE;

Diff for: wiretap/netscreen.h

-3
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,6 @@
4646
#define NETSCREEN_INGRESS FALSE
4747
#define NETSCREEN_EGRESS TRUE
4848

49-
50-
#define NETSCREEN_MAX_PACKET_LEN 65536
51-
5249
wtap_open_return_val netscreen_open(wtap *wth, int *err, gchar **err_info);
5350

5451
#endif

0 commit comments

Comments
 (0)