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

2021-09-17 Thread Lorenzo
Control: tags -1 patch

Hi,

On Tue, 10 Aug 2021 11:07:24 +0200 Ansgar  wrote:
> Package: opensysusers
> Version: 0.6-2
> Severity: serious
> Tags: security upstream
> X-Debbugs-Cc: Debian Security Team 
> 
> 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



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

2021-08-10 Thread Ansgar
Package: opensysusers
Version: 0.6-2
Severity: serious
Tags: security upstream
X-Debbugs-Cc: Debian Security Team 

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