osmith
/
wireshark
forked from osmocom/wireshark
1
0
Fork 0
Code Pull Requests Releases Wiki Activity

dot11crypt: add bounds check for TDLS elements 

Fixes a buffer overrun (read) of at most 255 bytes which could occur
while processing FTE in Dot11DecryptTDLSDeriveKey.

While at it, according to 802.11-2016 9.4.1.9, "A status code of
SUCCESS_POWER_SAVE_MODE also indicates a successful operation.". No idea
when it makes a difference, but let's implement it too.

Bug: 14686
Change-Id: Ia7a41cd965704a4d51fb5a4dc4d01885fc17375c
Fixes: v2.1.0rc0-1825-g6991149557 ("[airpdcap] Add support to decrypt TDLS traffic")
Link: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=8189
Reviewed-on: https://code.wireshark.org/review/27618
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
This commit is contained in:
Peter Wu 2018-05-17 17:31:54 +02:00 committed by Anders Broman
parent 39586110d8
commit f440561b8c
1 changed files with 30 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

40
epan/crypt/dot11decrypt.c
View File

@ -17,6 +17,7 @@
#include <wsutil/crc32.h>
#include <wsutil/pint.h>
#include <epan/proto.h> /* for DISSECTOR_ASSERT. */
#include <epan/tvbuff.h>
#include <epan/to_str.h>
#include <epan/strutil.h>
@ -529,6 +530,10 @@ static INT Dot11DecryptScanForKeys(
#endif
DOT11DECRYPT_DEBUG_TRACE_START("Dot11DecryptScanForKeys");
/* Callers provide these guarantees, so let's make them explicit. */
DISSECTOR_ASSERT(tot_len >= mac_header_len + DOT11DECRYPT_CRYPTED_DATA_MINLEN);
DISSECTOR_ASSERT(tot_len <= DOT11DECRYPT_MAX_CAPLEN);
/* cache offset in the packet data */
offset = mac_header_len;
@ -633,7 +638,9 @@ static INT Dot11DecryptScanForKeys(
guint status, offset_rsne = 0, offset_fte = 0, offset_link = 0, offset_timeout = 0;
DOT11DECRYPT_DEBUG_PRINT_LINE("Dot11DecryptScanForKeys", "Authentication: TDLS Action Frame", DOT11DECRYPT_DEBUG_LEVEL_3);
/* skip LLC header */
/* Skip LLC header, after this we have at least
* DOT11DECRYPT_CRYPTED_DATA_MINLEN-10 = 7 bytes in "data[offset]". That
* TDLS payload contains a TDLS Action field (802.11-2016 9.6.13) */
offset+=10;
/* check if the packet is a TDLS response or confirm */
@ -642,11 +649,11 @@ static INT Dot11DecryptScanForKeys(
DOT11DECRYPT_DEBUG_PRINT_LINE("Dot11DecryptScanForKeys", "Not Response nor confirm", DOT11DECRYPT_DEBUG_LEVEL_3);
return DOT11DECRYPT_RET_NO_VALID_HANDSHAKE;
}
/* check status */
offset++;
/* Check for SUCCESS (0) or SUCCESS_POWER_SAVE_MODE (85) Status Code */
status=pntoh16(data+offset);
if (status!=0) {
if (status != 0 && status != 85) {
DOT11DECRYPT_DEBUG_PRINT_LINE("Dot11DecryptScanForKeys", "TDLS setup not successfull", DOT11DECRYPT_DEBUG_LEVEL_3);
return DOT11DECRYPT_RET_NO_VALID_HANDSHAKE;
}
@ -657,20 +664,33 @@ static INT Dot11DecryptScanForKeys(
/* search for RSN, Fast BSS Transition, Link Identifier and Timeout Interval IEs */
while(offset < (tot_len - 2)) {
if (data[offset] == 48) {
guint8 element_id = data[offset];
guint8 length = data[offset + 1];
guint min_length = length;
switch (element_id) {
case 48: /* RSN (802.11-2016 9.4.2.35) */
offset_rsne = offset;
} else if (data[offset] == 55) {
min_length = 1;
break;
case 55: /* FTE (802.11-2016 9.4.2.48) */
offset_fte = offset;
} else if (data[offset] == 56) {
/* Plus variable length optional parameter(s) */
min_length = 2 + 16 + 32 + 32;
break;
case 56: /* Timeout Interval (802.11-2016 9.4.2.49) */
offset_timeout = offset;
} else if (data[offset] == 101) {
min_length = 1 + 4;
break;
case 101: /* Link Identifier (802.11-2016 9.4.2.62) */
offset_link = offset;
min_length = 6 + 6 + 6;
break;
}
if (tot_len < offset + data[offset + 1] + 2) {
if (length < min_length || tot_len < offset + 2 + length) {
return DOT11DECRYPT_RET_NO_VALID_HANDSHAKE;
}
offset += data[offset + 1] + 2;
offset += 2 + length;
}
if (offset_rsne == 0 || offset_fte == 0 ||