17 июля 2015 г. 20:07 пользователь "Stefan Sperling" <s...@stsp.name> написал:
>
> On Fri, Jul 17, 2015 at 03:59:12AM +0300, Vadim Zhukov wrote:
> > 2015-07-17 2:53 GMT+03:00 Stefan Sperling <s...@stsp.name>:
> > > I don't think we should bother with invalid data. If it's not valid,
> > > we should silently skip it. Else tcpdump would have to print all sorts
> > > of warnings.
> >
> > I don't insist on explicit warning ("invalid nchan" or something like
> > that), no. But, IMHO, we should display data we gathered without
> > mangling; otherwise, we'll fool the user that nchan is 1 when it's 0
> > actually.
>
> Fair enough. I figure we can roll this into the code path for operating
> element IDs (which were also skipped because because I don't find them
> very interesting) and print the data in raw form.
>
> Like this?
Yeah, I like this. But I found one more issue. :)
> Index: print-802_11.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/tcpdump/print-802_11.c,v
> retrieving revision 1.19
> diff -u -p -r1.19 print-802_11.c
> --- print-802_11.c 16 Jul 2015 20:57:13 -0000 1.19
> +++ print-802_11.c 17 Jul 2015 17:00:02 -0000
> @@ -78,6 +78,7 @@ int ieee80211_hdr(struct ieee80211_fram
> int ieee80211_data(struct ieee80211_frame *, u_int);
> void ieee80211_print_element(u_int8_t *, u_int);
> void ieee80211_print_essid(u_int8_t *, u_int);
> +void ieee80211_print_country(u_int8_t *, u_int);
> void ieee80211_print_htcaps(u_int8_t *, u_int);
> int ieee80211_elements(struct ieee80211_frame *, u_int);
> int ieee80211_frame(struct ieee80211_frame *, u_int);
> @@ -233,6 +234,48 @@ ieee80211_print_essid(u_int8_t *essid, u
>
> /* Caller checks len */
> void
> +ieee80211_print_country(u_int8_t *data, u_int len)
> +{
> + u_int8_t first_chan, nchan, maxpower;
> +
> + if (len < 6)
> + return;
> +
> + /* country string */
> + printf(" '%c%c%c'", data[0], data[1], data[2]);
Looking here again, I see direct printing of untrusted data. Other
tcpdump parts use isprint() check for safety, and print either some
other character, or character code instead:
print-icmp6.c: printf((isprint(*cp) ? "%c" : "\\%03o"), *cp);
print-ike.c: printf("%c",(isprint(*p) ? *p : '.'));
The tcpdump is hard. :(
> +
> + len -= 3;
> + data += 3;
> +
> + /* channels and corresponding TX power limits */
> + while (len > 0) {
> + if (len < 3)
> + break;
BTW, you can just write "while (len >= 3)" then. ;)
> + /* no pretty-printing for nonsensical zero values,
> + * nor for operating extension IDs (values >= 201) */
> + if (data[0] == 0 || data[1] == 0 ||
> + data[0] >= 201 || data[1] >= 201) {
> + printf(", %d %d %d", data[0], data[1], data[2]);
> + continue;
> + }
> +
> + first_chan = data[0];
> + nchan = data[1];
> + maxpower = data[2];
> +
> + printf(", channel%s %d", nchan == 1 ? "" : "s", first_chan);
> + if (nchan > 1)
> + printf("-%d", first_chan + nchan - 1);
> + printf(" limit %ddB", maxpower);
> +
> + len -= 3;
> + data += 3;
> + }
> +}
> +
> +/* Caller checks len */
> +void
> ieee80211_print_htcaps(u_int8_t *data, u_int len)
> {
> u_int16_t htcaps;
> @@ -392,8 +435,7 @@ ieee80211_elements(struct ieee80211_fram
> break;
> case IEEE80211_ELEMID_COUNTRY:
> printf(", country");
> - for (i = len; i > 0; i--, data++)
> - printf(" %u", data[0]);
> + ieee80211_print_country(data, len);
> break;
> case IEEE80211_ELEMID_CHALLENGE:
> printf(", challenge");
> @@ -436,6 +478,10 @@ ieee80211_elements(struct ieee80211_fram
> printf(", htcaps");
> if (vflag)
> ieee80211_print_htcaps(data, len);
> + break;
> + case IEEE80211_ELEMID_POWER_CONSTRAINT:
> + ELEM_CHECK(1);
> + printf(", power constraint %udB", data[0]);
> break;
> case IEEE80211_ELEMID_VENDOR:
> printf(", vendor");
--
Vadim Zhukov
