Skip to content

Commit e509c63

Browse files
thom311johannbg
authored andcommitted
fix(network-manager): ensure safe content of /tmp/dhclient."$ifname".dhcpopts
NetworkManager leaves state files behind in "/run/NetworkManager/devices". These files are in keyfile format (glib's GKeyFile API [1]). From the statefile, the dracut module writes a .dhcpopts file. And other users want to parse that file, for example anaconda ([2]). To be fair, anaconda seems to parse a different file, so I am a bit confused who uses this file how. In any case, it seems somebody might be tempted to execute this as a script. We need to write the .dhcpopts file in a format that is defined and easy to handle from a shell script. As already previously, this format is a bash script that sets certain variables. That means, to load the file, the user could execute it as bash script. But this is dangerous, as the file contains potentially untrusted data from the network. Optimally, users still don't trust the .dhcpopts file to be safe for executing! It would be better if users too try to parse the file instead of executing it. That is not trivial however because in face of special characters, as we use bash's `printf '%q'` to escape the values and parsing bash escaping is not trivial. Anyway, make sure we properly quote and handle the content so that also executing is safe. In the best case, there are no special characters that require escaping, and naive parsing can be done with `sed`. Otherwise, executing is now also supposed to be safe. In this case we parse DHCP options from the state file. They are themselves backslash escaped UTF-8 strings (C escape sequences), which then are stored via keyfile API. The properly parse them, we would first need to load the file with GKeyFile (which undoes one level of backslash escaping) and then use g_str_compress() (to undo the second level). We mimic that with shell. [1] https://github.com/rhinstaller/anaconda/blob/b3411d6780aa0d76ee1e81a38710ec05a2d1978b/dracut/fetch-kickstart-net.sh#L30 [2] https://developer.gnome.org/glib/stable/glib-Key-value-file-parser.html Signed-off-by: Thomas Haller <[email protected]>
1 parent c868feb commit e509c63

File tree

2 files changed

+40
-6
lines changed

2 files changed

+40
-6
lines changed

modules.d/35network-manager/module-setup.sh

+1-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ check() {
1010

1111
# called by dracut
1212
depends() {
13-
echo dbus
13+
echo dbus bash
1414
return 0
1515
}
1616

modules.d/35network-manager/nm-run.sh

+39-5
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
#!/bin/sh
1+
#!/bin/bash
22

33
type source_hook > /dev/null 2>&1 || . /lib/dracut-lib.sh
44

@@ -24,11 +24,45 @@ if [ -s /run/NetworkManager/initrd/hostname ]; then
2424
cat /run/NetworkManager/initrd/hostname > /proc/sys/kernel/hostname
2525
fi
2626

27+
kf_get_string() {
28+
# NetworkManager writes keyfiles (glib's GKeyFile API). Have a naive
29+
# parser for it.
30+
#
31+
# But GKeyFile will backslash escape certain keys (\s, \t, \n) but also
32+
# escape backslash. As an approximation, interpret the string with printf's
33+
# '%b'.
34+
#
35+
# This is supposed to mimic g_key_file_get_string() (poorly).
36+
37+
v1="$(sed -n "s/^$1=/=/p" | sed '1!d')"
38+
test "$v1" = "${v1#=}" && return 1
39+
printf "%b" "${v1#=}"
40+
}
41+
42+
kf_unescape() {
43+
# Another layer of unescaping. While values in GKeyFile format
44+
# are backslash escaped, the original strings (which are in no
45+
# defined encoding) are backslash escaped too to be valid UTF-8.
46+
# This will undo the second layer of escaping to give binary "strings".
47+
printf "%b" "$1"
48+
}
49+
50+
kf_parse() {
51+
v3="$(kf_get_string "$1")" || return 1
52+
v3="$(kf_unescape "$v3")"
53+
printf '%s=%s\n' "$2" "$(printf '%q' "$v3")"
54+
}
55+
56+
dhcpopts_create() {
57+
kf_parse root-path new_root_path < "$1"
58+
kf_parse next-server new_next_server < "$1"
59+
}
60+
2761
for _i in /sys/class/net/*; do
28-
state=/run/NetworkManager/devices/$(cat "$_i"/ifindex)
29-
grep -q connection-uuid= "$state" 2> /dev/null || continue
30-
ifname=${_i##*/}
31-
sed -n 's/root-path/new_root_path/p;s/next-server/new_next_server/p' < "$state" > /tmp/dhclient."$ifname".dhcpopts
62+
state="/run/NetworkManager/devices/$(cat "$_i"/ifindex)"
63+
grep -q '^connection-uuid=' "$state" 2> /dev/null || continue
64+
ifname="${_i##*/}"
65+
dhcpopts_create "$state" > /tmp/dhclient."$ifname".dhcpopts
3266
source_hook initqueue/online "$ifname"
3367
/sbin/netroot "$ifname"
3468
done

0 commit comments

Comments
 (0)