[SCM] GNU Inetutils branch, master, updated. inetutils-1_9_2-39-g3a46064

[Mats Erik Andersson]

This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GNU Inetutils ".

The branch, master has been updated
       via  3a46064ca67ec96e3128e686268b45edc1c2191e (commit)
      from  c10c9f97862ce98439f1e52b2922235d5b997e3a (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
http://git.savannah.gnu.org/cgit/inetutils.git/commit/?id=3a46064ca67ec96e3128e686268b45edc1c2191e


commit 3a46064ca67ec96e3128e686268b45edc1c2191e
Author: Mats Erik Andersson <address@hidden>
Date:   Fri Oct 17 22:09:13 2014 +0200

    syslogd, logger: Incomplete range checking.
    
    Protect against attacks using excessive facility numbers.

diff --git a/ChangeLog b/ChangeLog
index e0dd174..f1f1bd1 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,36 @@
+2014-10-17  Mats Erik Andersson  <address@hidden>
+
+       syslogd, logger: Incomplete range checking.
+
+       * src/logger.c (IU_MAX_FAC): Delete macro.
+       (decode): Use LOG_FACMASK in range checking.  The use of
+       numeric facilities was broken for the most part.
+
+       * src/logprio.h [!LOG_MAKEPRI] (LOG_MAKEPRI): Change to
+       a portable and correct definition.
+       [!INTERNAL_MARK] (INTERNAL_MARK): Adjust to the corrected
+       behaviour of LOG_MAKEPRI.
+
+       * src/syslogd.c [!LOG_MAKEPRI] (LOG_MAKEPRI): Change to
+       a portable and correct definition.
+       (printline): Reset `pri' to DEFUPRI whenever the facility
+       is non-existent.  This protects against CVE-2014-3684.
+       Make a three bit shift of LOG_KERN for code consistency.
+       (cfline): After decoding priority, check also upper limit.
+       Likewise for facility, and update `facilities_seen' only
+       after a completed range error check.
+
+       * tests/syslogd.sh (OUT_UNOTICE, OUT_LOCAL0): New variables.
+       (COUNT4, COUNT5, COUNT4_notice, COUNT4_illegal, COUNT5_user)
+       (COUNT5_local): Likewise.
+       Add three new configuration stanzas to initial setup, all with
+       invalid information.  In the second setup, capture facility
+       `info' exactly, and make a new `notice' also exact.  Add a
+       further file in the conf-directory, specifying capture of
+       `local0.=notice'.  Add five new test messages using Unix
+       socket, intending to detect more accurately the handling of
+       priorities, numerical as well as illegal or default values.
+
 2014-08-29  Mats Erik Andersson  <address@hidden>
 
        * tests/libls.sh: Add a dummy execution of $LS in order
diff --git a/src/logger.c b/src/logger.c
index 57e23b0..f628511 100644
--- a/src/logger.c
+++ b/src/logger.c
@@ -72,12 +72,6 @@ static int host_family = AF_INET;
 
 
 
-#ifdef LOG_NFACILITIES
-# define IU_MAX_FAC LOG_NFACILITIES
-#else
-# define IU_MAX_FAC 32
-#endif
-
 int
 decode (char *name, CODE *codetab, const char *what)
 {
@@ -88,8 +82,16 @@ decode (char *name, CODE *codetab, const char *what)
       char *p;
       unsigned long n = strtoul (name, &p, 0);
 
-      if (*p || n >= IU_MAX_FAC)       /* Includes range errors.  */
-       error (EXIT_FAILURE, 0, "%s: invalid %s number", what, name);
+      /* For portability reasons, a numerical facility is entered
+       * as a decimal integer, shifted left by three binary bits.
+       * Any overflow check must adapt to this fact.
+       * For the purpose of remote logging from a local system,
+       * tests based on LOG_NFACILITIES are insufficient, as a
+       * remote system may well distinguish more facilities than
+       * the local system does!
+       */
+      if (*p || n > LOG_FACMASK)       /* Includes range errors.  */
+       error (EXIT_FAILURE, 0, "invalid %s number: %s", what, name);
 
       return n;
     }
diff --git a/src/logprio.h b/src/logprio.h
index 907fa3f..abbe7c4 100644
--- a/src/logprio.h
+++ b/src/logprio.h
@@ -67,7 +67,7 @@ typedef struct _code {
 #endif
 
 #ifndef LOG_MAKEPRI
-#  define LOG_MAKEPRI(fac, p)  (((fac) << 3) | (p))
+#  define LOG_MAKEPRI(fac, p)  ((fac) | (p))
 #endif
 
 #ifndef INTERNAL_NOPRI
@@ -75,7 +75,7 @@ typedef struct _code {
 #endif
 
 #ifndef INTERNAL_MARK  /* mark "facility", first unused value */
-#  define INTERNAL_MARK        LOG_MAKEPRI(LOG_NFACILITIES, 0)
+#  define INTERNAL_MARK        LOG_MAKEPRI(LOG_NFACILITIES << 3, 0)
 #endif
 
 #ifndef LOG_FACMASK
diff --git a/src/syslogd.c b/src/syslogd.c
index cc96439..c999466 100644
--- a/src/syslogd.c
+++ b/src/syslogd.c
@@ -117,7 +117,7 @@
 #endif
 
 #ifndef LOG_MAKEPRI
-#  define LOG_MAKEPRI(fac, p)  (((fac) << 3) | (p))
+#  define LOG_MAKEPRI(fac, p)  ((fac) | (p))
 #endif
 
 #include <error.h>
@@ -1060,11 +1060,17 @@ printline (const char *hname, const char *msg)
       if (*p == '>')
        ++p;
     }
+
+  /* This overrides large positive and overflowing negative values.  */
   if (pri & ~(LOG_FACMASK | LOG_PRIMASK))
     pri = DEFUPRI;
 
-  /* don't allow users to log kernel messages */
-  if (LOG_FAC (pri) == LOG_KERN)
+  /* Avoid undefined facilities.  */
+  if (LOG_FAC (pri) > LOG_NFACILITIES)
+    pri = DEFUPRI;
+
+  /* Do not allow users to log kernel messages.  */
+  if (LOG_FAC (pri) == (LOG_KERN >> 3))
     pri = LOG_MAKEPRI (LOG_USER, LOG_PRI (pri));
 
   q = line;
@@ -2242,7 +2248,7 @@ cfline (const char *line, struct filed *f)
       else
        {
          pri = decode (bp, prioritynames);
-         if (pri < 0)
+         if (pri < 0 || (pri > LOG_PRIMASK && pri != INTERNAL_NOPRI))
            {
              snprintf (ebuf, sizeof (ebuf),
                        "unknown priority name \"%s\"", bp);
@@ -2288,9 +2294,8 @@ cfline (const char *line, struct filed *f)
          else
            {
              i = decode (buf, facilitynames);
-             facilities_seen |= (1 << LOG_FAC (i));
 
-             if (i < 0)
+             if (i < 0 || i > (LOG_NFACILITIES << 3))
                {
                  snprintf (ebuf, sizeof (ebuf),
                            "unknown facility name \"%s\"", buf);
@@ -2300,6 +2305,8 @@ cfline (const char *line, struct filed *f)
 
              f->f_pmask[LOG_FAC (i)] &= ~pri_clear;
              f->f_pmask[LOG_FAC (i)] |= pri_set;
+
+             facilities_seen |= (1 << LOG_FAC (i));
            }
          while (*p == ',' || *p == ' ')
            p++;
diff --git a/tests/syslogd.sh b/tests/syslogd.sh
index fda16f3..0885001 100755
--- a/tests/syslogd.sh
+++ b/tests/syslogd.sh
@@ -101,12 +101,6 @@ fi
 SYSLOGD=${SYSLOGD:-../src/syslogd$EXEEXT}
 LOGGER=${LOGGER:-../src/logger$EXEEXT}
 
-if [ $VERBOSE ]; then
-    set -x
-    $SYSLOGD --version | $SED '1q'
-    $LOGGER --version | $SED '1q'
-fi
-
 if [ ! -x $SYSLOGD ]; then
     echo "Missing executable '$SYSLOGD'.  Failing." >&2
     exit 77
@@ -117,6 +111,12 @@ if [ ! -x $LOGGER ]; then
     exit 77
 fi
 
+if [ $VERBOSE ]; then
+    set -x
+    $SYSLOGD --version | $SED '1q'
+    $LOGGER --version | $SED '1q'
+fi
+
 # For file creation below IU_TESTDIR.
 umask 0077
 
@@ -132,7 +132,7 @@ if [ ! -d "$IU_TESTDIR" ]; then
            echo 'Failed at creating test directory.  Aborting.' >&2
            exit 77
        }
-elif expr X"$IU_TESTDIR" : X"\.\{1,2\}/\{0,1\}$" >/dev/null; then
+elif expr X"$IU_TESTDIR" : X'\.\{1,2\}/\{0,1\}$' >/dev/null; then
     # Eliminating directories: . ./ .. ../
     echo 'Dangerous input for test directory.  Aborting.' >&2
     exit 77
@@ -220,7 +220,7 @@ if test ! -d "$IU_TMPDIR"; then
        EOT
 else
     # Append a slash if it is missing.
-    expr X"$IU_TMPDIR" : X".*/$" >/dev/null || IU_TMPDIR="$IU_TMPDIR/"
+    expr X"$IU_TMPDIR" : X'.*/$' >/dev/null || IU_TMPDIR="$IU_TMPDIR/"
 
     IU_TMPDIR="`$MKTEMP -d "${IU_TMPDIR}iu.XXXXXX" 2>/dev/null`" ||
        {   # Directory creation failed.  Disable test.
@@ -308,14 +308,22 @@ if locate_port $PROTO $PORT; then
     do_inet_socket=false
 fi
 
-# A minimal, catch-all configuration.
+# A minimal, catch-all configuration, with some discarded oddities.
 #
 cat > "$CONF" <<-EOT
        *.*     $OUT
-       # Recover from missing action field and short selector.
+       # Empty priority and action.
        12345
+       # Missing action.
        *.*
+       # Empty priority.
        *.      /dev/null
+       # Priority out of range.
+       user.8  /dev/null
+       # Unknown facility.
+       bom.7   /dev/null
+       # Facility out of range for all known systems.
+       512.err /dev/null
 EOT
 
 # Add a user recipient in verbose mode.
@@ -432,22 +440,31 @@ fi
 # discrimination of severity and facility.  This is made active
 # by sending SIGHUP to the server process.
 #
+OUT_UNOTICE="$IU_TESTDIR"/usernotice.log
 OUT_USER="$IU_TESTDIR"/user.log
 OUT_DEBUG="$IU_TESTDIR"/debug.log
+OUT_LOCAL0="$IU_TESTDIR"/local0.log
 
 # Create the new files to avoid false negatives.
+: > "$OUT_UNOTICE"
 : > "$OUT_USER"
 : > "$OUT_DEBUG"
+: > "$OUT_LOCAL0"
 
 cat > "$CONF" <<-EOT
        *.*             $OUT
-       user.info       $OUT_USER
+       user.=info      $OUT_USER
+       user.=notice    $OUT_UNOTICE
 EOT
 
 cat > "$CONFD/debug" <<-EOT
        *.=debug        $OUT_DEBUG
 EOT
 
+cat > "$CONFD/local0" <<-EOT
+       local0.=notice  $OUT_LOCAL0
+EOT
+
 # Use another tag for better discrimination.
 TAG2="syslogd-reload-test"
 
@@ -463,6 +480,39 @@ if $do_unix_socket; then
        "user.info as BSD message. (pid $$)"
     $LOGGER -h "$SOCKET" -p user.debug -t "$TAG2" \
        "user.debug as BSD message. (pid $$)"
+
+    # Numerical priority:
+    #   user.info = (1 << 3).6 = 8.6
+    TESTCASES=`expr $TESTCASES + 2`
+    $LOGGER -h "$SOCKET" -p 8.6 -t "$TAG2" \
+       "user.info using numerical priority. (pid $$)"
+
+    # Numerical facility and default priority:
+    #   local0.notice = (16 << 3).5 = 128.5
+    TESTCASES=`expr $TESTCASES + 2`
+    $LOGGER -h "$SOCKET" -p 128 -t "$TAG2" \
+       "local0.notice using numerical facility. (pid $$)"
+
+    # Default facility and priority:
+    #   user.notice
+    TESTCASES=`expr $TESTCASES + 2`
+    $LOGGER -h "$SOCKET" -t "$TAG2" \
+       "Default facility in BSD message. (pid $$)"
+
+    # A message of facility `kern' from a user process should
+    # be rewritten for `user' by SYSLOGD.
+    TESTCASES=`expr $TESTCASES + 2`
+    $LOGGER -h "$SOCKET" -t "$TAG2" \
+       "Fake kern facility in BSD message. (pid $$)"
+
+    # An undefined facility should be rewritten for the default
+    # facility `user' by SYSLOGD.  Typically, LOG_NFACILITIES
+    # is 24, while LOG_FACMASK is 1016, so a value in excess
+    # of (24 << 8), i.e., 192 is sufficient.
+    # The priority `info' will be overwritten as `notice'.
+    TESTCASES=`expr $TESTCASES + 2`
+    $LOGGER -h "$SOCKET" -p 512.info -t "$TAG2" \
+       "Illegal facility in BSD message. (pid $$)"
 fi
 
 if $do_inet_socket; then
@@ -516,6 +566,11 @@ COUNT=`$GREP -c "$TAG" "$OUT"`
 # Second set-up after SIGHUP.
 COUNT2=`$GREP -c "$TAG2" "$OUT_USER"`
 COUNT3=`$GREP -c "$TAG2" "$OUT_DEBUG"`
+COUNT4=`$GREP -c -e "$TAG2.*Default facility" \
+                -e "$TAG2.*Fake kern facility" \
+                -e "$TAG2.*Illegal facility" \
+       "$OUT_UNOTICE"`
+COUNT5=`$GREP -c "$TAG2" "$OUT_LOCAL0"`
 
 # No debug message should enter with selector 'user.info'.
 COUNT2_debug=`$GREP -c "$TAG2.*user.debug" "$OUT_USER"`
@@ -523,21 +578,39 @@ COUNT2_debug=`$GREP -c "$TAG2.*user.debug" "$OUT_USER"`
 # No info message should enter with selector '*.=debug'.
 COUNT3_info=`$GREP -c "$TAG2.*user.info" "$OUT_DEBUG"`
 
+# No info or debug message should enter with selector 'user.=notice'.
+COUNT4_notice=`$GREP -c -e "$TAG2.*user.info" -e "$TAG2.*user.debug" \
+               "$OUT_UNOTICE"`
+# Undefined facility should overwrite also the priority.
+COUNT4_illegal=`$GREP -c "$TAG2.*Illegal facility" "$OUT_USER"`
+
+# No user message should enter with selector 'local0', and conversely.
+COUNT5_user=`$GREP -c "$TAG2.*user" "$OUT_LOCAL0"`
+COUNT5_local=`cat "$OUT_USER" "$OUT_UNOTICE" "$OUT_DEBUG" | \
+             $GREP -c "$TAG2.*local0"`
+
 SUCCESSES=`expr $SUCCESSES + $COUNT \
                + 2 \* $COUNT2 - $COUNT2_debug \
-               + 2 \* $COUNT3 - $COUNT3_info`
+               + 2 \* $COUNT3 - $COUNT3_info \
+               + 2 \* $COUNT4 - $COUNT4_notice - $COUNT4_illegal \
+               + 2 \* $COUNT5 - $COUNT5_user - $COUNT5_local`
 
 if [ -n "${VERBOSE+yes}" ]; then
     cat <<-EOT
        ---------- Successfully detected messages. ----------
        `$GREP "$TAG" "$OUT"`
-       `$GREP -h "$TAG2" "$OUT_USER" "$OUT_DEBUG"`
+       `$GREP -h "$TAG2" "$OUT_UNOTICE" "$OUT_USER" "$OUT_DEBUG" \
+                         "$OUT_LOCAL0"`
        ---------- Full message log for syslogd. ------------
        `cat "$OUT"`
-       ---------- User message log. ------------------------
+       ---------- User notice message log. -----------------
+       `cat "$OUT_UNOTICE"`
+       ---------- User info message log. -------------------
        `cat "$OUT_USER"`
        ---------- Debug message log. -----------------------
        `cat "$OUT_DEBUG"`
+       ---------- Local0 message log. ----------------------
+       `cat "$OUT_LOCAL0"`
        -----------------------------------------------------
        EOT
 fi

-----------------------------------------------------------------------

Summary of changes:
 ChangeLog        |   33 +++++++++++++++++
 src/logger.c     |   18 +++++----
 src/logprio.h    |    4 +-
 src/syslogd.c    |   19 +++++++---
 tests/syslogd.sh |  101 ++++++++++++++++++++++++++++++++++++++++++++++-------
 5 files changed, 145 insertions(+), 30 deletions(-)


hooks/post-receive
-- 
GNU Inetutils