Skip to content

Commit

Permalink
pkexec: Use realpath when comparing org.freedesktop.policykit.exec.path
Browse files Browse the repository at this point in the history
This changes the pkexec path that is compared from the original supplied
path to the path resolved by realpath(3).

That means that "/bin/something" might now be matched as
"/usr/bin/something", a review of your
  <annotate key="org.freedesktop.policykit.exec.path">
actions might be in order.

Fixes: polkit-org#194

See also: systemd/systemd#34714
  • Loading branch information
wdoekes committed Oct 25, 2024
1 parent 99f67a4 commit 7488e4b
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 2 deletions.
29 changes: 27 additions & 2 deletions src/programs/pkexec.c
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,7 @@ main (int argc, char *argv[])
gchar *action_id;
gboolean allow_gui;
gchar **exec_argv;
gchar *path_abs;
gchar *path;
struct passwd pwstruct;
gchar pwbuf[8192];
Expand Down Expand Up @@ -507,6 +508,7 @@ main (int argc, char *argv[])
result = NULL;
action_id = NULL;
saved_env = NULL;
path_abs = NULL;
path = NULL;
exec_argv = NULL;
command_line = NULL;
Expand Down Expand Up @@ -623,6 +625,8 @@ main (int argc, char *argv[])
* but do check this is the case.
*
* We also try to locate the program in the path if a non-absolute path is given.
*
* And then we resolve the real path of the program.
*/
g_assert (argv[argc] == NULL);
path = g_strdup (argv[n]);
Expand All @@ -646,7 +650,7 @@ main (int argc, char *argv[])
}
if (path[0] != '/')
{
/* g_find_program_in_path() is not suspectible to attacks via the environment */
/* g_find_program_in_path() is not susceptible to attacks via the environment */
s = g_find_program_in_path (path);
if (s == NULL)
{
Expand All @@ -661,9 +665,29 @@ main (int argc, char *argv[])
*/
if (argv[n] != NULL)
{
argv[n] = path;
/* Must copy because we might replace path later on. */
path_abs = g_strdup(path);
/* argv[n:] is used as argv arguments to execv(). The called program
* sees the original called path, but we make sure it's absolute. */
if (path_abs != NULL)
argv[n] = path_abs;
}
}
#if _POSIX_C_SOURCE >= 200809L
s = realpath(path, NULL);
#else
s = NULL;
# error We have to deal with realpath(3) PATH_MAX madness
#endif
if (s != NULL)
{
/* The called program resolved to the canonical location. We don't update
* argv[n] this time. The called program still sees the original
* called path. This is very important for multi-call binaries like
* busybox. */
g_free (path);
path = s;
}
if (access (path, F_OK) != 0)
{
g_printerr ("Error accessing %s: %s\n", path, g_strerror (errno));
Expand Down Expand Up @@ -1078,6 +1102,7 @@ main (int argc, char *argv[])
}

g_free (original_cwd);
g_free (path_abs);
g_free (path);
g_free (command_line);
g_free (cmdline_short);
Expand Down
9 changes: 9 additions & 0 deletions test/integration/pkexec/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -142,3 +142,12 @@ sudo -u "$TEST_USER" expect "$TMP_DIR/SIGTRAP-on-EOF.exp" | tee "$TMP_DIR/SIGTRA
grep -q "AUTHENTICATION FAILED" "$TMP_DIR/SIGTRAP-on-EOF.log"
grep -q "Not authorized" "$TMP_DIR/SIGTRAP-on-EOF.log"
rm -f "$TMP_DIR/SIGTRAP-on-EOF.log"

: "Check absolute (but not canonicalized) path"
BASH_PATH=$(command -v bash)
ln -s "$BASH_PATH" ./my-bash
sudo -u "$TEST_USER" expect "$TMP_DIR/basic-auth.exp" "$TEST_USER_PASSWORD" ./my-bash -c true | tee "$TMP_DIR/absolute-path.log"
grep -Eq "Authentication is needed to run \`/.*/${PWD##*/}/./my-bash -c true' as the super user" "$TMP_DIR/absolute-path.log"
grep -q "AUTHENTICATION COMPLETE" "$TMP_DIR/absolute-path.log"
rm -f "$TMP_DIR/absolute-path.log"
rm -f "./my-bash"

0 comments on commit 7488e4b

Please sign in to comment.