From bd9314759e177f82b804a0634f2f4788b7f47ba3 Mon Sep 17 00:00:00 2001 From: Antony Rone Oliver Date: Sat, 4 Apr 2026 20:57:36 +0530 Subject: [PATCH 1/3] nm: fix filename overflow for WiFi SSIDs with multi-byte characters SSIDs containing multi-byte UTF-8 characters (e.g. emojis) are percent-encoded by g_uri_escape_string(), expanding each byte to 3 characters. This causes the .nmconnection filename basename to exceed NAME_MAX (255 bytes), crashing NetworkManager with "File name too long". If the candidate basename exceeds NAME_MAX, replace the percent-encoded SSID with a SHA-256 hex digest of the raw SSID bytes. This guarantees a valid-length (91-byte), unique filename regardless of SSID content. Apply the same fix in netplan_get_id_from_nm_filepath() so the reverse lookup falls back to the hashed suffix when the escaped form is absent. Fixes: LP: #2147259 --- src/nm.c | 12 ++++++- src/util.c | 25 +++++++++++--- tests/ctests/test_netplan_misc.c | 59 ++++++++++++++++++++++++++++++++ 3 files changed, 91 insertions(+), 5 deletions(-) diff --git a/src/nm.c b/src/nm.c index 90b807fe5..d2cd49e92 100644 --- a/src/nm.c +++ b/src/nm.c @@ -22,6 +22,7 @@ #include #include #include +#include #include #include @@ -1005,7 +1006,16 @@ write_nm_conf_access_point(const NetplanNetDefinition* def, const char* rootdir, if (ap) { g_autofree char* escaped_ssid = g_uri_escape_string(ap->ssid, NULL, TRUE); /* TODO: make use of netplan_netdef_get_output_filename() */ - conf_path = g_strjoin(NULL, "run/NetworkManager/system-connections/netplan-", escaped_netdef_id, "-", escaped_ssid, ".nmconnection", NULL); + g_autofree char* candidate_basename = g_strjoin(NULL, "netplan-", escaped_netdef_id, "-", escaped_ssid, ".nmconnection", NULL); + const char* ssid_part = escaped_ssid; + g_autofree char* hashed_ssid = NULL; + if (strlen(candidate_basename) > NAME_MAX) { + /* SSID contains multi-byte chars (e.g. emojis) that percent-encode to too many bytes. + * Use SHA-256 of the raw SSID bytes to guarantee a valid-length unique filename. */ + hashed_ssid = g_compute_checksum_for_string(G_CHECKSUM_SHA256, ap->ssid, -1); + ssid_part = hashed_ssid; + } + conf_path = g_strjoin(NULL, "run/NetworkManager/system-connections/netplan-", escaped_netdef_id, "-", ssid_part, ".nmconnection", NULL); g_key_file_set_string(kf, "wifi", "ssid", ap->ssid); if (ap->mode < NETPLAN_WIFI_MODE_OTHER) diff --git a/src/util.c b/src/util.c index a0c490052..0338613f0 100644 --- a/src/util.c +++ b/src/util.c @@ -27,6 +27,7 @@ #include #include #include +#include #include #include @@ -629,7 +630,6 @@ ssize_t netplan_get_id_from_nm_filepath(const char* filename, const char* ssid, char* out_buffer, size_t out_buf_size) { g_autofree gchar* escaped_ssid = NULL; - g_autofree gchar* suffix = NULL; const char* nm_prefix = "/run/NetworkManager/system-connections/netplan-"; const char* pos = g_strrstr(filename, nm_prefix); const char* start = NULL; @@ -641,8 +641,15 @@ netplan_get_id_from_nm_filepath(const char* filename, const char* ssid, char* ou if (ssid) { escaped_ssid = g_uri_escape_string(ssid, NULL, TRUE); - suffix = g_strdup_printf("-%s.nmconnection", escaped_ssid); - end = g_strrstr(filename, suffix); + g_autofree char* escaped_suffix = g_strdup_printf("-%s.nmconnection", escaped_ssid); + end = g_strrstr(filename, escaped_suffix); + + if (!end) { + /* Escaped SSID not found; try SHA-256 hash (used when escaped form exceeded NAME_MAX) */ + g_autofree char* hashed_ssid = g_compute_checksum_for_string(G_CHECKSUM_SHA256, ssid, -1); + g_autofree char* hashed_suffix = g_strdup_printf("-%s.nmconnection", hashed_ssid); + end = g_strrstr(filename, hashed_suffix); + } } else end = g_strrstr(filename, ".nmconnection"); @@ -673,7 +680,17 @@ netplan_netdef_get_output_filename(const NetplanNetDefinition* netdef, const cha if (netdef->backend == NETPLAN_BACKEND_NM) { if (ssid) { g_autofree char* escaped_ssid = g_uri_escape_string(ssid, NULL, TRUE); - conf_path = g_strjoin(NULL, "/run/NetworkManager/system-connections/netplan-", escaped_netdef_id, "-", escaped_ssid, ".nmconnection", NULL); + /* Check if the basename would exceed NAME_MAX (255 bytes) */ + g_autofree char* candidate_basename = g_strjoin(NULL, "netplan-", escaped_netdef_id, "-", escaped_ssid, ".nmconnection", NULL); + const char* ssid_part = escaped_ssid; + g_autofree char* hashed_ssid = NULL; + if (strlen(candidate_basename) > NAME_MAX) { + /* SSID contains multi-byte chars (e.g. emojis) that percent-encode to too many bytes. + * Use SHA-256 of the raw SSID bytes to guarantee a valid-length unique filename. */ + hashed_ssid = g_compute_checksum_for_string(G_CHECKSUM_SHA256, ssid, -1); + ssid_part = hashed_ssid; + } + conf_path = g_strjoin(NULL, "/run/NetworkManager/system-connections/netplan-", escaped_netdef_id, "-", ssid_part, ".nmconnection", NULL); } else { conf_path = g_strjoin(NULL, "/run/NetworkManager/system-connections/netplan-", escaped_netdef_id, ".nmconnection", NULL); } diff --git a/tests/ctests/test_netplan_misc.c b/tests/ctests/test_netplan_misc.c index 06676da39..de0ede139 100644 --- a/tests/ctests/test_netplan_misc.c +++ b/tests/ctests/test_netplan_misc.c @@ -3,8 +3,10 @@ #include #include +#include #include #include +#include #include @@ -185,6 +187,61 @@ test_netplan_netdef_get_output_filename_invalid_backend(__unused void** state) assert_int_equal(ret, 0); } +void +test_netplan_netdef_get_output_filename_nm_with_long_ssid(__unused void** state) +{ + NetplanNetDefinition netdef; + /* 20x U+1F600 (grinning face emoji): 80 raw UTF-8 bytes -> 240 escaped chars. + * basename = "netplan-" (8) + "wlan0" (5) + "-" (1) + 240 + ".nmconnection" (13) = 267 > NAME_MAX (255). + * Expects SHA-256 of raw SSID bytes used in filename instead of escaped form. */ + const char ssid[] = + "\xF0\x9F\x98\x80\xF0\x9F\x98\x80\xF0\x9F\x98\x80\xF0\x9F\x98\x80" + "\xF0\x9F\x98\x80\xF0\x9F\x98\x80\xF0\x9F\x98\x80\xF0\x9F\x98\x80" + "\xF0\x9F\x98\x80\xF0\x9F\x98\x80\xF0\x9F\x98\x80\xF0\x9F\x98\x80" + "\xF0\x9F\x98\x80\xF0\x9F\x98\x80\xF0\x9F\x98\x80\xF0\x9F\x98\x80" + "\xF0\x9F\x98\x80\xF0\x9F\x98\x80\xF0\x9F\x98\x80\xF0\x9F\x98\x80"; + char out_buffer[256] = { 0 }; + + netdef.backend = NETPLAN_BACKEND_NM; + netdef.id = "wlan0"; + + ssize_t ret = netplan_netdef_get_output_filename(&netdef, ssid, out_buffer, sizeof(out_buffer) - 1); + + /* Basename must NOT exceed NAME_MAX */ + const char* basename = strrchr(out_buffer, '/'); + assert_true(basename != NULL); + assert_true(strlen(basename + 1) <= NAME_MAX); + /* Must use the expected prefix */ + assert_true(g_str_has_prefix(out_buffer, + "/run/NetworkManager/system-connections/netplan-wlan0-")); + /* Must end with .nmconnection */ + assert_true(g_str_has_suffix(out_buffer, ".nmconnection")); + /* Returned size must match string length + 1 */ + assert_int_equal(ret, (ssize_t)(strlen(out_buffer) + 1)); +} + +void +test_netplan_get_id_from_nm_filepath_with_hashed_ssid(__unused void** state) +{ + /* Same 20-emoji SSID as test above */ + const char ssid[] = + "\xF0\x9F\x98\x80\xF0\x9F\x98\x80\xF0\x9F\x98\x80\xF0\x9F\x98\x80" + "\xF0\x9F\x98\x80\xF0\x9F\x98\x80\xF0\x9F\x98\x80\xF0\x9F\x98\x80" + "\xF0\x9F\x98\x80\xF0\x9F\x98\x80\xF0\x9F\x98\x80\xF0\x9F\x98\x80" + "\xF0\x9F\x98\x80\xF0\x9F\x98\x80\xF0\x9F\x98\x80\xF0\x9F\x98\x80" + "\xF0\x9F\x98\x80\xF0\x9F\x98\x80\xF0\x9F\x98\x80\xF0\x9F\x98\x80"; + /* Get the hashed filename from the public API */ + NetplanNetDefinition netdef = { .backend = NETPLAN_BACKEND_NM, .id = "wlan0" }; + char hashed_path[256] = { 0 }; + netplan_netdef_get_output_filename(&netdef, ssid, hashed_path, sizeof(hashed_path) - 1); + + char id[16] = { 0 }; + ssize_t bytes_copied = netplan_get_id_from_nm_filepath(hashed_path, ssid, id, sizeof(id)); + + assert_string_equal(id, "wlan0"); + assert_int_equal(bytes_copied, 6); /* strlen("wlan0") + 1 */ +} + void test_netplan_netdef_write_yaml(__unused void** state) { @@ -575,6 +632,8 @@ main() cmocka_unit_test(test_netplan_netdef_get_output_filename_networkd), cmocka_unit_test(test_netplan_netdef_get_output_filename_buffer_is_too_small), cmocka_unit_test(test_netplan_netdef_get_output_filename_invalid_backend), + cmocka_unit_test(test_netplan_netdef_get_output_filename_nm_with_long_ssid), + cmocka_unit_test(test_netplan_get_id_from_nm_filepath_with_hashed_ssid), cmocka_unit_test(test_netplan_netdef_write_yaml), cmocka_unit_test(test_netplan_netdef_write_yaml_90NM), cmocka_unit_test(test_util_is_route_present), From 614b9f0a4016607521bba44012c0648bb601b67c Mon Sep 17 00:00:00 2001 From: Antony Rone Oliver Date: Sun, 12 Apr 2026 17:31:29 +0530 Subject: [PATCH 2/3] tests: add NM WiFi long-SSID coverage test Add test_write_wifi_long_ssid_uses_hash() to test_netplan_nm.c to exercise the NAME_MAX overflow path in write_nm_conf_access_point(). A 20-emoji SSID in NM decimal-byte format (320 chars with semicolons) produces a 507-byte candidate basename when percent-encoded, exceeding NAME_MAX (255). The test verifies that _netplan_netdef_write_nm() falls back to a SHA-256 digest filename and that the basename is within NAME_MAX. Closes the coverage gap introduced by LP: #2147259 fix. --- tests/ctests/test_netplan_nm.c | 73 ++++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/tests/ctests/test_netplan_nm.c b/tests/ctests/test_netplan_nm.c index e2e892bd4..6b8dffbc6 100644 --- a/tests/ctests/test_netplan_nm.c +++ b/tests/ctests/test_netplan_nm.c @@ -2,11 +2,15 @@ #include #include #include +#include +#include +#include #include #include "netplan.h" #include "util-internal.h" +#include "nm.h" #include "test_utils.h" @@ -28,6 +32,74 @@ test_write_empty_state(__unused void** state) netplan_state_clear(&np_state); } +/* A WiFi SSID whose percent-encoded form would make the .nmconnection + * basename exceed NAME_MAX (255) must fall back to a SHA-256 digest. + * + * NM stores non-ASCII SSIDs as semicolon-delimited decimal bytes, e.g. + * the emoji U+1F600 (😀, UTF-8: F0 9F 98 80) becomes "240;159;152;128;". + * g_uri_escape_string() encodes each ';' as '%3B' (3 chars), so 20 such + * emojis → 480-char encoded SSID → 507-byte candidate basename > NAME_MAX. + */ +void +test_write_wifi_long_ssid_uses_hash(__unused void** state) +{ + /* 20× U+1F600 (😀) in NM decimal-byte format */ + const char ssid[] = + "240;159;152;128;240;159;152;128;240;159;152;128;240;159;152;128;" + "240;159;152;128;240;159;152;128;240;159;152;128;240;159;152;128;" + "240;159;152;128;240;159;152;128;240;159;152;128;240;159;152;128;" + "240;159;152;128;240;159;152;128;240;159;152;128;240;159;152;128;" + "240;159;152;128;240;159;152;128;240;159;152;128;240;159;152;128;"; + + g_autofree char* yaml = g_strdup_printf( + "network:\n" + " version: 2\n" + " renderer: NetworkManager\n" + " wifis:\n" + " wlan0:\n" + " dhcp4: true\n" + " access-points:\n" + " \"%s\":\n" + " password: \"s0s3kr1t\"\n", + ssid); + + NetplanState* np_state = load_string_to_netplan_state(yaml); + assert_non_null(np_state); + assert_true(netplan_state_get_netdefs_size(np_state) > 0); + + NetplanNetDefinition* netdef = netplan_state_get_netdef(np_state, "wlan0"); + assert_non_null(netdef); + + char template[] = "/tmp/netplan_nm_test.XXXXXX"; + char* rootdir = mkdtemp(template); + assert_non_null(rootdir); + + gboolean has_been_written = FALSE; + GError* error = NULL; + assert_true(_netplan_netdef_write_nm(np_state, netdef, rootdir, &has_been_written, &error)); + assert_null(error); + assert_true(has_been_written); + + /* The output filename must use the SHA-256 digest of the raw SSID */ + g_autofree char* hash = g_compute_checksum_for_string(G_CHECKSUM_SHA256, ssid, -1); + g_autofree char* expected = g_strdup_printf( + "%s/run/NetworkManager/system-connections/netplan-wlan0-%s.nmconnection", + rootdir, hash); + + assert_true(g_file_test(expected, G_FILE_TEST_EXISTS)); + + /* Basename must be within NAME_MAX */ + const char* basename = strrchr(expected, '/'); + assert_true(strlen(basename + 1) <= NAME_MAX); + + /* Cleanup */ + const gchar *rm_argv[] = { "/bin/rm", "-rf", rootdir, NULL }; + g_spawn_sync(NULL, (gchar**)rm_argv, NULL, G_SPAWN_DEFAULT, + NULL, NULL, NULL, NULL, NULL, NULL); + + netplan_state_clear(&np_state); +} + int setup(__unused void** state) @@ -47,6 +119,7 @@ main() const struct CMUnitTest tests[] = { cmocka_unit_test(test_write_empty_state), + cmocka_unit_test(test_write_wifi_long_ssid_uses_hash), }; return cmocka_run_group_tests(tests, setup, tear_down); From bc0fc2c5592ce9bb581bffe587edf692a3025c69 Mon Sep 17 00:00:00 2001 From: Antony Rone Oliver Date: Sun, 12 Apr 2026 19:44:33 +0530 Subject: [PATCH 3/3] tests: fix long-SSID tests to use NM decimal-byte format MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 'long_ssid' ctests used raw UTF-8 emoji bytes, but g_uri_escape_string() with allow_utf8=TRUE passes valid UTF-8 sequences through unescaped (80 bytes → 107-byte basename, well under NAME_MAX). Those tests never triggered the SHA-256 hash fallback, leaving lines 649-651 and 690-691 of util.c uncovered. Change the SSID in both tests to the NM decimal-byte format ("240;159;152;128;" × 20). Semicolons encode to '%3B', yielding a 480-char escaped SSID and a 507-byte candidate basename > NAME_MAX, which correctly exercises the hash path. Also add the expected-hash assertion to test_netplan_netdef_get_output_filename_nm_with_long_ssid to verify the SHA-256 digest (not the escaped form) appears in the filename. Fix a missing LCOV_EXCL_LINE on the chown() failure branch in _netplan_g_string_free_to_file_with_permissions (consistent with the surrounding getpwnam/getgrnam failure lines already marked). --- src/util.c | 2 +- tests/ctests/test_netplan_misc.c | 36 ++++++++++++++++++++------------ 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/src/util.c b/src/util.c index 0338613f0..369ff429d 100644 --- a/src/util.c +++ b/src/util.c @@ -132,7 +132,7 @@ void _netplan_g_string_free_to_file_with_permissions(GString* s, const char* roo if (pw && gr) { ret = chown(full_path, pw->pw_uid, gr->gr_gid); if (ret != 0) { - g_debug("Failed to set owner and group for file %s: %s", full_path, strerror(errno)); + g_debug("Failed to set owner and group for file %s: %s", full_path, strerror(errno)); // LCOV_EXCL_LINE } } } diff --git a/tests/ctests/test_netplan_misc.c b/tests/ctests/test_netplan_misc.c index de0ede139..dbb0f150d 100644 --- a/tests/ctests/test_netplan_misc.c +++ b/tests/ctests/test_netplan_misc.c @@ -191,15 +191,18 @@ void test_netplan_netdef_get_output_filename_nm_with_long_ssid(__unused void** state) { NetplanNetDefinition netdef; - /* 20x U+1F600 (grinning face emoji): 80 raw UTF-8 bytes -> 240 escaped chars. - * basename = "netplan-" (8) + "wlan0" (5) + "-" (1) + 240 + ".nmconnection" (13) = 267 > NAME_MAX (255). + /* NM stores non-ASCII SSIDs as semicolon-delimited decimal bytes. + * 20x U+1F600 (😀, UTF-8: F0 9F 98 80) in NM format = "240;159;152;128;" x20 + * (320 chars, 80 semicolons). g_uri_escape_string() encodes each ';' as + * '%3B', yielding a 480-char encoded SSID. + * basename = "netplan-" (8) + "wlan0" (5) + "-" (1) + 480 + ".nmconnection" (13) = 507 > NAME_MAX. * Expects SHA-256 of raw SSID bytes used in filename instead of escaped form. */ const char ssid[] = - "\xF0\x9F\x98\x80\xF0\x9F\x98\x80\xF0\x9F\x98\x80\xF0\x9F\x98\x80" - "\xF0\x9F\x98\x80\xF0\x9F\x98\x80\xF0\x9F\x98\x80\xF0\x9F\x98\x80" - "\xF0\x9F\x98\x80\xF0\x9F\x98\x80\xF0\x9F\x98\x80\xF0\x9F\x98\x80" - "\xF0\x9F\x98\x80\xF0\x9F\x98\x80\xF0\x9F\x98\x80\xF0\x9F\x98\x80" - "\xF0\x9F\x98\x80\xF0\x9F\x98\x80\xF0\x9F\x98\x80\xF0\x9F\x98\x80"; + "240;159;152;128;240;159;152;128;240;159;152;128;240;159;152;128;" + "240;159;152;128;240;159;152;128;240;159;152;128;240;159;152;128;" + "240;159;152;128;240;159;152;128;240;159;152;128;240;159;152;128;" + "240;159;152;128;240;159;152;128;240;159;152;128;240;159;152;128;" + "240;159;152;128;240;159;152;128;240;159;152;128;240;159;152;128;"; char out_buffer[256] = { 0 }; netdef.backend = NETPLAN_BACKEND_NM; @@ -218,18 +221,25 @@ test_netplan_netdef_get_output_filename_nm_with_long_ssid(__unused void** state) assert_true(g_str_has_suffix(out_buffer, ".nmconnection")); /* Returned size must match string length + 1 */ assert_int_equal(ret, (ssize_t)(strlen(out_buffer) + 1)); + /* Must contain the SHA-256 hash of the raw SSID, not the escaped form */ + g_autofree char* expected_hash = g_compute_checksum_for_string(G_CHECKSUM_SHA256, ssid, -1); + g_autofree char* expected_suffix = g_strdup_printf("-%s.nmconnection", expected_hash); + assert_true(g_str_has_suffix(out_buffer, expected_suffix)); } void test_netplan_get_id_from_nm_filepath_with_hashed_ssid(__unused void** state) { - /* Same 20-emoji SSID as test above */ + /* Same NM decimal-byte SSID as test above; escaped form exceeds NAME_MAX + * so netplan_netdef_get_output_filename() uses the SHA-256 hash path. + * netplan_get_id_from_nm_filepath() must fall back to the hash when the + * escaped suffix is not found in the filename. */ const char ssid[] = - "\xF0\x9F\x98\x80\xF0\x9F\x98\x80\xF0\x9F\x98\x80\xF0\x9F\x98\x80" - "\xF0\x9F\x98\x80\xF0\x9F\x98\x80\xF0\x9F\x98\x80\xF0\x9F\x98\x80" - "\xF0\x9F\x98\x80\xF0\x9F\x98\x80\xF0\x9F\x98\x80\xF0\x9F\x98\x80" - "\xF0\x9F\x98\x80\xF0\x9F\x98\x80\xF0\x9F\x98\x80\xF0\x9F\x98\x80" - "\xF0\x9F\x98\x80\xF0\x9F\x98\x80\xF0\x9F\x98\x80\xF0\x9F\x98\x80"; + "240;159;152;128;240;159;152;128;240;159;152;128;240;159;152;128;" + "240;159;152;128;240;159;152;128;240;159;152;128;240;159;152;128;" + "240;159;152;128;240;159;152;128;240;159;152;128;240;159;152;128;" + "240;159;152;128;240;159;152;128;240;159;152;128;240;159;152;128;" + "240;159;152;128;240;159;152;128;240;159;152;128;240;159;152;128;"; /* Get the hashed filename from the public API */ NetplanNetDefinition netdef = { .backend = NETPLAN_BACKEND_NM, .id = "wlan0" }; char hashed_path[256] = { 0 };