From e509c638e68a8e3cae446d1a4f9f86e3aa6e7a99 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Tue, 27 Jul 2021 16:04:58 +0200
Subject: [PATCH] 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 <thaller@redhat.com>
---
 modules.d/35network-manager/module-setup.sh |  2 +-
 modules.d/35network-manager/nm-run.sh       | 44 ++++++++++++++++++---
 2 files changed, 40 insertions(+), 6 deletions(-)

diff --git a/modules.d/35network-manager/module-setup.sh b/modules.d/35network-manager/module-setup.sh
index ab2afa2c51..04898a39e7 100755
--- a/modules.d/35network-manager/module-setup.sh
+++ b/modules.d/35network-manager/module-setup.sh
@@ -10,7 +10,7 @@ check() {
 
 # called by dracut
 depends() {
-    echo dbus
+    echo dbus bash
     return 0
 }
 
diff --git a/modules.d/35network-manager/nm-run.sh b/modules.d/35network-manager/nm-run.sh
index 359bc9ee7d..28ad4aaa5c 100755
--- a/modules.d/35network-manager/nm-run.sh
+++ b/modules.d/35network-manager/nm-run.sh
@@ -1,4 +1,4 @@
-#!/bin/sh
+#!/bin/bash
 
 type source_hook > /dev/null 2>&1 || . /lib/dracut-lib.sh
 
@@ -24,11 +24,45 @@ if [ -s /run/NetworkManager/initrd/hostname ]; then
     cat /run/NetworkManager/initrd/hostname > /proc/sys/kernel/hostname
 fi
 
+kf_get_string() {
+    # NetworkManager writes keyfiles (glib's GKeyFile API). Have a naive
+    # parser for it.
+    #
+    # But GKeyFile will backslash escape certain keys (\s, \t, \n) but also
+    # escape backslash. As an approximation, interpret the string with printf's
+    # '%b'.
+    #
+    # This is supposed to mimic g_key_file_get_string() (poorly).
+
+    v1="$(sed -n "s/^$1=/=/p" | sed '1!d')"
+    test "$v1" = "${v1#=}" && return 1
+    printf "%b" "${v1#=}"
+}
+
+kf_unescape() {
+    # Another layer of unescaping. While values in GKeyFile format
+    # are backslash escaped, the original strings (which are in no
+    # defined encoding) are backslash escaped too to be valid UTF-8.
+    # This will undo the second layer of escaping to give binary "strings".
+    printf "%b" "$1"
+}
+
+kf_parse() {
+    v3="$(kf_get_string "$1")" || return 1
+    v3="$(kf_unescape "$v3")"
+    printf '%s=%s\n' "$2" "$(printf '%q' "$v3")"
+}
+
+dhcpopts_create() {
+    kf_parse root-path new_root_path < "$1"
+    kf_parse next-server new_next_server < "$1"
+}
+
 for _i in /sys/class/net/*; do
-    state=/run/NetworkManager/devices/$(cat "$_i"/ifindex)
-    grep -q connection-uuid= "$state" 2> /dev/null || continue
-    ifname=${_i##*/}
-    sed -n 's/root-path/new_root_path/p;s/next-server/new_next_server/p' < "$state" > /tmp/dhclient."$ifname".dhcpopts
+    state="/run/NetworkManager/devices/$(cat "$_i"/ifindex)"
+    grep -q '^connection-uuid=' "$state" 2> /dev/null || continue
+    ifname="${_i##*/}"
+    dhcpopts_create "$state" > /tmp/dhclient."$ifname".dhcpopts
     source_hook initqueue/online "$ifname"
     /sbin/netroot "$ifname"
 done