diff mbox series

[Openvpn-devel,v2] misc: Fix username overflow spilling into password

Message ID 20261103081357.8587-1-gert@greenie.muc.de
State New
Headers
Series [Openvpn-devel,v2] misc: Fix username overflow spilling into password |

Commit Message

Gert Doering Nov. 3, 2026, 8:13 a.m. UTC 
  From: Selva Nair <selva.nair@gmail.com>

Consume the complete username line before reading the password
from an auth file or inline credentials. This also fixes an
exactly full username buffer leaving its newline to be read as
an empty password in auth files.

Existing credential truncation behaviour is unchanged.

Tests: Add regression coverage for long and boundary-length
usernames with missing, blank, or supplied passwords in both
inlined and external file inputs.

Github: OpenVPN/openvpn#1140
Change-Id: I3c8c62c7d90c2b94dd1cd149b4c13b383a2303e9
Signed-off-by: Selva Nair <selva.nair@gmail.com>
Acked-by: Razvan Cojocaru <razvanc@mailbox.org>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/2096
---

This change was reviewed on Gerrit and approved by at least one
developer. I request to merge it to master.

Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/2096
This mail reflects revision 2 of this Change.

Acked-by according to Gerrit (reflected above):
Razvan Cojocaru <razvanc@mailbox.org>
diff mbox series

Patch

diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c
index 304a87e..8c9ad08 100644
--- a/src/openvpn/misc.c
+++ b/src/openvpn/misc.c
@@ -250,6 +250,10 @@ 
                 if (!(flags & GET_USER_PASS_PASSWORD_ONLY))
                 {
                     buf_parse(&buf, '\n', up->username, USER_PASS_LEN);
+                    /* The password starts on the next line, even if the username was truncated. */
+                    const char *password = strchr(auth_file, '\n');
+                    password = password ? password + 1 : "";
+                    buf_set_read(&buf, (uint8_t *)password, strlen(password) + 1);
                 }
                 buf_parse(&buf, '\n', up->password, USER_PASS_LEN);
 
@@ -283,6 +287,14 @@ 
                         msg(M_FATAL, "Error reading username from %s authfile: %s", prefix,
                             auth_file);
                     }
+                    if (!strchr(up->username, '\n'))
+                    {
+                        /* Consume the rest of the username line before reading the password. */
+                        int c;
+                        while ((c = fgetc(fp)) != '\n' && c != EOF)
+                        {
+                        }
+                    }
                 }
                 chomp(up->username);
 
diff --git a/tests/unit_tests/openvpn/test_user_pass.c b/tests/unit_tests/openvpn/test_user_pass.c
index adcf74a..776a86a 100644
--- a/tests/unit_tests/openvpn/test_user_pass.c
+++ b/tests/unit_tests/openvpn/test_user_pass.c
@@ -366,6 +366,58 @@ 
     assert_string_equal(up.password, "fuser");
 }
 
+static void
+test_get_user_pass_long_username(void **state)
+{
+    struct gc_arena gc = gc_new();
+    const char *authfile = platform_create_temp_file(platform_get_tmp_dir(), "user_pass", &gc);
+    assert_non_null(authfile);
+    const size_t lengths[] = { USER_PASS_LEN - 2, USER_PASS_LEN - 1, USER_PASS_LEN + 10 };
+    const char *endings[] = { "", "\n", "\n\n", "\nfpassword\n", "\r\nfpassword\r\n" };
+
+    const bool inline_mode[] = { false, true };
+    for (size_t mode = 0; mode < SIZE(inline_mode); mode++)
+    {
+        const bool use_inline_creds = inline_mode[mode];
+        for (size_t i = 0; i < SIZE(lengths); i++)
+        {
+            for (size_t j = 0; j < SIZE(endings); j++)
+            {
+                struct user_pass up = { 0 };
+                char creds[USER_PASS_LEN + 64];
+                memset(creds, 'u', lengths[i]);
+                strcpy(creds + lengths[i], endings[j]);
+                if (!use_inline_creds)
+                {
+                    FILE *fp = platform_fopen(authfile, "w");
+                    assert_non_null(fp);
+                    assert_true(fputs(creds, fp) >= 0);
+                    assert_int_equal(fclose(fp), 0);
+                }
+
+                const bool missing_password = j < 3;
+                if (missing_password)
+                {
+                    expect_string(query_user_exec_builtin, query_user[i].prompt,
+                                  "Enter UT Password:");
+                    will_return(query_user_exec_builtin, "cpassword");
+                }
+                will_return(query_user_exec_builtin, true);
+                assert_true(get_user_pass_cr(&up, use_inline_creds ? creds : authfile, "UT",
+                                             use_inline_creds ? GET_USER_PASS_INLINE_CREDS : 0, NULL));
+                /* Truncation must not change where the password line starts. */
+                creds[lengths[i] < USER_PASS_LEN ? lengths[i] : USER_PASS_LEN - 1] = '\0';
+                assert_true(up.defined);
+                assert_string_equal(up.username, creds);
+                assert_string_equal(up.password,
+                                    missing_password ? "cpassword" : "fpassword");
+            }
+        }
+    }
+    assert_true(platform_unlink(authfile));
+    gc_free(&gc);
+}
+
 #ifdef ENABLE_MANAGEMENT
 static void
 test_get_user_pass_dynamic_challenge(void **state)
@@ -474,6 +526,7 @@ 
     cmocka_unit_test(test_get_user_pass_inline_creds),
     cmocka_unit_test(test_get_user_pass_authfile_stdin),
     cmocka_unit_test(test_get_user_pass_authfile_file),
+    cmocka_unit_test(test_get_user_pass_long_username),
 #ifdef ENABLE_MANAGEMENT
     cmocka_unit_test(test_get_user_pass_dynamic_challenge),
     cmocka_unit_test(test_get_user_pass_static_challenge),