[sane-devel] saned bug: possible remote DoS (denial of service)

Henning Meier-Geinitz henning@meier-geinitz.de
Sun, 9 Feb 2003 17:47:44 +0100


--XMCwj5IQnwKtuyBG
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

Hi everyone,

Summary:
--------

The SANE network scanning dameon (saned) contains a combination of
bugs that allow a remote attacker to cause a segfault and/or consume
arbitrary amounts of memory. The attack is successful, even if the
attacker's computer isn't listed in saned.conf.


Vulnerable:
-----------

sane-backends-1.0.10 and all previous versions

You are only vulnerable if you actually run saned e.g. in xinetd or
inetd. If the entries in xinetd/inetd are commented out or do not
exist, you are safe.

Try "telnet localhost 6566" on the server that may run saned. If you
get "connection refused" saned is not running.


Counter messures:
----------------

If you run saned, apply the attached patch to sane-backends-1.0.10 or
upgrade to sane-backends-1.0.11 as soon as it will be available.

Generally, saned should be secured by using tcpwrappers and/or a
firewall setup. It's not intended to be used over the internet.
Setting limits for memory consumption is also a good idea.


Details:
--------

a) saned checks the identity (IP address) of the remote host only
   after the first communication took place (SANE_NET_INIT). So
   everyone can send that RPC, even if the remote host is not allowed
   to scan (not listed in saned.conf).
   
b) saned lacks error checking nearly everywhere in the code. So
   connection drops are detected very late. If the drop of the
   connection isn't detected, the access to the internal wire buffer
   leaves the limits of the allocated memory. So random memory "after"
   the wire buffer is read which will be followed by a segmentation
   fault.
   
c) If saned expects strings, it mallocs the memory necessary to store
   the complete string after it receives the size of the string. If
   the connection was dropped before transmitting the size, malloc
   will reserve an arbitrary size of memory. Depending on that size
   and the amount of memory available either malloc fails (->saned
   quits nicely) or a huge amount of memory is allocated. Swapping and
   and OOM measures may occur depending on the kernel.

d) saned doesn't check the validity of the RPC numbers it gets before
   getting the parameters.

e) If debug messages are enabled and a connection is dropped,
   non-null-terminated strings may be printed and segamentation faults
   may occur.

f) It's possible to alocate an arbitrary amount of memory on the
   server running saned even if the connection isn't dropped.


a)-e) are fixed by the patch. f) is not fixed because i think there is
no sensible fix IMHO. Better limit the total amount of memory saned
uses (ulimit).

I don't think that it's possible to run arbitrary commands because
there is no overflow of a buffer that's on the stack.

Thanks to Alexander Hvostov, who orignally found the segfault and
reported it to the Debian bugtracking system and to the Debian SANE
maintainers Julien BLACHE and Aurelien Jarno who helped to find out
what's the actual problem in saned.
   
Bye,
  Henning

--XMCwj5IQnwKtuyBG
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="sane-backends-1.0.10-1.0.11.diff"

diff -u -r -x '*.po' sane-backends-1.0.10/ChangeLog sane-backends-1.0.11/ChangeLog
--- sane-backends-1.0.10/ChangeLog	2003-02-01 11:56:04.000000000 +0100
+++ sane-backends-1.0.11/ChangeLog	2003-02-09 14:59:58.000000000 +0100
@@ -1,3 +1,13 @@
+******  Release of sane-backends 1.0.11 ******
+
+2003-02-09  Henning Meier-Geinitz <henning@meier-geinitz.de>
+
+	* frontend/saned.c sanei/sanei_codec_bin.c sanei/sanei_wire.c: Check
+	  the IP address of the remote host before any communication occurs.
+	  Check for a errors before trsuting values that came from remote.
+	  Make sure that strings are 0-terminated.
+	* configure configure.in: New version: 1.0.11.
+
 ******  Release of sane-backends 1.0.10. End of code freeze ******
 
 2003-02-01  Henning Meier-Geinitz <henning@meier-geinitz.de>
