[sane-devel] Bug#854804: saned: SANE_NET_CONTROL_OPTION response packet may contain memory contents of the server

[Olaf Meeuwissen]

Hi Kritphong,

Kritphong Mongkhonvanit writes:

> On 02/14/2017 09:04 PM, Olaf Meeuwissen wrote:
>> Could you run
>>
>>   SANE_DEBUG_SANEI_WIRE=128 saned -d128 2> saned.log
>>
>> reproduce and provide the saned.log (compressed if big)?
> The requested log is attached.

Thanks!!

I didn't write the code but, if my analysis is correct, it is actually
worse than sending server memory content over the wire.  It looks like
saned is clobbering memory, i.e. it's writing past the end of allocated
memory, as well.

According to your log (at line 4007), the saned process gets its first
SANE_NET_CONTROL_OPTION request.  That request tries to fetch the value
of the 8th option (compression) which is a string value that can be up
to 1024 (0x400) bytes long.  The request also sends a value with this
request, a NUL-terminated 1-byte long empty string.

# Code line references against f450049b.

At this point we are around line 4045 of the log.  Now let's switch to
the code.  The incoming request is handled in the case statement on line
1979 of frontend/saned.c.  The sanei_w_control_option_req() call has
taken care of the incoming request and the req structure now contains

  req.handle = 0;
  req.option = 8;      // 'compression'
  req.action = 0;      // SANE_ACTION_GET_VALUE
  req.value_type = 3;  // SANE_TYPE_STRING
  req.value_size = 1024;
  req.value      = "\0";

Most importantly, req.value was allocated as a *1*-byte buffer.  This
happens in the if-block starting at line 204 in sanei/sanei_wire.c.
Note that the `len` is passed back up via `len_ptr` but that that value
does *not* make it back to req.value_size because the w_option_value()
call in sanei_w_control_option_req() passes by value, not by reference.

This means that sane_control_option() on line 1999 in frontend/saned.c
happily passes a 1-byte buffer to the backend.  The backend assumes that
it can store up to 1024 bytes in that buffer and writes a NUL-terminated
five byte "JPEG" string into the 1-byte buffer.  Oops!

On line 2003 of frontend/saned.c the reply.value_size is set to the
value fo req.value_size (still 1024) and sanei_w_reply gets a reply
struct that:

 - has a pointer to a 1-byte block of memory
 - which holds a five byte string value
 - that is sent back as a 1024 buffer

Ouch!

This code has been around since the summer of 1999.  Seeing that we have
not had anyone complain about this before, please check my analysis with
care.  I have only "eyeballed" the code.  I have not tried to reproduce
or run things in a debugger or anything.

Attached is a minimal hack/patch that *tries* to fix it.  I have only
checked that it compiles.  Could you take a look at whether it fixes
the issue and does not break saned?

Hope this helps,
--
Olaf Meeuwissen, LPIC-2            FSF Associate Member since 2004-01-27
 GnuPG key: F84A2DD9/B3C0 2F47 EA19 64F4 9F13  F43E B8A4 A88A F84A 2DD9
 Support Free Software                        https://my.fsf.org/donate
 Join the Free Software Foundation              https://my.fsf.org/join
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Address-memory-corruption-and-information-leakage.patch
Type: text/x-diff
Size: 1572 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/sane-devel/attachments/20170219/87ddbb35/attachment.patch>