public inbox for iwd@lists.linux.dev
 help / color / mirror / Atom feed
* [PATCH 1/4] p2p: check connected peer before processing request
@ 2024-03-01 19:40 James Prestwood
  2024-03-01 19:40 ` [PATCH 2/4] eap-mschapv2: Check Password-Hash exists when loading settings James Prestwood
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: James Prestwood @ 2024-03-01 19:40 UTC (permalink / raw)
  To: iwd; +Cc: James Prestwood

Caught by static analysis, the dev->conn_peer pointer was being
dereferenced very early on without a NULL check, but further it
was being NULL checked. If there is a possibility of it being NULL
the check should be done much earlier.
---
 src/p2p.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/p2p.c b/src/p2p.c
index 08fe8444..205d4fcb 100644
--- a/src/p2p.c
+++ b/src/p2p.c
@@ -2310,6 +2310,9 @@ static void p2p_device_go_negotiation_req_cb(const struct mmpdu_header *mpdu,
 	if (!peer)
 		return;
 
+	if (!dev->conn_peer)
+		return;
+
 	if (body_len < 8)
 		return;
 
@@ -2438,7 +2441,7 @@ static void p2p_device_go_negotiation_req_cb(const struct mmpdu_header *mpdu,
 	memcpy(dev->conn_peer_interface_addr, req_info.intended_interface_addr,
 		6);
 
-	if (dev->is_go && dev->conn_peer) {
+	if (dev->is_go) {
 		p2p_set_group_id(dev);
 
 		dev->conn_config_delay =
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/4] eap-mschapv2: Check Password-Hash exists when loading settings
  2024-03-01 19:40 [PATCH 1/4] p2p: check connected peer before processing request James Prestwood
@ 2024-03-01 19:40 ` James Prestwood
  2024-03-01 19:40 ` [PATCH 3/4] monitor: properly mask HE capabilities bitfield James Prestwood
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: James Prestwood @ 2024-03-01 19:40 UTC (permalink / raw)
  To: iwd; +Cc: James Prestwood

Caught by static analysis, the Password-Hash was never validated so
it could end up memcpying from a NULL pointer.
---
 src/eap-mschapv2.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/eap-mschapv2.c b/src/eap-mschapv2.c
index ef0ce620..7f71cc82 100644
--- a/src/eap-mschapv2.c
+++ b/src/eap-mschapv2.c
@@ -532,6 +532,9 @@ static bool eap_mschapv2_load_settings(struct eap_state *eap,
 		snprintf(setting, sizeof(setting), "%sPassword-Hash", prefix);
 		hash = l_settings_get_bytes(settings, "Security", setting,
 						&hash_len);
+		if (!hash)
+			goto error;
+
 		memcpy(state->password_hash, hash, 16);
 		explicit_bzero(hash, 16);
 		l_free(hash);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 3/4] monitor: properly mask HE capabilities bitfield
  2024-03-01 19:40 [PATCH 1/4] p2p: check connected peer before processing request James Prestwood
  2024-03-01 19:40 ` [PATCH 2/4] eap-mschapv2: Check Password-Hash exists when loading settings James Prestwood
