Skip to content

Commit 101821b

Browse files
committed
DAG: Validate capture device names better.
The problem space of validating a DAG capture device name for libpcap purposes overlaps to an extent only with the current solution space of the validation code in dag_create() and the call to dag_parse_name() in dag_activate(). In dag_create() make the validation strict and syntax-only; when a rejected device name is likely enough a DAG device name, reject it authoritatively with an error message. In dag_activate() remove the now redundant call to dag_parse_name() and do semantic validation producing better error codes and messages. Add comments to show what exactly needs to be done and how/where it is done. Before: $ tcpdump -ni dag-1:2 tcpdump: This version of libpcap only supports DAG cards $ tcpdump -ni dag256:32 tcpdump: This version of libpcap only supports DAG cards $ tcpdump -ni dag17:256 tcpdump: This version of libpcap only supports DAG cards After: $ tcpdump -ni dag-1:2 tcpdump: DAG device name "dag-1:2" is invalid $ tcpdump -ni dag256:32 tcpdump: dag256:32: No such device exists (DAG device number 256 is too large) $ tcpdump -ni dag17:256 tcpdump: dag17:256: No such device exists (DAG stream number 256 is too large)
1 parent 3b7e305 commit 101821b

File tree

2 files changed

+71
-22
lines changed

2 files changed

+71
-22
lines changed

CHANGES

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ DayOfTheWeek, Month DD, YYYY / The Tcpdump Group
7878
Require the API to have 64-bit streams support.
7979
Make device descriptions more useful, make dagN conditional.
8080
Use PCAP_ERROR_NO_SUCH_DEVICE more in dag_activate().
81+
Validate capture device names better.
8182

8283
DayOfTheWeek, Month DD, YYYY / The Tcpdump Group
8384
Summary for 1.10.6 libpcap release (so far!)

pcap-dag.c

Lines changed: 70 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include <string.h>
1414
#include <errno.h>
1515
#include <endian.h>
16+
#include <limits.h>
1617

1718
#include "pcap-int.h"
1819

