2020-02-09 17:00:22

by Markus Theil

[permalink] [raw]
Subject: [PATCH 0/8] iw: parse measurement pilot and fix scan bugs

Besides adding a parser for the measurement pilot element, this
series fixes several bugs found while fuzzing the scan code of iw.

Markus Theil (8):
iw: scan: parse measurement pilot element
iw: scan: fix buffer over-read in print_ies()
iw: scan: fix buffer over-read in operation class parsing
iw: scan: fix buffer over-read in parsing roaming consortium
iw: scan: fix buffer over-read in print_wifi_wps
iw: scan: fix buffer over-read in print_p2p
iw: scan: fix undefined behaviour in rm capa print
iw: scan: fix undefined behaviour in print_vht_capa()

scan.c | 71 +++++++++++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 58 insertions(+), 13 deletions(-)

--
2.25.0


2020-02-09 17:00:22

by Markus Theil

[permalink] [raw]
Subject: [PATCH 3/8] iw: scan: fix buffer over-read in operation class parsing

Signed-off-by: Markus Theil <[email protected]>
---
scan.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/scan.c b/scan.c
index 14138ca..2d11f81 100644
--- a/scan.c
+++ b/scan.c
@@ -1507,7 +1507,7 @@ static void print_supp_op_classes(const uint8_t type, uint8_t len,

printf("\n");
printf("\t\t * current operating class: %d\n", *p);
- while (p++ < next_data) {
+ while (++p < next_data) {
if (*p == 130) {
one_hundred_thirty_delimiter = 1;
break;
@@ -1519,11 +1519,11 @@ static void print_supp_op_classes(const uint8_t type, uint8_t len,
printf("\t\t * operating class: %d\n", *p);
}
if (one_hundred_thirty_delimiter)
- while (p++ < next_data) {
+ while (++p < next_data) {
printf("\t\t * current operating class extension: %d\n", *p);
}
if (zero_delimiter)
- while (p++ < next_data - 1) {
+ while (++p < next_data - 1) {
printf("\t\t * operating class duple: %d %d\n", p[0], p[1]);
if (*p == 0)
break;
--
2.25.0

2020-02-09 17:01:26

by Markus Theil

[permalink] [raw]
Subject: [PATCH 1/8] iw: scan: parse measurement pilot element

Signed-off-by: Markus Theil <[email protected]>
---
scan.c | 42 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 42 insertions(+)

diff --git a/scan.c b/scan.c
index 50a4147..a6cb3bb 100644
--- a/scan.c
+++ b/scan.c
@@ -1530,6 +1530,47 @@ static void print_supp_op_classes(const uint8_t type, uint8_t len,
}
}

+static void print_measurement_pilot_tx(const uint8_t type, uint8_t len,
+ const uint8_t *data,
+ const struct print_ies_data *ie_buffer)
+{
+ printf("\n");
+ printf("\t\t * interval: %d TUs\n", data[0]);
+
+ if(len <= 1)
+ return;
+
+ uint8_t *p = (uint8_t *) data + 1;
+ uint8_t len_remaining = len - 1;
+
+ while (len_remaining >=5) {
+ uint8_t subelement_id = *p;
+ ++p;
+ uint8_t len = *p;
+ ++p;
+
+ len_remaining -= 2;
+
+ /* 802.11-2016 only allows vendor specific elements */
+ if (subelement_id != 221) {
+ printf("\t\t * <Invalid subelement ID %d>\n", subelement_id);
+ return;
+ }
+
+ printf("\t\t * vendor specific: OUI %.2x:%.2x:%.2x, data:",
+ p[0], p[1], p[2]);
+ len_remaining -= 3;
+
+ if (len > len_remaining)
+ printf(" <Parse error, element too short>\n");
+ return;
+
+ while (p < p + len)
+ printf(" %.2x", *p);
+ printf("\n");
+ }
+}
+
static void print_obss_scan_params(const uint8_t type, uint8_t len,
const uint8_t *data,
const struct print_ies_data *ie_buffer)
@@ -1652,6 +1693,7 @@ static const struct ie_print ieprinters[] = {
[45] = { "HT capabilities", print_ht_capa, 26, 26, BIT(PRINT_SCAN), },
[47] = { "ERP D4.0", print_erp, 1, 255, BIT(PRINT_SCAN), },
[59] = { "Supported operating classes", print_supp_op_classes, 1, 255, BIT(PRINT_SCAN), },
+ [66] = { "Measurement Pilot Transmission", print_measurement_pilot_tx, 1, 255, BIT(PRINT_SCAN), },
[74] = { "Overlapping BSS scan params", print_obss_scan_params, 14, 255, BIT(PRINT_SCAN), },
[61] = { "HT operation", print_ht_op, 22, 22, BIT(PRINT_SCAN), },
[62] = { "Secondary Channel Offset", print_secchan_offs, 1, 1, BIT(PRINT_SCAN), },
--
2.25.0

2020-02-09 17:01:27

by Markus Theil

[permalink] [raw]
Subject: [PATCH 5/8] iw: scan: fix buffer over-read in print_wifi_wps

Signed-off-by: Markus Theil <[email protected]>
---
scan.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scan.c b/scan.c
index 9a02363..f57925d 100644
--- a/scan.c
+++ b/scan.c
@@ -1818,7 +1818,7 @@ static void print_wifi_wps(const uint8_t type, uint8_t len, const uint8_t *data,
while (len >= 4) {
subtype = (data[0] << 8) + data[1];
sublen = (data[2] << 8) + data[3];
- if (sublen > len)
+ if (sublen > len - 4)
break;

switch (subtype) {
--
2.25.0

2020-02-09 17:01:38

by Markus Theil

[permalink] [raw]
Subject: [PATCH 7/8] iw: scan: fix undefined behaviour in rm capa print

Signed-off-by: Markus Theil <[email protected]>
---
scan.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/scan.c b/scan.c
index b3e7baa..e2a620a 100644
--- a/scan.c
+++ b/scan.c
@@ -586,10 +586,10 @@ static void print_rm_enabled_capabilities(const uint8_t type, uint8_t len,
const uint8_t *data,
const struct print_ies_data *ie_buffer)
{
- __u64 capa = data[0] |
- data[1] << 8 |
- data[2] << 16 |
- data[3] << 24 |
+ __u64 capa = ((__u64) data[0]) |
+ ((__u64) data[1]) << 8 |
+ ((__u64) data[2]) << 16 |
+ ((__u64) data[3]) << 24 |
((__u64) data[4]) << 32;

printf("\n");
--
2.25.0

2020-02-09 17:01:42

by Markus Theil

[permalink] [raw]
Subject: [PATCH 8/8] iw: scan: fix undefined behaviour in print_vht_capa()

Signed-off-by: Markus Theil <[email protected]>
---
scan.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scan.c b/scan.c
index e2a620a..8f8d8ba 100644
--- a/scan.c
+++ b/scan.c
@@ -1473,8 +1473,8 @@ static void print_vht_capa(const uint8_t type, uint8_t len, const uint8_t *data,
const struct print_ies_data *ie_buffer)
{
printf("\n");
- print_vht_info(data[0] | (data[1] << 8) |
- (data[2] << 16) | (data[3] << 24),
+ print_vht_info((__u32) data[0] | ((__u32)data[1] << 8) |
+ ((__u32)data[2] << 16) | ((__u32)data[3] << 24),
data + 4);
}

--
2.25.0

2020-02-09 17:02:33

by Markus Theil

[permalink] [raw]
Subject: [PATCH 6/8] iw: scan: fix buffer over-read in print_p2p

Signed-off-by: Markus Theil <[email protected]>
---
scan.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scan.c b/scan.c
index f57925d..b3e7baa 100644
--- a/scan.c
+++ b/scan.c
@@ -2036,7 +2036,7 @@ static inline void print_p2p(const uint8_t type, uint8_t len,
case 0x12: /* invitation flags */
case 0xdd: /* vendor specific */
default: {
- const __u8 *subdata = data + 4;
+ const __u8 *subdata = data + 3;
__u16 tmplen = sublen;

tab_on_first(&first);
--
2.25.0

2020-02-10 08:11:57

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 0/8] iw: parse measurement pilot and fix scan bugs

On Sun, 2020-02-09 at 17:58 +0100, Markus Theil wrote:
> this
> series fixes several bugs found while fuzzing the scan code of iw.

Nice, can you describe the setup you used for this?

johannes

2020-02-10 08:23:17

by Markus Theil

[permalink] [raw]
Subject: Re: [PATCH 0/8] iw: parse measurement pilot and fix scan bugs

On 2/10/20 9:11 AM, Johannes Berg wrote:
> On Sun, 2020-02-09 at 17:58 +0100, Markus Theil wrote:
>> this
>> series fixes several bugs found while fuzzing the scan code of iw.
> Nice, can you describe the setup you used for this?
>
> johannes
>
I used clang with its sanitizers (-fsanitize=address,fuzzer,undefined).
A file named fuzz_scan.c is used to call print_ies() with random input.
Some beacon frame TLVs were used as seed corpus for libfuzzer. I can also
post my small patches doing this, but the current integration into the Makefile can
be called "ad-hoc" at best :).

Markus

fuzz_scan.c:

#include "iw.h"

int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
print_ies((unsigned char *)data, size, size % 2, PRINT_SCAN);
return 0;
}


2020-02-11 10:13:37

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 0/8] iw: parse measurement pilot and fix scan bugs

On Mon, 2020-02-10 at 09:22 +0100, Markus Theil wrote:
> I can also
> post my small patches doing this, but the current integration into the Makefile can
> be called "ad-hoc" at best :).

Please do, I can always fix up things but then I have some incentive ;-)

johannes