diff -u -r -x '*.po' sane-backends-1.0.10/configure sane-backends-1.0.11/configure
--- sane-backends-1.0.10/configure	2003-01-31 20:40:54.000000000 +0100
+++ sane-backends-1.0.11/configure	2003-02-09 14:10:46.000000000 +0100
@@ -1,6 +1,6 @@
 #! /bin/sh
 # Guess values for system-dependent variables and create Makefiles.
-# Generated by GNU Autoconf 2.57 for sane-backends 1.0.10.
+# Generated by GNU Autoconf 2.57 for sane-backends 1.0.11.
 #
 # Report bugs to <sane-devel@mostang.com>.
 #
@@ -427,8 +427,8 @@
 # Identity of this package.
 PACKAGE_NAME='sane-backends'
 PACKAGE_TARNAME='sane-backends'
-PACKAGE_VERSION='1.0.10'
-PACKAGE_STRING='sane-backends 1.0.10'
+PACKAGE_VERSION='1.0.11'
+PACKAGE_STRING='sane-backends 1.0.11'
 PACKAGE_BUGREPORT='sane-devel@mostang.com'
 
 # Factoring default headers for most tests.
@@ -937,7 +937,7 @@
   # Omit some internal or obsolete options to make the list less imposing.
   # This message is too long to be a string in the A/UX 3.1 sh.
   cat <<_ACEOF
