Bug#992058: opensysusers: uses `eval` on data that is not supposed to be safe to eval

[Lorenzo <plorenzo@disroot.org>]

Control: tags -1 patch

Hi,

On Tue, 10 Aug 2021 11:07:24 +0200 Ansgar <ansgar@debian.org> wrote:
> Package: opensysusers
> Version: 0.6-2
> Severity: serious
> Tags: security upstream
> X-Debbugs-Cc: Debian Security Team <team@security.debian.org>
> 
> opensysusers uses the shell's `eval` on everything in sysusers.d like
> there is no tomorrow. These files can contain shell meta-characters
> that should not result in code execution, e.g., in the GECOS field.
> 
> +---
> | # mkdir /etc/sysusers.d
> | # echo 'u test-user - "Do not $(rm /etc/bash.bashrc)"
> /var/lib/test-users /bin/sh' > /etc/sysusers.d/test.conf | # ls -l
> /etc/bash.bashrc | -rw-r--r-- 1 root root 1994 Jun 22 02:26
> /etc/bash.bashrc | # systemd-sysusers # this is opensysusers
> | # ls -l /etc/bash*
> | ls: cannot access '/etc/bash*': No such file or directory
> +---[ opensysusers 0.6-2 ]
> 
> systemd's systemd-sysuser behaves differently:
> 
> +---
> | # mkdir /etc/sysusers.d
> | # echo 'u test-user - "Do not $(rm /etc/bash.bashrc)"
> /var/lib/test-users /bin/sh' > /etc/sysusers.d/test.conf | # ls -l
> /etc/bash.bashrc | -rw-r--r-- 1 root root 1994 Jun 22 02:26
> /etc/bash.bashrc | # systemd-sysusers
> | Creating group systemd-coredump with gid 999.
> | Creating user systemd-coredump (systemd Core Dumper) with uid 999
> and gid 999. | Creating group test-user with gid 998.
> | Creating user test-user (Do not $(rm /etc/bash.bashrc)) with uid
> 998 and gid 998. | # ls -l /etc/bash.bashrc
> | -rw-r--r-- 1 root root 1994 Jun 22 02:26 /etc/bash.bashrc
> | # getent passwd test-user
> | test-user:x:998:998:Do not $(rm
> /etc/bash.bashrc):/var/lib/test-users:/bin/sh +---[ systemd 247.3-6 ]
> 
> As opensysusers is supposed to be a drop-in requirement for
> systemd-sysusers it *must* behave as systemd does and not execute
> data.
> 
> Ansgar
> 

Attached is a patch that sets the GECOS field without using eval: under
the assumption that the double quote character is not valid for
Type,Name,ID field it should work. Did not have the time to test it yet.
If someone has a better idea I do welcome suggestion.

Lorenzo



--- ./sysusers  2020-12-22 12:41:37.754884910 +0100
+++ ./sysusers.new      2021-09-17 19:38:32.927974348 +0200 @@ -66,10
+66,30 @@ 
 parse_string() {
        [ -n "${1%%#*}" ] || return
+       full_line=$1
 
-       eval "set -- $1"
+       #eval "set -- $1" # do not eval, see #992058 and CVE-2021-40084
+       set -- $1
        type="$1" name="$2" id="$3" gecos="$4" home="$5"
 
+       # and now set the GECOS field without eval
+       if [ "${type}" = u ]; then
+               if  [ ! -z "$4" ] && [  "$4" != '-' ]; then
+                       # strip everything before the first "
+                       gecosplus=${full_line#*\"}
+                       # now strip everything after the last "
+                       gecos=${gecosplus%\"*}
+                       # check if there are other valid fields after
GECOS
+                       gecostest=$(echo $gecosplus | grep -o '".*' -)
+                       if [ "$gecostest" = '"' ]; then
+                               home=
+                       else
+                               set -- $gecostest
+                               home=$2
+                       fi
+               fi
+       fi
+
        case "${type}" in
                [gu])
                        case "${id}" in 65535|4294967295) warninvalid;
return; esac