Skip to content

Commit bf563e1

Browse files
committed
DAG: Validate environment variables better. [skip ci]
Make the environment variables named constants. Add a helper function to convert strings to integers using a more thorough validation, such that if it returns a non-negative integer for an input string, the string definitely represents the number. In dag_activate() reject integers that are not one of the valid values. In case of a validation error include the rejected string value in the error message, not the parsed integer value. Before: # ERF_FCS_BITS=invalid tcpdump -ni dag0 (works as if ERF_FCS_BITS=0) # ERF_DONT_STRIP_FCS=never tcpdump -ni dag0 (works as if ERF_DONT_STRIP_FCS=1) After: # ERF_FCS_BITS=invalid tcpdump -ni dag0 tcpdump: dag_activate dag0: invalid ERF_FCS_BITS value (invalid) in environment # ERF_DONT_STRIP_FCS=never tcpdump -ni dag0 tcpdump: dag_activate dag0: invalid ERF_DONT_STRIP_FCS value (never) in environment
1 parent e645387 commit bf563e1

File tree

3 files changed

+54
-16
lines changed

3 files changed

+54
-16
lines changed

CHANGES

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,8 @@ DayOfTheWeek, Month DD, YYYY / The Tcpdump Group
8080
Use PCAP_ERROR_NO_SUCH_DEVICE more in dag_activate().
8181
Validate capture device names better.
8282
Enumerate streams without trying to lock them.
83+
If the environment variable ERF_FCS_BITS is set, require it to be either
84+
0 or 16 or 32. Likewise for ERF_DONT_STRIP_FCS (either 0 or 1).
8385

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

doc/README.dag

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ a HDLC/PoS/PPP/Frame Relay link with 16 bit CRC/FCS, then set
106106
ERF_FCS_BITS=16.
107107

108108
If you wish to create a pcap file that DOES contain the Ethernet FCS,
109-
specify the environment variable ERF_DONT_STRIP_FCS. This will cause
109+
specify the environment variable ERF_DONT_STRIP_FCS=1. This will cause
110110
the existing FCS to be captured into the pcap file. Note some
111111
applications may incorrectly report capture errors or oversize packets
112112
when reading these files.

pcap-dag.c

Lines changed: 51 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,30 @@ static int dag_set_datalink(pcap_t *p, int dlt);
9393
static int dag_get_datalink(pcap_t *p);
9494
static int dag_setnonblock(pcap_t *p, int nonblock);
9595

96+
// Environment variables that can control behaviour of this libpcap module.
97+
#define ENV_RX_FCS_BITS "ERF_FCS_BITS"
98+
#define ENV_RX_FCS_NOSTRIP "ERF_DONT_STRIP_FCS"
99+
100+
/*
101+
* Convert the return value of getenv() to an integer using matching stricter
102+
* than atoi(). If the environment variable is not set, return the default
103+
* value. Otherwise return an integer in the interval [0, INT32_MAX] or -1 on
104+
* error.
105+
*/
106+
static int32_t
107+
strtouint31(const char *str, const int32_t defaultval) {
108+
if (! str)
109+
return defaultval;
110+
if (! str[0])
111+
return -1;
112+
113+
char * endp;
114+
unsigned long val = strtoul(str, &endp, 10);
115+
if (*endp || val > INT32_MAX)
116+
return -1;
117+
return (int32_t)val;
118+
}
119+
96120
static void
97121
delete_pcap_dag(const pcap_t *p)
98122
{
@@ -829,33 +853,45 @@ static int dag_activate(pcap_t* p)
829853
p->linktype_ext = LT_FCS_DATALINK_EXT(0);
830854
} else {
831855
/*
832-
* Start out assuming it's 32 bits.
856+
* Assume Rx FCS length to be 32 bits unless the user has
857+
* requested a different value, in which case validate it well.
833858
*/
834-
pd->dag_fcs_bits = 32;
835-
836-
/* Allow an environment variable to override. */
837-
if ((s = getenv("ERF_FCS_BITS")) != NULL) {
838-
if ((n = atoi(s)) == 0 || n == 16 || n == 32) {
839-
pd->dag_fcs_bits = n;
840-
} else {
841-
ret = PCAP_ERROR;
842-
snprintf(p->errbuf, PCAP_ERRBUF_SIZE,
843-
"%s %s: bad ERF_FCS_BITS value (%d) in environment",
844-
__func__, device, n);
845-
goto failstop;
846-
}
859+
s = getenv(ENV_RX_FCS_BITS);
860+
switch ((n = strtouint31(s, 32))) {
861+
case 0:
862+
case 16:
863+
case 32:
864+
pd->dag_fcs_bits = n;
865+
break;
866+
default:
867+
ret = PCAP_ERROR;
868+
snprintf(p->errbuf, PCAP_ERRBUF_SIZE,
869+
"%s %s: invalid %s value (%s) in environment",
870+
__func__, device, ENV_RX_FCS_BITS, s);
871+
goto failstop;
847872
}
848873

849874
/*
850875
* Did the user request that they not be stripped?
851876
*/
852-
if ((s = getenv("ERF_DONT_STRIP_FCS")) != NULL) {
877+
s = getenv(ENV_RX_FCS_NOSTRIP);
878+
switch ((n = strtouint31(s, 0))) {
879+
case 0:
880+
break;
881+
case 1:
853882
/* Yes. Note the number of 16-bit words that will be
854883
supplied. */
855884
p->linktype_ext = LT_FCS_DATALINK_EXT(pd->dag_fcs_bits/16);
856885

857886
/* And don't strip them. */
858887
pd->dag_fcs_bits = 0;
888+
break;
889+
default:
890+
ret = PCAP_ERROR;
891+
snprintf(p->errbuf, PCAP_ERRBUF_SIZE,
892+
"%s %s: invalid %s value (%s) in environment",
893+
__func__, device, ENV_RX_FCS_NOSTRIP, s);
894+
goto failstop;
859895
}
860896
}
861897

0 commit comments

Comments
 (0)