-\`configure' configures sane-backends 1.0.10 to adapt to many kinds of systems.
+\`configure' configures sane-backends 1.0.11 to adapt to many kinds of systems.
 
 Usage: $0 [OPTION]... [VAR=VALUE]...
 
@@ -1003,7 +1003,7 @@
 
 if test -n "$ac_init_help"; then
   case $ac_init_help in
-     short | recursive ) echo "Configuration of sane-backends 1.0.10:";;
+     short | recursive ) echo "Configuration of sane-backends 1.0.11:";;
    esac
   cat <<\_ACEOF
 
@@ -1111,7 +1111,7 @@
 test -n "$ac_init_help" && exit 0
 if $ac_init_version; then
   cat <<\_ACEOF
-sane-backends configure 1.0.10
+sane-backends configure 1.0.11
 generated by GNU Autoconf 2.57
 
 Copyright 1992, 1993, 1994, 1995, 1996, 1998, 1999, 2000, 2001, 2002
@@ -1126,7 +1126,7 @@
 This file contains any messages produced by compilers while
 running configure, to aid debugging if configure makes a mistake.
 
-It was created by sane-backends $as_me 1.0.10, which was
+It was created by sane-backends $as_me 1.0.11, which was
 generated by GNU Autoconf 2.57.  Invocation command line was
 
   $ $0 $@
@@ -1464,7 +1464,7 @@
 
 V_MAJOR=1
 V_MINOR=0
-V_REV=10
+V_REV=11
 is_release=yes
 
           ac_config_headers="$ac_config_headers include/sane/config.h"
@@ -1484,7 +1484,7 @@
 
 
 PACKAGE=sane-backends
-VERSION=1.0.10
+VERSION=1.0.11
 NUMBER_VERSION=${V_MAJOR}.${V_MINOR}.${V_REV}
 
 cat >>confdefs.h <<_ACEOF
@@ -13224,7 +13224,7 @@
 } >&5
 cat >&5 <<_CSEOF
 
-This file was extended by sane-backends $as_me 1.0.10, which was
+This file was extended by sane-backends $as_me 1.0.11, which was
 generated by GNU Autoconf 2.57.  Invocation command line was
 
   CONFIG_FILES    = $CONFIG_FILES
@@ -13284,7 +13284,7 @@
 
 cat >>$CONFIG_STATUS <<_ACEOF
 ac_cs_version="\\
-sane-backends config.status 1.0.10
+sane-backends config.status 1.0.11
 configured by $0, generated by GNU Autoconf 2.57,
   with options \\"`echo "$ac_configure_args" | sed 's/[\\""\`\$]/\\\\&/g'`\\"
 
diff -u -r -x '*.po' sane-backends-1.0.10/configure.in sane-backends-1.0.11/configure.in
--- sane-backends-1.0.10/configure.in	2003-01-31 20:40:44.000000000 +0100
+++ sane-backends-1.0.11/configure.in	2003-02-09 14:10:33.000000000 +0100
@@ -3,10 +3,10 @@
 dnl ***********************************************************************
 dnl When preparing a release, increase the numeric and string version numbers,
 dnl remove the "-cvs" suffix, and set is_release=yes
-AC_INIT([sane-backends],[1.0.10],[sane-devel@mostang.com])
+AC_INIT([sane-backends],[1.0.11],[sane-devel@mostang.com])
 V_MAJOR=1
 V_MINOR=0
-V_REV=10
+V_REV=11
 is_release=yes
 dnl ***********************************************************************
 
diff -u -r -x '*.po' sane-backends-1.0.10/frontend/saned.c sane-backends-1.0.11/frontend/saned.c
--- sane-backends-1.0.10/frontend/saned.c	2003-01-29 18:38:19.000000000 +0100
+++ sane-backends-1.0.11/frontend/saned.c	2003-02-09 14:08:45.000000000 +0100
@@ -183,6 +183,12 @@
       return;
     }
 
+  if (wire.status)
+    {
+      DBG(DBG_ERR, "auth_callback: bad status %d\n", wire.status);
+      return;
+    }
+
   switch (current_request)
     {
     case SANE_NET_OPEN:
@@ -222,10 +228,24 @@
 	   current_request, res);
       break;
     }
+
+  if (wire.status)
+    {
+      DBG(DBG_ERR, "auth_callback: bad status %d\n", wire.status);
+      return;
+    }
+
   reset_watchdog ();
 
   sanei_w_set_dir (&wire, WIRE_DECODE);
   sanei_w_word (&wire, &word);
+
+  if (wire.status)
+    {
+      DBG(DBG_ERR, "auth_callback: bad status %d\n", wire.status);
+      return;
+    }
+
   procnum = word;
   if (procnum != SANE_NET_AUTHORIZE)
     {
@@ -237,6 +257,12 @@
     }
 
   sanei_w_authorization_req (&wire, &req);
+  if (wire.status)
+    {
+      DBG(DBG_ERR, "auth_callback: bad status %d\n", wire.status);
+      return;
+    }
+
   if (req.username)
     strcpy (username, req.username);
   if (req.password)
@@ -514,30 +540,51 @@
 
   reset_watchdog ();
 
+  status = check_host (w->io.fd);
+  if (status != SANE_STATUS_GOOD)
+    {
+      DBG (DBG_WARN, "init: access by host %s denied\n", remote_hostname);
+      return -1;
+    }
+
   sanei_w_set_dir (w, WIRE_DECODE);
+  if (w->status)
+    {
+      DBG (DBG_ERR, "init: bad status after sanei_w_set_dir: %d\n", w->status);
+      return -1;
+    }
+  
   sanei_w_word (w, &word);	/* decode procedure number */
-  sanei_w_init_req (w, &req);
-  w->version = SANEI_NET_PROTOCOL_VERSION;
-
   if (w->status || word != SANE_NET_INIT)
     {
       DBG (DBG_ERR, "init: bad status=%d or procnum=%d\n",
 	   w->status, word);
       return -1;
     }
+
+  sanei_w_init_req (w, &req);
+  if (w->status)
+    {
+      DBG (DBG_ERR, "init: bad status after sanei_w_init_req: %d\n", w->status);
+      return -1;
+    }
+
+  w->version = SANEI_NET_PROTOCOL_VERSION;
   if (req.username)
     default_username = strdup (req.username);
 
   sanei_w_free (w, (WireCodecFunc) sanei_w_init_req, &req);
+  if (w->status)
+    {
+      DBG (DBG_ERR, "init: bad status after sanei_w_free: %d\n", w->status);
+      return -1;
+    }
 
   reply.version_code = SANE_VERSION_CODE (V_MAJOR, V_MINOR,
 					  SANEI_NET_PROTOCOL_VERSION);
 
-  status = check_host (w->io.fd);
-
-  DBG (DBG_WARN, "init: access by %s@%s %s\n",
-       default_username, remote_hostname,
-       (status == SANE_STATUS_GOOD) ? "accepted" : "rejected");
+  DBG (DBG_WARN, "init: access by %s@%s accepted\n",
+       default_username, remote_hostname);
 
   if (status == SANE_STATUS_GOOD)
     {
@@ -823,6 +870,14 @@
   DBG (DBG_DBG, "process_request: waiting for request\n");
   sanei_w_set_dir (w, WIRE_DECODE);
   sanei_w_word (w, &word);	/* decode procedure number */
+
+  if (w->status)
+    {
+      DBG (DBG_ERR,
+	   "process_request: bad status %d\n", w->status);
+      quit (0);
+    }
+
   current_request = word;
 
   DBG (DBG_MSG, "process_request: got request %d\n", current_request);
diff -u -r -x '*.po' sane-backends-1.0.10/sanei/sanei_codec_bin.c sane-backends-1.0.11/sanei/sanei_codec_bin.c
--- sane-backends-1.0.10/sanei/sanei_codec_bin.c	2002-01-14 22:44:36.000000000 +0100
+++ sane-backends-1.0.11/sanei/sanei_codec_bin.c	2003-02-09 14:09:01.000000000 +0100
@@ -54,6 +54,9 @@
   SANE_Byte *b = v;
 
   sanei_w_space (w, 1);
+  if (w->status)
+    return;
+
   switch (w->direction)
     {
     case WIRE_ENCODE:
@@ -82,8 +85,14 @@
 	len = strlen (*s) + 1;
     }
   sanei_w_array (w, &len, v, w->codec.w_byte, 1);
-  if (w->direction == WIRE_DECODE && !len)
-    *s = 0;
+
+  if (w->direction == WIRE_DECODE)
+    {
+      if (len == 0)
+	*s = 0;
+      else if (w->status == 0)
+	*(*s + len - 1) = '\0';
+    }
 }
 
 static void
@@ -92,6 +101,8 @@
   SANE_Word val, *word = v;
 
   sanei_w_space (w, 4);
+  if (w->status)
+    return;
   switch (w->direction)
     {
     case WIRE_ENCODE:
diff -u -r -x '*.po' sane-backends-1.0.10/sanei/sanei_wire.c sane-backends-1.0.11/sanei/sanei_wire.c
--- sane-backends-1.0.10/sanei/sanei_wire.c	2002-03-19 21:28:06.000000000 +0100
+++ sane-backends-1.0.11/sanei/sanei_wire.c	2003-02-09 14:08:55.000000000 +0100
@@ -192,8 +192,14 @@
     len = *len_ptr;
   DBG (4, "sanei_w_array: send/receive array length\n");
   sanei_w_word (w, &len);
-  DBG (4, "sanei_w_array: array has %d elements\n", len);
 
+  if (w->status)
+    {
+      DBG (1, "sanei_w_array: bad status: %d\n", w->status);
+      return;
+    }
+  DBG (4, "sanei_w_array: array has %d elements\n", len);
+      
   if (w->direction == WIRE_DECODE)
     {
       *len_ptr = len;
@@ -219,6 +225,11 @@
     {
       (*w_element) (w, val);
       val += element_size;
+      if (w->status)
+	{
+	  DBG (1, "sanei_w_array: bad status: %d\n", w->status);
+	  return;
+	}
     }
   DBG (4, "sanei_w_array: done\n");
 }
@@ -251,6 +262,11 @@
 
   DBG (4, "sanei_w_ptr: send/receive is_null\n");
   sanei_w_word (w, &is_null);
+  if (w->status)
+    {
+      DBG (1, "sanei_w_ptr: bad status: %d\n", w->status);
+      return;
+    }
 
   if (!is_null)
     {
@@ -307,7 +323,7 @@
 {
   DBG (3, "sanei_w_string: wire %d\n", w->io.fd);
   (*w->codec.w_string) (w, v);
-  if (w->direction != WIRE_FREE)
+  if (w->direction != WIRE_FREE && w->status == 0)
     DBG (4, "sanei_w_string: value = %s\n", *v);
 }
 

--XMCwj5IQnwKtuyBG--