[PATCH 01/10] anqp: fix potential NULL pointer dereference

[PATCH 01/10] anqp: fix potential NULL pointer dereference

[James Prestwood]

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

---
 src/anqp.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/anqp.c b/src/anqp.c
index a47530ab..5e867656 100644
--- a/src/anqp.c
+++ b/src/anqp.c
@@ -287,6 +287,9 @@ uint32_t anqp_request(uint32_t ifindex, const uint8_t *addr,
 	uint32_t duration = 300;
 	struct netdev *netdev = netdev_find(ifindex);
 
+	if (!netdev)
+		return 0;
+
 	/*
 	 * TODO: Netdev dependencies will eventually be removed so we need
 	 * another way to figure out wiphy capabilities.
-- 
2.17.1

[PATCH 02/10] erp: check return of hkdf_expand

[James Prestwood]

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

---
 src/erp.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/erp.c b/src/erp.c
index 22938c32..2884eb83 100644
--- a/src/erp.c
+++ b/src/erp.c
@@ -509,8 +509,9 @@ int erp_rx_packet(struct erp_state *erp, const uint8_t *pkt, size_t len)
 	l_put_be16(64, ptr);
 	ptr += 2;
 
-	hkdf_expand(L_CHECKSUM_SHA256, erp->r_rk, erp->cache->emsk_len,
-			info, ptr - info, erp->rmsk, erp->cache->emsk_len);
+	if (!hkdf_expand(L_CHECKSUM_SHA256, erp->r_rk, erp->cache->emsk_len,
+			info, ptr - info, erp->rmsk, erp->cache->emsk_len))
+		goto eap_failed;
 
 	return 0;
 
-- 
2.17.1

[PATCH 03/10] eap-pwd: fix potential memory leak

[James Prestwood]

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

---
 src/eap-pwd.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/eap-pwd.c b/src/eap-pwd.c
index e70fc37c..3aa16222 100644
--- a/src/eap-pwd.c
+++ b/src/eap-pwd.c
@@ -115,8 +115,10 @@ static bool kdf(uint8_t *key, size_t key_len, const char *label,
 		iov[iov_pos].iov_base = &L;
 		iov[iov_pos++].iov_len = 2;
 
-		if (!l_checksum_updatev(hmac, iov, iov_pos))
+		if (!l_checksum_updatev(hmac, iov, iov_pos)) {
+			l_checksum_free(hmac);
 			return false;
+		}
 
 		l_checksum_get_digest(hmac, out + len, minsize(olen - len, 32));
 		l_checksum_free(hmac);
-- 
2.17.1

[PATCH 04/10] crypto: fix potential memory leak

[James Prestwood]

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

---
 src/crypto.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/crypto.c b/src/crypto.c
index 62edd447..638098a9 100644
--- a/src/crypto.c
+++ b/src/crypto.c
@@ -706,6 +706,7 @@ bool hkdf_extract(enum l_checksum_type type, const uint8_t *key,
 	}
 
 	if (!l_checksum_updatev(hmac, iov, num_args)) {
+		l_checksum_free(hmac);
 		va_end(va);
 		return false;
 	}
-- 
2.17.1

[PATCH 05/10] hotspot: fix multiple potential memory leaks

[James Prestwood]

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

---
 src/hotspot.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/hotspot.c b/src/hotspot.c
index 20ebb2fd..1a783958 100644
--- a/src/hotspot.c
+++ b/src/hotspot.c
@@ -342,7 +342,11 @@ static struct hs20_config *hs20_config_new(struct l_settings *settings,
 	config = l_new(struct hs20_config, 1);
 
 	if (hessid_str) {
-		util_string_to_address(hessid_str, config->hessid);
+		if (!util_string_to_address(hessid_str, config->hessid)) {
+			l_error("Invalid HESSID in settings");
+			memset(config->hessid, 0, 6);
+		}
+
 		l_free(hessid_str);
 	}
 
@@ -423,6 +427,8 @@ static void hs20_dir_watch_cb(const char *filename,
 
 		l_queue_push_head(hs20_settings, config);
 
+		l_settings_free(new);
+
 		break;
 	case L_DIR_WATCH_EVENT_REMOVED:
 		config = l_queue_remove_if(hs20_settings, match_filename,
@@ -453,6 +459,8 @@ static void hs20_dir_watch_cb(const char *filename,
 
 		known_network_update(&config->super, new, connected_time);
 
+		l_settings_free(new);
+
 		break;
 	case L_DIR_WATCH_EVENT_ACCESSED:
 		break;
-- 
2.17.1

[PATCH 06/10] ie: fix uninitialized rx/tx_nss values

[James Prestwood]

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

---
 src/ie.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/ie.c b/src/ie.c
index e90b5cfa..4bc56589 100644
--- a/src/ie.c
+++ b/src/ie.c
@@ -1912,9 +1912,9 @@ static int ie_parse_vht_capability(struct ie_tlv_iter *vht_iter,
 	uint8_t rx_mcs_map[2];
 	uint8_t tx_mcs_map[2];
 	unsigned int max_rx_mcs = 0;
-	unsigned int rx_nss;
+	unsigned int rx_nss = 1;
 	unsigned int max_tx_mcs = 0;
-	unsigned int tx_nss;
+	unsigned int tx_nss = 1;
 	uint8_t ht_cap;
 	bool short_gi_20mhz;
 	bool short_gi_40mhz;
-- 
2.17.1

[PATCH 07/10] owe: check for error return getting shared_secret

[James Prestwood]

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

---
 src/owe.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/owe.c b/src/owe.c
index daaf1f30..9af3a0de 100644
--- a/src/owe.c
+++ b/src/owe.c
@@ -165,9 +165,11 @@ static bool owe_compute_keys(struct owe_sm *owe, const void *public_key,
 	l_ecc_point_free(other_public);
 
 	nbytes = l_ecc_scalar_get_data(shared_secret, ss_buf, sizeof(ss_buf));
-
 	l_ecc_scalar_free(shared_secret);
 
+	if (nbytes < 0)
+		return false;
+
 	ptr += l_ecc_point_get_x(owe->public_key, ptr, sizeof(key));
 	memcpy(ptr, public_key, nbytes);
 	ptr += nbytes;
-- 
2.17.1

[PATCH 08/10] owe: fix potential double free on error

[James Prestwood]

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

---
 src/owe.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/owe.c b/src/owe.c
index 9af3a0de..9fff986f 100644
--- a/src/owe.c
+++ b/src/owe.c
@@ -217,7 +217,6 @@ static bool owe_compute_keys(struct owe_sm *owe, const void *public_key,
 
 failed:
 	memset(ss_buf, 0, sizeof(ss_buf));
-	l_ecc_scalar_free(shared_secret);
 	return false;
 }
 
-- 
2.17.1

[PATCH 09/10] owe: fix potential uninitialized variable

[James Prestwood]

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

---
 src/owe.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/owe.c b/src/owe.c
index 9fff986f..afdd32ea 100644
--- a/src/owe.c
+++ b/src/owe.c
@@ -246,7 +246,7 @@ static int owe_rx_associate(struct auth_proto *ap, const uint8_t *frame,
 	size_t owe_dh_len = 0;
 	const uint8_t *owe_dh = NULL;
 	struct ie_rsn_info info;
-	bool akm_found;
+	bool akm_found = false;
 	const void *data;
 
 	mpdu = mpdu_validate(frame, len);
-- 
2.17.1

[PATCH 10/10] sae: check return getting k_point

[James Prestwood]

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

---
 src/sae.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/sae.c b/src/sae.c
index 8f9425f1..232a63b9 100644
--- a/src/sae.c
+++ b/src/sae.c
@@ -498,6 +498,9 @@ static int sae_process_commit(struct sae_sm *sm, const uint8_t *from,
 
 	l_ecc_point_free(k_point);
 
+	if (klen < 0)
+		goto reject;
+
 	/* keyseed = H(<0>32, k) */
 	hmac_sha256(zero_key, 32, k, klen, keyseed, 32);
 
-- 
2.17.1

Re: [PATCH 01/10] anqp: fix potential NULL pointer dereference

[Denis Kenzior]

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

Hi James,

On 10/16/19 5:29 PM, James Prestwood wrote:
> ---
>   src/anqp.c | 3 +++
>   1 file changed, 3 insertions(+)
> 

All except patch 5 applied now.

Regards,
-Denis

Re: [PATCH 05/10] hotspot: fix multiple potential memory leaks

[Denis Kenzior]

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

Hi James,

On 10/16/19 5:29 PM, James Prestwood wrote:
> ---
>   src/hotspot.c | 10 +++++++++-
>   1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/src/hotspot.c b/src/hotspot.c
> index 20ebb2fd..1a783958 100644
> --- a/src/hotspot.c
> +++ b/src/hotspot.c
> @@ -342,7 +342,11 @@ static struct hs20_config *hs20_config_new(struct l_settings *settings,
>   	config = l_new(struct hs20_config, 1);
>   
>   	if (hessid_str) {
> -		util_string_to_address(hessid_str, config->hessid);
> +		if (!util_string_to_address(hessid_str, config->hessid)) {
> +			l_error("Invalid HESSID in settings");
> +			memset(config->hessid, 0, 6);

Why the memset?  Also, shouldn't we fail here?  Or at the very least 
delay checking the presence of hessid || nai_realms || rc_str until 
after the parsing has been validated?

> +		}
> +
>   		l_free(hessid_str);
>   	}
>   

Also this seems to be unrelated to the patch description?

Regards,
-Denis