diff mbox series

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

Message ID 20261104222956.882475-1-selva.nair@gmail.com
State New
Headers
Series [Openvpn-devel,for,2.7] misc: Fix username overflow spilling into password |

Commit Message

Selva Nair Nov. 4, 2026, 10:29 p.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.

For release/2.7, adapt the test temporary-directory lookup because
platform_get_tmp_dir() is only available on master.

Github: OpenVPN/openvpn#1140
Change-Id: I3c8c62c7d90c2b94dd1cd149b4c13b383a2303e9
Signed-off-by: Selva Nair <selva.nair@gmail.com>
---
 src/openvpn/misc.c                        | 12 +++++
 tests/unit_tests/openvpn/test_user_pass.c | 63 +++++++++++++++++++++++
 2 files changed, 75 insertions(+)
diff mbox series

Patch

diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c
index 1a23e13b..a4280cb9 100644
--- a/src/openvpn/misc.c
+++ b/src/openvpn/misc.c
@@ -255,6 +255,10 @@  get_user_pass_cr(struct user_pass *up, const char *auth_file, const char *prefix
                 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);
 
@@ -288,6 +292,14 @@  get_user_pass_cr(struct user_pass *up, const char *auth_file, const char *prefix
                         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 5ad4730b..a0206296 100644
--- a/tests/unit_tests/openvpn/test_user_pass.c
+++ b/tests/unit_tests/openvpn/test_user_pass.c
@@ -366,6 +366,68 @@  test_get_user_pass_authfile_file(void **state)
     assert_string_equal(up.password, "fuser");
 }
 
+static void
+test_get_user_pass_long_username(void **state)
+{
+    struct gc_arena gc = gc_new();
+#ifdef _WIN32
+    const char *tmpdir = win_get_tempdir();
+#else
+    const char *tmpdir = getenv("TMPDIR");
+    if (!tmpdir)
+    {
+        tmpdir = "/tmp";
+    }
+#endif
+    assert_non_null(tmpdir);
+    const char *authfile = platform_create_temp_file(tmpdir, "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 +536,7 @@  const struct CMUnitTest user_pass_tests[] = {
     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),