1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
|
https://github.com/polkit-org/polkit/commit/9aa43e089d870a8ee695e625237c5b731b250678
From 9aa43e089d870a8ee695e625237c5b731b250678 Mon Sep 17 00:00:00 2001
From: Walter Doekes <walter+github@wjd.nu>
Date: Fri, 25 Oct 2024 23:18:16 +0200
Subject: [PATCH] pkexec: Use realpath when comparing
org.freedesktop.policykit.exec.path
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/polkit#194
See also: systemd/systemd#34714
---
src/programs/pkexec.c | 29 +++++++++++++++++++++++++++--
test/integration/pkexec/test.sh | 23 +++++++++++++++++++++++
2 files changed, 50 insertions(+), 2 deletions(-)
diff --git a/src/programs/pkexec.c b/src/programs/pkexec.c
index 65c13090..b439475f 100644
--- a/src/programs/pkexec.c
+++ b/src/programs/pkexec.c
@@ -452,6 +452,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];
@@ -508,6 +509,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;
@@ -624,6 +626,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]);
@@ -647,7 +651,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)
{
@@ -662,9 +666,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));
@@ -1084,6 +1108,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);
diff --git a/test/integration/pkexec/test.sh b/test/integration/pkexec/test.sh
index 4c76687b..e57b948f 100755
--- a/test/integration/pkexec/test.sh
+++ b/test/integration/pkexec/test.sh
@@ -142,3 +142,26 @@ 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_ABS=$(command -v bash)
+ln -s "$BASH_ABS" ./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"
+
+: "Check canonicalized path"
+if command -v strace; then
+ BASH_ABS=$(command -v bash)
+ ln -s "$BASH_ABS" ./my-bash
+ sudo -u "$TEST_USER" strace -s 512 -o "$TMP_DIR/canonical-path.strace" -feexecve \
+ expect "$TMP_DIR/basic-auth.exp" "$TEST_USER_PASSWORD" ./my-bash -c true | tee "$TMP_DIR/canonical-path.log"
+ cat "$TMP_DIR/canonical-path.strace"
+ grep -qF "execve(\"$BASH_ABS\", [\"$PWD/./my-bash\"," "$TMP_DIR/canonical-path.strace"
+ grep -q "AUTHENTICATION COMPLETE" "$TMP_DIR/canonical-path.log"
+ rm -f "$TMP_DIR/canonical-path.log" "$TMP_DIR/canonical-path.strace"
+ rm -f "./my-bash"
+ rm -f "$TMP_DIR/preload.c" "$TMP_DIR/preload.so"
+fi
|
