the-tcpdump-group / tcpslice

tcpslice concatenates multiple pcap files together, or extracts time slices from one or more pcap files.
67 stars 23 forks source link

tcpslice doesn't like file names that begin with digits? #4

Closed infrastation closed 4 years ago

infrastation commented 8 years ago

Originally filed as tcpslice SF bug 3. Originally filed as tcpslice SF ticket 1800003. Submitted: Gabriel Friedmann ( gfriedmann ) - 2007-09-21 15:48:11 PDT

When i try to run tcpslice with an input file that contains dashes, i get an error. To reproduce, attempt the following command

tcpslice 2007-09-21_xxx_xxxxxxxxxxx_server_fixed_segmented.cap
tcpslice: at least one input file must be given

This is in tcpslice version 1.1a3

infrastation commented 8 years ago

This reproduces as described on the current git master of tcpslice.

guyharris commented 4 years ago

I've tried it with the current tip of the master branch, and it didn't core dump on macOS 10.15.4, at least. I don't know whether that's an issue of the file, the time, the build environment (64-bit x86, macOS, Apple clang version 11.0.3 (clang-1103.0.32.59)), or changes that have been made to it (this is version 1.2a1, or at least that's what it currently calls itself) since Mr. Friedmann tried it.

infrastation commented 4 years ago

The bug does reproduce for me on Linux and current master branch. The input file name triggers it:

$ cp ../tcpdump/tests/ieee802.11_rx-stbc.pcap .
$ ./tcpslice ieee802.11_rx-stbc.pcap 
tcpslice: stdout is a terminal; redirect or use -w
$ mv ieee802.11_rx-stbc.pcap 2007-09-21_xxx_xxxxxxxxxxx_server_fixed_segmented.cap
$ ./tcpslice 2007-09-21_xxx_xxxxxxxxxxx_server_fixed_segmented.cap 
tcpslice: at least one input file must be given

It is not a core dump, but an unexpected error message, because tcpclice tries to interpret the file name as a time value (tcpslice.c lines 236 onwards). Moreover, the man page describes this bug:

       An input filename that beings with a digit or a  `+'  can  be  confused
       with  a start/end time.  Such filenames can be specified with a leading
       `./';   for   example,   specify   the    file    `04Jul76.trace'    as
       `./04Jul76.trace'.

Whilst the statement above is true, it would be better to interpret a string as a time value iff the string can successfully be parsed as a time value, that is, fully conforms to the format and represents a valid time (not just begins with a character, as in the original report). Let me see if I can do that quickly.

guyharris commented 4 years ago

Fixed title to reflect reality - it's the initial digit, not the dash in the name, that's causing the problem, as per @infrastation's comments.

infrastation commented 4 years ago

Looking into respective code, I have found another semi-related issue, which I am going to fix, if not in the course of this bugfix, then on its own. Working on it.

infrastation commented 4 years ago

The changes look good in my working copy, I am going to make a couple more testing rounds today before committing.

infrastation commented 4 years ago

Committed to master, original issue resolved, closing.