[PATCH 1/9] monitor: fix spelling Exausted -> Exhausted

[PATCH 1/9] monitor: fix spelling Exausted -> Exhausted

[James Prestwood]

Caught by codespell
---
 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

[PATCH 2/9] scan: fix out of bound array access for survey results

[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.
---
 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

Re: [PATCH 2/9] scan: fix out of bound array access for survey results

[Denis Kenzior]

Hi James,

On 4/16/25 7:34 AM, James Prestwood wrote:
> 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.
> ---
>   src/scan.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 

Can I get some Fixes: tags for these?

Regards,
-Denis

Re: [PATCH 2/9] scan: fix out of bound array access for survey results

[James Prestwood]

On 4/16/25 10:20 AM, Denis Kenzior wrote:
> Hi James,
>
> On 4/16/25 7:34 AM, James Prestwood wrote:
>> 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.
>> ---
>>   src/scan.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>
> Can I get some Fixes: tags for these?
Sure, I'll see what I can gather up.
>
> Regards,
> -Denis

[PATCH 3/9] dpp-util: fail on duplicate values in URI

[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.
---
 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

[PATCH 4/9] unit: add test for duplicate URI elements

[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

[PATCH 5/9] monitor: add size check for interworking IE parsing

[James Prestwood]

---
 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

[PATCH 6/9] storage: add length check in __storage_decrypt

[James Prestwood]

The length of EncryptedSecurity was assumed to be at least 16 bytes
and anything less would underflow the length to l_malloc.
---
 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

[PATCH 7/9] unit: add test-storage

[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

[PATCH 8/9] station: check return when advancing iterator

[James Prestwood]

---
 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

[PATCH 9/9] eap-mschapv2: Fix leak of state->user on error path

[James Prestwood]

---
 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