Bug#984590: scanlogd: /run/scanlogd/empty must not be writeable by scanlogd process

[Solar Designer]

Hi Mike,

Thank you for creating the bug.

On Fri, Mar 05, 2021 at 02:32:54PM +, Mike Gabriel wrote:
>  if [ ! -d $RDIR/empty ]; then
>  mkdir -p $RDIR/empty
> -chown -R scanlogd:nogroup $RDIR
> +chown scanlogd:nogroup $RDIR
> +chown root:root $RDIR/empty
>  fi

> @Alexander: you ok with this change? It should be sufficient, shouldn't it?

No, and no.  Two issues here:

1. ALL directories in the path must not be writable by any user other
than root.  This means all must have root as their owner.

2. If someone had already installed the previous package revision, these
new chown's wouldn't be reached (because of the "if") and thus insecure
permissions would persist over a package upgrade and service restart.

I think we need this:

mkdir -p $RDIR/empty
# The directory and its parent directories must not be writable by anyone but 
root
chown root:root $RDIR $RDIR/empty

and drop the "if".  "mkdir -p" is happy to ignore an already existing
directory (this is part of what the "-p" option does).

So just these 3 lines (including the comment) instead of the entire
if/fi block.

Alexander

Bug#984590: scanlogd: /run/scanlogd/empty must not be writeable by scanlogd process

[Mike Gabriel]

Package: scanlogd
Severity: important
Version: 2.2.7-0.1
X-Debbugs-Cc: so...@openwall.com

- Weitergeleitete Nachricht von Solar Designer  -
  Datum: Thu, 4 Mar 2021 14:25:23 +0100
Von: Solar Designer 
Betreff: Re: Debian scanlogd
 An: Mike Gabriel 
 Cc: sunwea...@debian.org

Hi Mike,

On Sat, Feb 27, 2021 at 02:08:21PM +0100, Solar Designer wrote:

Unfortunately, there's still a major issue:

As the comment in params.h says, "The directory and its parent
directories must not be writable by anyone but root."  However, you set
the directory to /run/scanlogd/empty and then do:

# the rundir is used for chroot'ing scanlogd into before
# starting with portscan detections
umask 022
if [ ! -d $RDIR/empty ]; then
mkdir -p $RDIR/empty
chown -R scanlogd:nogroup $RDIR
fi

The "chown" breaks scanlogd's security hardening, and must be removed.


If you're not going to fix this now, then let's create a Debian bug for
it, so that it's not forgotten.  Will you, please?

Alexander

- Ende der weitergeleiteten Nachricht -

The propose patch for this is:

```
[sunweaver@sunobo scanlogd (master)]$ git diff
diff --git a/debian/scanlogd.init b/debian/scanlogd.init
index 1095d97..90c9d8e 100644
--- a/debian/scanlogd.init
+++ b/debian/scanlogd.init
@@ -33,7 +33,8 @@ set -e
 umask 022
 if [ ! -d $RDIR/empty ]; then
 mkdir -p $RDIR/empty
-chown -R scanlogd:nogroup $RDIR
+chown scanlogd:nogroup $RDIR
+chown root:root $RDIR/empty
 fi

 case "$1" in

```

@Alexander: you ok with this change? It should be sufficient, shouldn't it?

Mike



--

mike gabriel aka sunweaver (Debian Developer)
mobile: +49 (1520) 1976 148
landline: +49 (4351) 486 14 27

GnuPG Fingerprint: 9BFB AEE8 6C0A A5FF BF22  0782 9AF4 6B30 2577 1B31
mail: sunwea...@debian.org, http://sunweavers.net



pgphTHuTiGcND.pgp
Description: Digitale PGP-Signatur