[PATCH 1/2] eap-aka: round to nearest word on message buffers

[PATCH 1/2] eap-aka: round to nearest word on message buffers

[James Prestwood]

[-- Attachment #1: Type: text/plain, Size: 1513 bytes --]

One of these message buffers was overflowing due to padding not
being taken into account (caught by sanitizers). Wrapped the length
of all message buffers with EAP_SIM_ROUND as to account for any
padding that attributes may add.
---
 src/eap-aka.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/eap-aka.c b/src/eap-aka.c
index 3ac0b662..9b693875 100644
--- a/src/eap-aka.c
+++ b/src/eap-aka.c
@@ -189,7 +189,7 @@ static void check_milenage_cb(const uint8_t *res, const uint8_t *ck,
 	struct eap_aka_handle *aka = eap_get_data(eap);
 
 	size_t resp_len = aka->protected ? 44 : 40;
-	uint8_t response[resp_len + 4];
+	uint8_t response[EAP_SIM_ROUND(resp_len + 4)];
 	uint8_t *pos = response;
 
 	if (auts) {
@@ -512,7 +512,7 @@ static void handle_notification(struct eap_state *eap, const uint8_t *pkt,
 	if (value == EAP_SIM_SUCCESS && aka->protected &&
 			aka->state == EAP_AKA_STATE_CHALLENGE) {
 		/* header + MAC + MAC header */
-		uint8_t response[8 + EAP_SIM_MAC_LEN + 4];
+		uint8_t response[EAP_SIM_ROUND(8 + EAP_SIM_MAC_LEN + 4)];
 		uint8_t *pos = response;
 
 		/*
@@ -564,7 +564,7 @@ static void handle_identity(struct eap_state *eap, const uint8_t *pkt,
 		size_t len)
 {
 	struct eap_aka_handle *aka = eap_get_data(eap);
-	uint8_t response[8 + strlen(aka->identity) + 4];
+	uint8_t response[EAP_SIM_ROUND(8 + strlen(aka->identity) + 4)];
 	uint8_t *pos = response;
 
 	if (aka->state != EAP_AKA_STATE_UNCONNECTED) {
-- 
2.31.1

[PATCH 2/2] p2p: fix out of scope read

[James Prestwood]

[-- Attachment #1: Type: text/plain, Size: 2757 bytes --]

The authorized macs pointer was being set to either the wsc_beacon
or wsc_probe_response structures, which were initialized out of
scope to where 'amacs' was being used. This resulted in an out of
scope read, caught by address sanitizers.
---
 src/p2p.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/src/p2p.c b/src/p2p.c
index abdb69d2..4c059173 100644
--- a/src/p2p.c
+++ b/src/p2p.c
@@ -1817,6 +1817,8 @@ static bool p2p_provision_scan_notify(int err, struct l_queue *bss_list,
 		struct p2p_capability_attr *capability;
 		enum wsc_device_password_id device_password_id;
 		const uint8_t *amacs;
+		struct wsc_probe_response wsc_probe_info;
+		struct wsc_beacon wsc_beacon_info;
 
 		/*
 		 * Check if we found our target GO, some of these checks may
@@ -1844,15 +1846,13 @@ static bool p2p_provision_scan_notify(int err, struct l_queue *bss_list,
 		}
 
 		if (bss->source_frame == SCAN_BSS_PROBE_RESP) {
-			struct wsc_probe_response wsc_info;
-
 			if (!bss->p2p_probe_resp_info) {
 				l_error("SSID matched but no valid P2P IE");
 				continue;
 			}
 
 			if (wsc_parse_probe_response(bss->wsc, bss->wsc_size,
-							&wsc_info) < 0) {
+							&wsc_probe_info) < 0) {
 				l_error("SSID matched but can't parse WSC "
 					"Probe Response info");
 				continue;
@@ -1860,30 +1860,28 @@ static bool p2p_provision_scan_notify(int err, struct l_queue *bss_list,
 
 			group_id = bss->p2p_probe_resp_info->
 				device_info.device_addr;
-			selected_reg = wsc_info.selected_registrar;
+			selected_reg = wsc_probe_info.selected_registrar;
 			capability = &bss->p2p_probe_resp_info->capability;
-			device_password_id = wsc_info.device_password_id;
-			amacs = wsc_info.authorized_macs;
+			device_password_id = wsc_probe_info.device_password_id;
+			amacs = wsc_probe_info.authorized_macs;
 		} else if (bss->source_frame == SCAN_BSS_BEACON) {
-			struct wsc_beacon wsc_info;
-
 			if (!bss->p2p_beacon_info) {
 				l_error("SSID matched but no valid P2P IE");
 				continue;
 			}
 
 			if (wsc_parse_beacon(bss->wsc, bss->wsc_size,
-						&wsc_info) < 0) {
+						&wsc_beacon_info) < 0) {
 				l_error("SSID matched but can't parse WSC "
 					"Beacon info");
 				continue;
 			}
 
 			group_id = bss->p2p_beacon_info->device_addr;
-			selected_reg = wsc_info.selected_registrar;
+			selected_reg = wsc_beacon_info.selected_registrar;
 			capability = &bss->p2p_beacon_info->capability;
-			device_password_id = wsc_info.device_password_id;
-			amacs = wsc_info.authorized_macs;
+			device_password_id = wsc_beacon_info.device_password_id;
+			amacs = wsc_beacon_info.authorized_macs;
 		} else
 			continue;
 
-- 
2.31.1

Re: [PATCH 1/2] eap-aka: round to nearest word on message buffers

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 429 bytes --]

Hi James,

On 7/30/21 10:07 AM, James Prestwood wrote:
> One of these message buffers was overflowing due to padding not
> being taken into account (caught by sanitizers). Wrapped the length
> of all message buffers with EAP_SIM_ROUND as to account for any
> padding that attributes may add.
> ---
>   src/eap-aka.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)

Applied, thanks.

Regards,
-Denis

Re: [PATCH 2/2] p2p: fix out of scope read

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 458 bytes --]

Hi James,

On 7/30/21 10:07 AM, James Prestwood wrote:
> The authorized macs pointer was being set to either the wsc_beacon
> or wsc_probe_response structures, which were initialized out of
> scope to where 'amacs' was being used. This resulted in an out of
> scope read, caught by address sanitizers.
> ---
>   src/p2p.c | 22 ++++++++++------------
>   1 file changed, 10 insertions(+), 12 deletions(-)
> 

Applied, thanks.

Regards,
-Denis