iwd.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/9] monitor: fix spelling Exausted -> Exhausted
@ 2025-04-16 17:33 James Prestwood
  2025-04-16 17:33 ` [PATCH v2 2/9] scan: fix out of bound array access for survey results James Prestwood
                   ` (8 more replies)
  0 siblings, 9 replies; 10+ messages in thread
From: James Prestwood @ 2025-04-16 17:33 UTC (permalink / raw)
  To: iwd; +Cc: James Prestwood

Caught by codespell

Fixes: 83a24575 ("monitor: add support for limiting PCAP size/count")
---
 monitor/nlmon.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/monitor/nlmon.c b/monitor/nlmon.c
index 76eb2db6..14a29eab 100644
--- a/monitor/nlmon.c
+++ b/monitor/nlmon.c
@@ -7433,7 +7433,7 @@ static bool check_pcap(struct nlmon *nlmon, size_t next_size)
 
 	pcap_close(nlmon->pcap);
 
-	/* Exausted the single PCAP file */
+	/* Exhausted the single PCAP file */
 	if (nlmon->max_files < 2) {
 		printf("Reached maximum size of PCAP, exiting\n");
 		nlmon->pcap = NULL;
-- 
2.34.1


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

* [PATCH v2 2/9] scan: fix out of bound array access for survey results
  2025-04-16 17:33 [PATCH v2 1/9] monitor: fix spelling Exausted -> Exhausted James Prestwood
@ 2025-04-16 17:33 ` James Prestwood
  2025-04-16 17:33 ` [PATCH v2 3/9] dpp-util: fail on duplicate values in URI James Prestwood
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: James Prestwood @ 2025-04-16 17:33 UTC (permalink / raw)
  To: iwd; +Cc: James Prestwood

The survey arrays were exactly the number of valid channels for a
given band (e.g. 14 for 2.4GHz) but since channels start at 1 this
means that the last channel for a band would overflow the array.

Fixes: 35808deb ("scan: use GET_SURVEY for SNR calculation in ranking")
---
 src/scan.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/scan.c b/src/scan.c
index aeab6516..dfd667bb 100644
--- a/src/scan.c
+++ b/src/scan.c
@@ -143,9 +143,9 @@ struct scan_survey {
 };
 
 struct scan_survey_results {
-	struct scan_survey survey_2_4[14];
-	struct scan_survey survey_5[196];
-	struct scan_survey survey_6[233];
+	struct scan_survey survey_2_4[15];
+	struct scan_survey survey_5[197];
+	struct scan_survey survey_6[234];
 };
 
 struct scan_results {
-- 
2.34.1


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

* [PATCH v2 3/9] dpp-util: fail on duplicate values in URI
  2025-04-16 17:33 [PATCH v2 1/9] monitor: fix spelling Exausted -> Exhausted James Prestwood
  2025-04-16 17:33 ` [PATCH v2 2/9] scan: fix out of bound array access for survey results James Prestwood
@ 2025-04-16 17:33 ` James Prestwood
  2025-04-16 17:33 ` [PATCH v2 4/9] unit: add test for duplicate URI elements James Prestwood
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: James Prestwood @ 2025-04-16 17:33 UTC (permalink / raw)
  To: iwd; +Cc: James Prestwood

The MAC and version elements weren't super critical but the channel
and bootstrapping key elements would result in memory leaks if there
were duplicates.

This patch now will not allow duplicate elements in the URI.

Fixes: f7f602e1 ("dpp-util: add URI parsing")
---
 src/dpp-util.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/src/dpp-util.c b/src/dpp-util.c
index cfdedbdd..1986a5cc 100644
--- a/src/dpp-util.c
+++ b/src/dpp-util.c
@@ -1166,21 +1166,34 @@ struct dpp_uri_info *dpp_parse_uri(const char *uri)
 
 		switch (*pos) {
 		case 'C':
+			if (L_WARN_ON(info->freqs))
+				goto free_info;
+
 			info->freqs = dpp_parse_class_and_channel(pos + 2, len);
 			if (!info->freqs)
 				goto free_info;
 			break;
 		case 'M':
+			if (L_WARN_ON(!l_memeqzero(info->mac,
+							sizeof(info->mac))))
+				goto free_info;
+
 			ret = dpp_parse_mac(pos + 2, len, info->mac);
 			if (ret < 0)
 				goto free_info;
 			break;
 		case 'V':
+			if (L_WARN_ON(info->version != 0))
+				goto free_info;
+
 			ret = dpp_parse_version(pos + 2, len, &info->version);
 			if (ret < 0)
 				goto free_info;
 			break;
 		case 'K':
+			if (L_WARN_ON(info->boot_public))
+				goto free_info;
+
 			info->boot_public = dpp_parse_key(pos + 2, len);
 			if (!info->boot_public)
 				goto free_info;
-- 
2.34.1


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

* [PATCH v2 4/9] unit: add test for duplicate URI elements
  2025-04-16 17:33 [PATCH v2 1/9] monitor: fix spelling Exausted -> Exhausted James Prestwood
  2025-04-16 17:33 ` [PATCH v2 2/9] scan: fix out of bound array access for survey results James Prestwood
  2025-04-16 17:33 ` [PATCH v2 3/9] dpp-util: fail on duplicate values in URI James Prestwood
@ 2025-04-16 17:33 ` James Prestwood
  2025-04-16 17:33 ` [PATCH v2 5/9] monitor: add size check for interworking IE parsing James Prestwood
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: James Prestwood @ 2025-04-16 17:33 UTC (permalink / raw)
  To: iwd; +Cc: James Prestwood

---
 unit/test-dpp.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/unit/test-dpp.c b/unit/test-dpp.c
index 70e79814..af2f5813 100644
--- a/unit/test-dpp.c
+++ b/unit/test-dpp.c
@@ -116,6 +116,29 @@ struct dpp_test_info bad_channels[] = {
 	},
 };
 
+struct dpp_test_info duplicates[] = {
+	/* Duplicate key */
+	{
+		.uri = "DPP:K:MDkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDIgADURzxmttZoIRIPWGoQMV00XHWCAQIhXruVWOz0NjlkIA=;K:MDkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDIgADURzxmttZoIRIPWGoQMV00XHWCAQIhXruVWOz0NjlkIA=;;",
+		.expect_fail = true
+	},
+	/* Duplicate frequencies*/
+	{
+		.uri = "DPP:C:81/1,115/36;C:81/1,115/36;;",
+		.expect_fail = true
+	},
+	/* Duplicate MACs*/
+	{
+		.uri = "DPP:M:5254005828e5;M:5254005828e5;;",
+		.expect_fail = true
+	},
+	/* Duplicate versions */
+	{
+		.uri = "DPP:V:2;V:2;;",
+		.expect_fail = true
+	},
+};
+
 static bool verify_info(const struct dpp_uri_info *parsed,
 			const struct dpp_test_info *result)
 {
@@ -158,6 +181,14 @@ static void test_bad_channels(const void *data)
 		test_uri_parse(&bad_channels[i]);
 }
 
+static void test_duplicate_in_uri(const void *data)
+{
+	unsigned int i;
+
+	for (i = 0; i < L_ARRAY_SIZE(duplicates); i++)
+		test_uri_parse(&duplicates[i]);
+}
+
 struct dpp_test_vector {
 	/* Initiator values */
 	const char *i_proto_public;
@@ -576,6 +607,7 @@ int main(int argc, char *argv[])
 	l_test_add("DPP URI bad key", test_uri_parse, &bad_key);
 	l_test_add("DPP URI bad channels", test_bad_channels, &bad_channels);
 	l_test_add("DPP URI unexpected ID", test_uri_parse, &unexpected_id);
+	l_test_add("DPP URI duplicates", test_duplicate_in_uri, &duplicates);
 
 	return l_test_run();
 }
-- 
2.34.1


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

* [PATCH v2 5/9] monitor: add size check for interworking IE parsing
  2025-04-16 17:33 [PATCH v2 1/9] monitor: fix spelling Exausted -> Exhausted James Prestwood
                   ` (2 preceding siblings ...)
  2025-04-16 17:33 ` [PATCH v2 4/9] unit: add test for duplicate URI elements James Prestwood
@ 2025-04-16 17:33 ` James Prestwood
  2025-04-16 17:33 ` [PATCH v2 6/9] storage: add length check in __storage_decrypt James Prestwood
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: James Prestwood @ 2025-04-16 17:33 UTC (permalink / raw)
  To: iwd; +Cc: James Prestwood

Fixes: e0c9b684 ("monitor: parse/print HS2.0/WFA IEs")
---
 monitor/nlmon.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/monitor/nlmon.c b/monitor/nlmon.c
index 14a29eab..7924f6f2 100644
--- a/monitor/nlmon.c
+++ b/monitor/nlmon.c
@@ -1915,7 +1915,7 @@ static void print_ie_interworking(unsigned int level,
 	size--;
 	ptr++;
 
-	if (!size)
+	if (size < 2)
 		return;
 
 	/*
-- 
2.34.1


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

* [PATCH v2 6/9] storage: add length check in __storage_decrypt
  2025-04-16 17:33 [PATCH v2 1/9] monitor: fix spelling Exausted -> Exhausted James Prestwood
                   ` (3 preceding siblings ...)
  2025-04-16 17:33 ` [PATCH v2 5/9] monitor: add size check for interworking IE parsing James Prestwood
@ 2025-04-16 17:33 ` James Prestwood
  2025-04-16 17:33 ` [PATCH v2 7/9] unit: add test-storage James Prestwood
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: James Prestwood @ 2025-04-16 17:33 UTC (permalink / raw)
  To: iwd; +Cc: James Prestwood

The length of EncryptedSecurity was assumed to be at least 16 bytes
and anything less would underflow the length to l_malloc.

Fixes: 01cd8587 ("storage: implement network profile encryption")
---
 src/storage.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/src/storage.c b/src/storage.c
index 843581fd..2115a879 100644
--- a/src/storage.c
+++ b/src/storage.c
@@ -500,6 +500,13 @@ int __storage_decrypt(struct l_settings *settings, const char *ssid,
 		return 0;
 	}
 
+	/*
+	 * It should likely be far larger than this, but that will get caught
+	 * later when reloading the decrypted data.
+	 */
+	if (elen < 16)
+		return -EBADMSG;
+
 	/*
 	 * AES-SIV automatically verifies the IV (16 bytes) and returns only
 	 * the decrypted data portion. We add one here for the NULL terminator
-- 
2.34.1


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

* [PATCH v2 7/9] unit: add test-storage
  2025-04-16 17:33 [PATCH v2 1/9] monitor: fix spelling Exausted -> Exhausted James Prestwood
                   ` (4 preceding siblings ...)
  2025-04-16 17:33 ` [PATCH v2 6/9] storage: add length check in __storage_decrypt James Prestwood
@ 2025-04-16 17:33 ` James Prestwood
  2025-04-16 17:33 ` [PATCH v2 8/9] station: check return when advancing iterator James Prestwood
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: James Prestwood @ 2025-04-16 17:33 UTC (permalink / raw)
  To: iwd; +Cc: James Prestwood

For now, a single test for __storage_decrypt that ensures an
invalid length fails as expected.
---
 Makefile.am         |  7 +++++-
 unit/test-storage.c | 55 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 61 insertions(+), 1 deletion(-)
 create mode 100644 unit/test-storage.c

diff --git a/Makefile.am b/Makefile.am
index 6404bdbb..92adfa6e 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -441,7 +441,7 @@ unit_tests += unit/test-cmac-aes \
 		unit/test-arc4 unit/test-wsc unit/test-eap-mschapv2 \
 		unit/test-eap-sim unit/test-sae unit/test-p2p unit/test-band \
 		unit/test-dpp unit/test-json unit/test-nl80211util \
-		unit/test-pmksa
+		unit/test-pmksa unit/test-storage
 endif
 
 if CLIENT
@@ -605,6 +605,11 @@ unit_test_nl80211util_LDADD = $(ell_ldadd)
 unit_test_pmksa_SOURCES = unit/test-pmksa.c src/pmksa.c src/pmksa.h \
 				src/module.h src/util.h
 unit_test_pmksa_LDADD = $(ell_ldadd)
+
+unit_test_storage_SOURCES = unit/test-storage.c src/storage.c src/storage.h \
+				src/crypto.c src/crypto.h \
+				src/common.c src/common.h
+unit_test_storage_LDADD = $(ell_ldadd)
 endif
 
 if CLIENT
diff --git a/unit/test-storage.c b/unit/test-storage.c
new file mode 100644
index 00000000..c40518e6
--- /dev/null
+++ b/unit/test-storage.c
@@ -0,0 +1,55 @@
+/*
+ *
+ *  Wireless daemon for Linux
+ *
+ *  Copyright (C) 2025  Locus Robotics. All rights reserved.
+ *
+ *  This library is free software; you can redistribute it and/or
+ *  modify it under the terms of the GNU Lesser General Public
+ *  License as published by the Free Software Foundation; either
+ *  version 2.1 of the License, or (at your option) any later version.
+ *
+ *  This library is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ *  Lesser General Public License for more details.
+ *
+ *  You should have received a copy of the GNU Lesser General Public
+ *  License along with this library; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <assert.h>
+
+#include <ell/ell.h>
+
+#include "src/storage.h"
+
+static void test_short_encrypted_bytes(const void *data)
+{
+	struct l_settings *settings = l_settings_new();
+	bool changed;
+
+	l_settings_set_string(settings, "Security", "EncryptedSecurity", "012345");
+	l_settings_set_string(settings, "Security", "EncryptedSalt", "012345");
+
+	assert(__storage_decrypt(settings, "mySSID", &changed) < 0);
+	l_settings_free(settings);
+}
+
+int main(int argc, char *argv[])
+{
+	l_test_init(&argc, &argv);
+
+	storage_init((const uint8_t *)"abc123", 6);
+
+	l_test_add("/storage/profile encryption",
+			test_short_encrypted_bytes, NULL);
+
+	return l_test_run();
+}
-- 
2.34.1


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

* [PATCH v2 8/9] station: check return when advancing iterator
  2025-04-16 17:33 [PATCH v2 1/9] monitor: fix spelling Exausted -> Exhausted James Prestwood
                   ` (5 preceding siblings ...)
  2025-04-16 17:33 ` [PATCH v2 7/9] unit: add test-storage James Prestwood
@ 2025-04-16 17:33 ` James Prestwood
  2025-04-16 17:33 ` [PATCH v2 9/9] eap-mschapv2: Fix leak of state->user on error path James Prestwood
  2025-04-16 19:58 ` [PATCH v2 1/9] monitor: fix spelling Exausted -> Exhausted Denis Kenzior
  8 siblings, 0 replies; 10+ messages in thread
From: James Prestwood @ 2025-04-16 17:33 UTC (permalink / raw)
  To: iwd; +Cc: James Prestwood

Fixes: f4ec1ee5 ("station: add Affinities DBus property")
---
 src/station.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/station.c b/src/station.c
index 9972ea76..14c93671 100644
--- a/src/station.c
+++ b/src/station.c
@@ -4836,7 +4836,8 @@ static struct l_dbus_message *station_property_set_affinities(
 		return dbus_error_invalid_args(message);
 
 	/* Get first entry, there should be only one */
-	l_dbus_message_iter_next_entry(&array, &new_path);
+	if (!l_dbus_message_iter_next_entry(&array, &new_path))
+		return dbus_error_invalid_args(message);
 
 	if (l_dbus_message_iter_next_entry(&array, &new_path))
 		return dbus_error_invalid_args(message);
-- 
2.34.1


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

* [PATCH v2 9/9] eap-mschapv2: Fix leak of state->user on error path
  2025-04-16 17:33 [PATCH v2 1/9] monitor: fix spelling Exausted -> Exhausted James Prestwood
                   ` (6 preceding siblings ...)
  2025-04-16 17:33 ` [PATCH v2 8/9] station: check return when advancing iterator James Prestwood
@ 2025-04-16 17:33 ` James Prestwood
  2025-04-16 19:58 ` [PATCH v2 1/9] monitor: fix spelling Exausted -> Exhausted Denis Kenzior
  8 siblings, 0 replies; 10+ messages in thread
From: James Prestwood @ 2025-04-16 17:33 UTC (permalink / raw)
  To: iwd; +Cc: James Prestwood

Fixes: 6dc5d2c3 ("eap-mschapv2: Load credentials obtained from agent")
---
 src/eap-mschapv2.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/eap-mschapv2.c b/src/eap-mschapv2.c
index 1639f578..00976d73 100644
--- a/src/eap-mschapv2.c
+++ b/src/eap-mschapv2.c
@@ -544,7 +544,8 @@ static bool eap_mschapv2_load_settings(struct eap_state *eap,
 	return true;
 
 error:
-	free(state);
+	l_free(state->user);
+	l_free(state);
 	return false;
 }
 
-- 
2.34.1


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

* Re: [PATCH v2 1/9] monitor: fix spelling Exausted -> Exhausted
  2025-04-16 17:33 [PATCH v2 1/9] monitor: fix spelling Exausted -> Exhausted James Prestwood
                   ` (7 preceding siblings ...)
  2025-04-16 17:33 ` [PATCH v2 9/9] eap-mschapv2: Fix leak of state->user on error path James Prestwood
@ 2025-04-16 19:58 ` Denis Kenzior
  8 siblings, 0 replies; 10+ messages in thread
From: Denis Kenzior @ 2025-04-16 19:58 UTC (permalink / raw)
  To: James Prestwood, iwd

Hi James,

On 4/16/25 12:33 PM, James Prestwood wrote:
> Caught by codespell
> 
> Fixes: 83a24575 ("monitor: add support for limiting PCAP size/count")
> ---
>   monitor/nlmon.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

All applied, thanks.

Regards,
-Denis

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

end of thread, other threads:[~2025-04-16 19:58 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-16 17:33 [PATCH v2 1/9] monitor: fix spelling Exausted -> Exhausted James Prestwood
2025-04-16 17:33 ` [PATCH v2 2/9] scan: fix out of bound array access for survey results James Prestwood
2025-04-16 17:33 ` [PATCH v2 3/9] dpp-util: fail on duplicate values in URI James Prestwood
2025-04-16 17:33 ` [PATCH v2 4/9] unit: add test for duplicate URI elements James Prestwood
2025-04-16 17:33 ` [PATCH v2 5/9] monitor: add size check for interworking IE parsing James Prestwood
2025-04-16 17:33 ` [PATCH v2 6/9] storage: add length check in __storage_decrypt James Prestwood
2025-04-16 17:33 ` [PATCH v2 7/9] unit: add test-storage James Prestwood
2025-04-16 17:33 ` [PATCH v2 8/9] station: check return when advancing iterator James Prestwood
2025-04-16 17:33 ` [PATCH v2 9/9] eap-mschapv2: Fix leak of state->user on error path James Prestwood
2025-04-16 19:58 ` [PATCH v2 1/9] monitor: fix spelling Exausted -> Exhausted Denis Kenzior

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).