ask-password: refuse agent requests with unsafe characters in prompt fields

The message, icon and id fields are written verbatim into single-line
assignments of the [Ask] section of the agent request file, so a newline in
them lets the caller append arbitrary further assignments. Agents let a later
assignment override an earlier one, so an injected Socket= line redirects the
password to a path of the injector's choosing. Validate the fields and refuse
the request instead.

diff --git a/src/shared/ask-password-api.c b/src/shared/ask-password-api.c
index e955e9d1e08999707cec52200ad6d2331679e1e6..7887d027fc5c1174386b8bcff940b1dd0492ddb9 100644 (file)
--- a/src/shared/ask-password-api.c
+++ b/src/shared/ask-password-api.c
@@ -828,6 +828,21 @@ static int create_socket(const char *askpwdir, char **ret) {
         return TAKE_FD(fd);
 }
 
+bool ask_password_agent_prompt_fields_are_safe(const AskPasswordRequest *req) {
+        /* These fields end up in single-line "Key=value" assignments in the agent request file, hence a
+         * newline (or any other control character) in them would let the caller add further assignments to
+         * the [Ask] section, which agents then happily honour. Only the characters string_is_safe() rejects
+         * by default matter here, the rest is fine to appear in a prompt. */
+        const StringSafeFlags flags = STRING_ALLOW_EMPTY | STRING_ALLOW_BACKSLASHES |
+                                      STRING_ALLOW_QUOTES | STRING_ALLOW_GLOBS;
+
+        assert(req);
+
+        return (!req->message || string_is_safe(req->message, flags)) &&
+               (!req->icon || string_is_safe(req->icon, flags)) &&
+               (!req->id || string_is_safe(req->id, flags));
+}
+
 int ask_password_agent(
                 const AskPasswordRequest *req,
                 AskPasswordFlags flags,
@@ -850,6 +865,10 @@ int ask_password_agent(
         if (req->flag_file)
                 return -EOPNOTSUPP;
 
+        if (!ask_password_agent_prompt_fields_are_safe(req))
+                return log_debug_errno(SYNTHETIC_ERRNO(EINVAL),
+                                       "Password request contains unsafe characters, refusing.");
+
         _cleanup_free_ char *askpwdir = NULL;
         r = get_ask_password_directory_for_flags(flags, &askpwdir);
         if (r < 0)

diff --git a/src/shared/ask-password-api.h b/src/shared/ask-password-api.h
index a470caf5da8f435c9058602564d43508b931e6b9..78fdfaae4dbce34910fef5540c440e36c6f0b602 100644 (file)
--- a/src/shared/ask-password-api.h
+++ b/src/shared/ask-password-api.h
@@ -32,6 +32,7 @@ typedef struct AskPasswordRequest {
 
 int ask_password_tty(const AskPasswordRequest *req, AskPasswordFlags flags, char ***ret);
 int ask_password_plymouth(const AskPasswordRequest *req, AskPasswordFlags flags, char ***ret);
+bool ask_password_agent_prompt_fields_are_safe(const AskPasswordRequest *req) _pure_;
 int ask_password_agent(const AskPasswordRequest *req, AskPasswordFlags flags, char ***ret);
 int ask_password_auto(const AskPasswordRequest *req, AskPasswordFlags flags, char ***ret);
 

diff --git a/src/test/meson.build b/src/test/meson.build
index b63007cf2546bbeabf1fae86cc2311be1c46bd21..e41233a3c8da081ecfcc90d859cf57e7d3f07419 100644 (file)
--- a/src/test/meson.build
+++ b/src/test/meson.build
@@ -60,6 +60,7 @@ simple_tests += files(
         'test-architecture.c',
         'test-argv-util.c',
         'test-arphrd-util.c',
+        'test-ask-password-agent.c',
         'test-audit-util.c',
         'test-barrier.c',
         'test-binfmt-util.c',

diff --git a/src/test/test-ask-password-agent.c b/src/test/test-ask-password-agent.c
new file mode 100644 (file)
index 0000000..71c91f4
--- /dev/null
+++ b/src/test/test-ask-password-agent.c
@@ -0,0 +1,35 @@
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+
+#include "ask-password-api.h"
+#include "tests.h"
+
+TEST(prompt_fields_are_safe) {
+        AskPasswordRequest req = {
+                .message = "Password for /dev/disk/by-label/über?",
+                .icon = "drive-harddisk",
+                .id = "cryptsetup:/dev/disk\\by-label/'data'[*]",
+                .tty_fd = -EBADF,
+                .hup_fd = -EBADF,
+        };
+
+        ASSERT_TRUE(ask_password_agent_prompt_fields_are_safe(&req));
+
+        req.message = "";
+        ASSERT_TRUE(ask_password_agent_prompt_fields_are_safe(&req));
+
+        req.message = "first line\nSocket=/tmp/injected";
+        ASSERT_FALSE(ask_password_agent_prompt_fields_are_safe(&req));
+
+        req.message = "Password:";
+        req.icon = "drive\rIcon";
+        ASSERT_FALSE(ask_password_agent_prompt_fields_are_safe(&req));
+
+        req.icon = "drive-harddisk";
+        req.id = "cryptsetup:\x01/dev/sda";
+        ASSERT_FALSE(ask_password_agent_prompt_fields_are_safe(&req));
+
+        req.id = "cryptsetup:\xff/dev/sda";
+        ASSERT_FALSE(ask_password_agent_prompt_fields_are_safe(&req));
+}
+
+DEFINE_TEST_MAIN(LOG_INFO);