Re: tcpdump: decode 802.11 QoS frames correctly

[Stefan Sperling]

On Sun, Jan 31, 2016 at 03:14:46PM +0100, Mark Kettenis wrote:
> I think you should add a TCHECK somewhere at the top for the qos frame
> size otherwise this might look beyond the end of the buffer for
> specially crafted frames.

Oh yes, indeed.

Index: print-802_11.c
===================================================================
RCS file: /cvs/src/usr.sbin/tcpdump/print-802_11.c,v
retrieving revision 1.28
diff -u -p -r1.28 print-802_11.c
--- print-802_11.c      12 Jan 2016 09:28:10 -0000      1.28
+++ print-802_11.c      31 Jan 2016 19:56:07 -0000
@@ -153,14 +153,25 @@ int
 ieee80211_data(struct ieee80211_frame *wh, u_int len)
 {
        u_int8_t *t = (u_int8_t *)wh;
-       struct ieee80211_frame_addr4 *w4;
        u_int datalen;
        int data = !(wh->i_fc[0] & IEEE80211_FC0_SUBTYPE_NODATA);
+       int hasqos = ((wh->i_fc[0] &
+           (IEEE80211_FC0_TYPE_MASK | IEEE80211_FC0_SUBTYPE_QOS)) ==
+           (IEEE80211_FC0_TYPE_DATA | IEEE80211_FC0_SUBTYPE_QOS));
        u_char *esrc = NULL, *edst = NULL;
 
-       TCHECK(*wh);
-       t += sizeof(struct ieee80211_frame);
-       datalen = len - sizeof(struct ieee80211_frame);
+       if (hasqos) {
+               struct ieee80211_qosframe *wq;
+
+               wq = (struct ieee80211_qosframe *) wh;
+               TCHECK(*wq);
+               t += sizeof(*wq);
+               datalen = len - sizeof(*wq);
+       } else {
+               TCHECK(*wh);
+               t += sizeof(*wh);
+               datalen = len - sizeof(*wh);
+       }
 
        switch (wh->i_fc[1] & IEEE80211_FC1_DIR_MASK) {
        case IEEE80211_FC1_DIR_TODS:
@@ -176,12 +187,25 @@ ieee80211_data(struct ieee80211_frame *w
                edst = wh->i_addr1;
                break;
        case IEEE80211_FC1_DIR_DSTODS:
-               w4 = (struct ieee80211_frame_addr4 *) wh;
-               TCHECK(*w4);
-               t = (u_int8_t *) (w4 + 1);
-               datalen = len - sizeof(*w4);
-               esrc = w4->i_addr4;
-               edst = w4->i_addr3;
+               if (hasqos) {
+                       struct ieee80211_qosframe_addr4 *w4;
+
+                       w4 = (struct ieee80211_qosframe_addr4 *) wh;
+                       TCHECK(*w4);
+                       t = (u_int8_t *) (w4 + 1);
+                       datalen = len - sizeof(*w4);
+                       esrc = w4->i_addr4;
+                       edst = w4->i_addr3;
+               } else {
+                       struct ieee80211_frame_addr4 *w4;
+
+                       w4 = (struct ieee80211_frame_addr4 *) wh;
+                       TCHECK(*w4);
+                       t = (u_int8_t *) (w4 + 1);
+                       datalen = len - sizeof(*w4);
+                       esrc = w4->i_addr4;
+                       edst = w4->i_addr3;
+               }
                break;
        }