@@ -62,7 +63,8 @@ struct pcap_dag {
6263
u_char *dag_mem_top; /* DAG card current memory top pointer */
6364
int dag_fcs_bits; /* Number of checksum bits from link layer */
6465
int dag_flags; /* Flags */
65-
int dag_stream; /* DAG stream number */
66+
int dag_devnum; /* This is the N in "dagN" or "dagN:M". */
67+
int dag_stream; /* And this is the M. */
6668
int dag_timeout; /* timeout specified to pcap_open_live.
6769
* Same as in linux above, introduce
6870
* generally? */
@@ -624,26 +626,29 @@ static int dag_activate(pcap_t* p)
624626
char *s;
625627
int n;
626628
daginf_t* daginf;
627-
char device[DAGNAME_BUFSIZE];
629+
char * device = p->opt.device;
628630
int ret;
629631
dag_size_t mindata;
630632
struct timeval maxwait;
631633
struct timeval poll;
632634

633-
/* Parse input name to get dag device and stream number if provided */
634-
if (dag_parse_name(p->opt.device, device, sizeof(device), &pd->dag_stream) < 0) {
635-
/*
636-
* XXX - it'd be nice if this indicated what was wrong
637-
* with the name. Does this reliably set errno?
638-
* Should this return PCAP_ERROR_NO_SUCH_DEVICE in some
639-
* cases?
640-
*/
641-
ret = PCAP_ERROR;
642-
pcapint_fmt_errmsg_for_errno(p->errbuf, PCAP_ERRBUF_SIZE,
643-
errno, "dag_parse_name");
635+
/*
636+
* dag_create() has validated the device name syntax and stored the
637+
* parsed device and stream numbers to p->priv. Validate these values
638+
* semantically.
639+
*/
640+
if (pd->dag_devnum >= DAG_MAX_BOARDS) {
641+
snprintf(p->errbuf, PCAP_ERRBUF_SIZE,
642+
"DAG device number %d is too large", pd->dag_devnum);
643+
ret = PCAP_ERROR_NO_SUCH_DEVICE;
644+
goto fail;
645+
}
646+
if (pd->dag_stream >= DAG_STREAM_MAX) {
647+
snprintf(p->errbuf, PCAP_ERRBUF_SIZE,
648+
"DAG stream number %d is too large", pd->dag_stream);
649+
ret = PCAP_ERROR_NO_SUCH_DEVICE;
644650
goto fail;
645651
}
646-
647652
if (pd->dag_stream%2) {
648653
/*
649654
* dag_findalldevs() does not return any Tx streams, so
@@ -924,12 +929,21 @@ pcap_t *dag_create(const char *device, char *ebuf, int *is_ours)
924929
pcap_t *p;
925930
long stream = 0;
926931

932+
/*
933+
* The nominal libpcap DAG device name format is either "dagN" or
934+
* "dagN:M", as returned from dag_findalldevs().
935+
*
936+
* First attempt the most basic syntax validation. If the device string
937+
* does not look like a potentially valid DAG device name, reject it
938+
* silently to have pcap_create() try another capture source type.
939+
*/
940+
*is_ours = 0;
941+
927942
/* Does this look like a DAG device? */
928943
cp = device;
929944
/* Does it begin with "dag"? */
930945
if (strncmp(cp, "dag", 3) != 0) {
931946
/* Nope, doesn't begin with "dag" */
932-
*is_ours = 0;
933947
return NULL;
934948
}
935949
/* Yes - is "dag" followed by a number from 0 to DAG_MAX_BOARDS-1 */
@@ -942,25 +956,56 @@ pcap_t *dag_create(const char *device, char *ebuf, int *is_ours)
942956

943957
if (cpend == cp || *cpend != '\0') {
944958
/* Not followed by a number. */
945-
*is_ours = 0;
946959
return NULL;
947960
}
948961

949-
if (devnum < 0 || devnum >= DAG_MAX_BOARDS) {
962+
/*
963+
* OK, it's probably ours, validate the syntax further. From now on
964+
* reject the device string authoritatively with an error message to
965+
* have pcap_create() propagate the failure. Validate the device and
966+
* stream number ranges loosely only.
967+
*/
968+
*is_ours = 1;
969+
snprintf (ebuf, PCAP_ERRBUF_SIZE,
970+
"DAG device name \"%s\" is invalid", device);
971+
972+
if (devnum < 0 || devnum > INT_MAX) {
950973
/* Followed by a non-valid number. */
951-
*is_ours = 0;
952974
return NULL;
953975
}
954976

955-
if (stream <0 || stream >= DAG_STREAM_MAX) {
977+
if (stream < 0 || stream > INT_MAX) {
956978
/* Followed by a non-valid stream number. */
957-
*is_ours = 0;
958979
return NULL;
959980
}
960981

961-
/* OK, it's probably ours. */
962-
*is_ours = 1;
982+
/*
983+
* The syntax validation done so far is lax enough to accept some
984+
* device strings that are not actually acceptable in libpcap as
985+
* defined above. The device strings that are acceptable in libpcap
986+
* are a strict subset of the device strings that are acceptable in
987+
* dag_parse_name(), thus using the latter for validation in libpcap
988+
* would not work reliably. Instead from the detected device and
989+
* stream numbers produce the acceptable device string(s) and require
990+
* the input device string to match an acceptable string exactly.
991+
*/
992+
char buf[DAGNAME_BUFSIZE];
993+
snprintf(buf, sizeof(buf), "dag%ld:%ld", devnum, stream);
994+
char acceptable = ! strcmp(device, buf);
995+
if (! acceptable && stream == 0) {
996+
snprintf(buf, sizeof(buf), "dag%ld", devnum);
997+
acceptable = ! strcmp(device, buf);
998+
}
999+
if (! acceptable)
1000+
return NULL;
9631001

1002+
/*
1003+
* The device string syntax is acceptable, save the device and stream
1004+
* numbers for dag_activate(), which will do semantic and run-time
1005+
* validation and possibly reject the pcap_t using more specific error
1006+
* codes.
1007+
*/
1008+
ebuf[0] = '\0';
9641009
p = PCAP_CREATE_COMMON(ebuf, struct pcap_dag);
9651010
if (p == NULL)
9661011
return NULL;
@@ -984,6 +1029,9 @@ pcap_t *dag_create(const char *device, char *ebuf, int *is_ours)
9841029
p->tstamp_precision_list[0] = PCAP_TSTAMP_PRECISION_MICRO;
9851030
p->tstamp_precision_list[1] = PCAP_TSTAMP_PRECISION_NANO;
9861031
p->tstamp_precision_count = 2;
1032+
struct pcap_dag *pd = p->priv;
1033+
pd->dag_devnum = (int)devnum;
1034+
pd->dag_stream = (int)stream;
9871035
return p;
9881036
}
9891037

0 commit comments

Comments
 (0)