2020-02-10 09:46:58

by Markus Theil

Subject: [PATCH 1/2] iw: scan: fix endless loop in print_measurement_pilot_tx

---
scan.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/scan.c b/scan.c
index 98c5c10..a5beb0e 100644
--- a/scan.c
+++ b/scan.c
@@ -1548,6 +1548,7 @@ static void print_measurement_pilot_tx(const uint8_t type, uint8_t len,
++p;
uint8_t len = *p;
++p;
+ const uint8_t *end = p + len;

len_remaining -= 2;

@@ -1557,18 +1558,21 @@ static void print_measurement_pilot_tx(const uint8_t type, uint8_t len,
return;
}

- printf("\t\t * vendor specific: OUI %.2x:%.2x:%.2x, data:",
- p[0], p[1], p[2]);
- len_remaining -= 3;
-
- if (len > len_remaining) {
+ if (len < 3 || len > len_remaining) {
printf(" <Parse error, element too short>\n");
return;
}

- while (p < p + len)
+ printf("\t\t * vendor specific: OUI %.2x:%.2x:%.2x, data:",
+ p[0], p[1], p[2]);
+ /* add only two here and use ++p in while loop */
+ p += 2;
+
+ while (++p < end)
printf(" %.2x", *p);
printf("\n");
+
+ len_remaining -= len;
}
}

--
2.25.0

2020-02-10 09:46:58

by Markus Theil

Subject: [PATCH 2/2] iw: scan: better length checks in print_wifi_wps()

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

diff --git a/scan.c b/scan.c
index a5beb0e..dbfe44c 100644
--- a/scan.c
+++ b/scan.c
@@ -1829,6 +1829,11 @@ static void print_wifi_wps(const uint8_t type, uint8_t len, const uint8_t *data,
switch (subtype) {
case 0x104a:
tab_on_first(&first);
+ if (sublen < 1) {
+ printf("\t * Version: (invalid "
+ "length %d)\n", sublen);
+ break;
+ }
printf("\t * Version: %d.%d\n", data[4] >> 4, data[4] & 0xF);
break;
case 0x1011:
@@ -1861,6 +1866,11 @@ static void print_wifi_wps(const uint8_t type, uint8_t len, const uint8_t *data,
printf("\t * Model Number: %.*s\n", sublen, data + 4);
break;
case 0x103b: {
+ if (sublen < 1) {
+ printf("\t * Response Type: (invalid "
+ "length %d)\n", sublen);
+ break;
+ }
__u8 val = data[4];
tab_on_first(&first);
printf("\t * Response Type: %d%s\n",
@@ -1874,6 +1884,11 @@ static void print_wifi_wps(const uint8_t type, uint8_t len, const uint8_t *data,
break;
}
case 0x1041: {
+ if (sublen < 1) {
+ printf("\t * Selected Registrar: (invalid "
+ "length %d)\n", sublen);
+ break;
+ }
__u8 val = data[4];
tab_on_first(&first);
printf("\t * Selected Registrar: 0x%x\n", val);
@@ -1884,6 +1899,11 @@ static void print_wifi_wps(const uint8_t type, uint8_t len, const uint8_t *data,
printf("\t * Serial Number: %.*s\n", sublen, data + 4);
break;
case 0x1044: {
+ if (sublen < 1) {
+ printf("\t * Wi-Fi Protected Setup State: (invalid "
+ "length %d)\n", sublen);
+ break;
+ }
__u8 val = data[4];
tab_on_first(&first);
printf("\t * Wi-Fi Protected Setup State: %d%s%s\n",
@@ -1928,6 +1948,11 @@ static void print_wifi_wps(const uint8_t type, uint8_t len, const uint8_t *data,
}
case 0x1008:
case 0x1053: {
+ if (sublen < 2) {
+ printf("\t * Config methods: (invalid "
+ "length %d)\n", sublen);
+ break;
+ }
__u16 meth = (data[4] << 8) + data[5];
bool comma = false;
tab_on_first(&first);
--
2.25.0

2020-02-10 09:48:30

by Markus Theil

Subject: Re: [PATCH 1/2] iw: scan: fix endless loop in print_measurement_pilot_tx

On 2/10/20 10:46 AM, Markus Theil wrote:
I forgot my signed-off-by, feel free to add.
> ---
> scan.c | 16 ++++++++++------
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/scan.c b/scan.c
> index 98c5c10..a5beb0e 100644
> --- a/scan.c
> +++ b/scan.c
> @@ -1548,6 +1548,7 @@ static void print_measurement_pilot_tx(const uint8_t type, uint8_t len,
> ++p;
> uint8_t len = *p;
> ++p;
> + const uint8_t *end = p + len;
>
> len_remaining -= 2;
>
> @@ -1557,18 +1558,21 @@ static void print_measurement_pilot_tx(const uint8_t type, uint8_t len,
> return;
> }
>
> - printf("\t\t * vendor specific: OUI %.2x:%.2x:%.2x, data:",
> - p[0], p[1], p[2]);
> - len_remaining -= 3;
> -
> - if (len > len_remaining) {
> + if (len < 3 || len > len_remaining) {
> printf(" <Parse error, element too short>\n");
> return;
> }
>
> - while (p < p + len)
> + printf("\t\t * vendor specific: OUI %.2x:%.2x:%.2x, data:",
> + p[0], p[1], p[2]);
> + /* add only two here and use ++p in while loop */
> + p += 2;
> +
> + while (++p < end)
> printf(" %.2x", *p);
> printf("\n");
> +
> + len_remaining -= len;
> }
> }
>