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.
This commit is contained in:
committed by
Yu Watanabe
parent
a8e93919c3
commit
151dafcf16
@@ -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)
|
||||
|
||||
@@ -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);
|
||||
|
||||
|
||||
@@ -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',
|
||||
|
||||
@@ -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);
|
||||
Reference in New Issue
Block a user