@ 2024-03-01 19:40 ` James Prestwood
  2024-03-01 19:40 ` [PATCH 4/4] client: fix two issues caught by static analysis James Prestwood
  2024-03-12  3:10 ` [PATCH 1/4] p2p: check connected peer before processing request Denis Kenzior
  3 siblings, 0 replies; 5+ messages in thread
From: James Prestwood @ 2024-03-01 19:40 UTC (permalink / raw)
  To: iwd; +Cc: James Prestwood

Caught by static analysis, the bitfield was incorrect and was masking
8 entries (0xff), not 5 (0x1f).
---
 monitor/nlmon.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/monitor/nlmon.c b/monitor/nlmon.c
index bb8cd496..6fe63b8d 100644
--- a/monitor/nlmon.c
+++ b/monitor/nlmon.c
@@ -1689,7 +1689,7 @@ static void print_ie_he_capabilities(unsigned int level,
 {
 	const uint8_t *ptr = data;
 	uint8_t width_set = bit_field((ptr + 6)[0], 1, 7);
-	uint8_t mask = 0xff;
+	uint8_t mask = 0x1f;
 
 	const char *he_channel_width_bitfield[] = {
 		[0] = "40MHz supported (2.4GHz)",
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 4/4] client: fix two issues caught by static analysis
  2024-03-01 19:40 [PATCH 1/4] p2p: check connected peer before processing request James Prestwood
  2024-03-01 19:40 ` [PATCH 2/4] eap-mschapv2: Check Password-Hash exists when loading settings James Prestwood
  2024-03-01 19:40 ` [PATCH 3/4] monitor: properly mask HE capabilities bitfield James Prestwood
@ 2024-03-01 19:40 ` James Prestwood
  2024-03-12  3:10 ` [PATCH 1/4] p2p: check connected peer before processing request Denis Kenzior
  3 siblings, 0 replies; 5+ messages in thread
From: James Prestwood @ 2024-03-01 19:40 UTC (permalink / raw)
  To: iwd; +Cc: James Prestwood

 - va_end was not being called on an error condition
 - An uninitialized struct was being accessed if ioctl failed
---
 client/display.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/client/display.c b/client/display.c
index 64125934..83214c86 100644
--- a/client/display.c
+++ b/client/display.c
@@ -230,7 +230,8 @@ static void display_refresh_check_feasibility(void)
 {
 	const struct winsize ws;
 
-	ioctl(STDOUT_FILENO, TIOCGWINSZ, &ws);
+	if (ioctl(STDOUT_FILENO, TIOCGWINSZ, &ws) < 0)
+		return;
 
 	if (ws.ws_col < LINE_LEN - 1) {
 		if (display_refresh.enabled) {
@@ -564,8 +565,6 @@ void display_table_row(const char *margin, unsigned int ncolumns, ...)
 		str += entry_append(e, str);
 	}
 
-	va_end(va);
-
 	display("%s\n", buf);
 	str = buf;
 
@@ -591,6 +590,8 @@ void display_table_row(const char *margin, unsigned int ncolumns, ...)
 	}
 
 done:
+	va_end(va);
+
 	for (i = 0; i < ncolumns; i++) {
 		if (entries[i].color)
 			l_free(entries[i].color);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/4] p2p: check connected peer before processing request
  2024-03-01 19:40 [PATCH 1/4] p2p: check connected peer before processing request James Prestwood
                   ` (2 preceding siblings ...)
  2024-03-01 19:40 ` [PATCH 4/4] client: fix two issues caught by static analysis James Prestwood
@ 2024-03-12  3:10 ` Denis Kenzior
  3 siblings, 0 replies; 5+ messages in thread
From: Denis Kenzior @ 2024-03-12  3:10 UTC (permalink / raw)
  To: James Prestwood, iwd

Hi James,

On 3/1/24 13:40, James Prestwood wrote:
> Caught by static analysis, the dev->conn_peer pointer was being
> dereferenced very early on without a NULL check, but further it
> was being NULL checked. If there is a possibility of it being NULL
> the check should be done much earlier.
> ---
>   src/p2p.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)

All applied, thanks.

Regards,
-Denis


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2024-03-12  3:12 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-01 19:40 [PATCH 1/4] p2p: check connected peer before processing request James Prestwood
2024-03-01 19:40 ` [PATCH 2/4] eap-mschapv2: Check Password-Hash exists when loading settings James Prestwood
2024-03-01 19:40 ` [PATCH 3/4] monitor: properly mask HE capabilities bitfield James Prestwood
2024-03-01 19:40 ` [PATCH 4/4] client: fix two issues caught by static analysis James Prestwood
2024-03-12  3:10 ` [PATCH 1/4] p2p: check connected peer before processing request Denis Kenzior

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox