[PATCH 1/2] knownnetworks: fix potential out of bounds write

[PATCH 1/2] knownnetworks: fix potential out of bounds write

[Jiajie Chen]

If a very long ssid was used (e.g. CJK characters in SSID), it might do
out of bounds write to static variable for lack of checking the position
before the last snprintf() call.
---
 src/knownnetworks.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/knownnetworks.c b/src/knownnetworks.c
index 487b7017..6c575e50 100644
--- a/src/knownnetworks.c
+++ b/src/knownnetworks.c
@@ -176,7 +176,8 @@ static const char *known_network_get_path(const struct network_info *network)
 		pos += snprintf(path + pos, sizeof(path) - pos, "%02x",
 				network->ssid[i]);
 
-	snprintf(path + pos, sizeof(path) - pos, "_%s",
+	if (pos < sizeof(path))
+		snprintf(path + pos, sizeof(path) - pos, "_%s",
 			security_to_str(network->type));
 	path[sizeof(path) - 1] = '\0';
 
-- 
2.30.2

[PATCH 2/2] knownnetworks: fix printing SSID in hex

[Jiajie Chen]

Force conversion to unsigned char before printing to avoid sign
extension when printing SSID in hex. For example, if there are CJK
characters in SSID, it will generate a very long string like
/net/connman/iwd/ffffffe8ffffffaeffffffa1.
---
 src/knownnetworks.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/knownnetworks.c b/src/knownnetworks.c
index 6c575e50..d4d50a6f 100644
--- a/src/knownnetworks.c
+++ b/src/knownnetworks.c
@@ -174,7 +174,7 @@ static const char *known_network_get_path(const struct network_info *network)
 
 	for (i = 0; network->ssid[i] && pos < sizeof(path); i++)
 		pos += snprintf(path + pos, sizeof(path) - pos, "%02x",
-				network->ssid[i]);
+				(unsigned char)network->ssid[i]);
 
 	if (pos < sizeof(path))
 		snprintf(path + pos, sizeof(path) - pos, "_%s",
-- 
2.30.2

Re: [PATCH 1/2] knownnetworks: fix potential out of bounds write

[Denis Kenzior]

Hi Jiajie,

On 2/26/23 00:25, Jiajie Chen wrote:
> If a very long ssid was used (e.g. CJK characters in SSID), it might do
> out of bounds write to static variable for lack of checking the position
> before the last snprintf() call.
> ---
>   src/knownnetworks.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 

Nice catch.  Applied, thanks.

Regards,
-Denis