Control: tags -1 patch
Hi,
On Tue, 10 Aug 2021 11:07:24 +0200 Ansgar <ans...@debian.org> wrote:
> Package: opensysusers
> Version: 0.6-2
> Severity: serious
> Tags: security upstream
> X-Debbugs-Cc: Debian Security Team <t...@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
