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
>From 46bab8cea5000b363ca4eb360e635285feb14ac7 Mon Sep 17 00:00:00 2001
From: Olaf Meeuwissen <paddy-h...@member.fsf.org>
Date: Sun, 19 Feb 2017 16:45:45 +0900
Subject: [PATCH] Address memory corruption and information leakage.
---
frontend/saned.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/frontend/saned.c b/frontend/saned.c
index 0aba1755..8c4fa13a 100644
--- a/frontend/saned.c
+++ b/frontend/saned.c
@@ -1992,6 +1992,32 @@ process_request (Wire * w)
return 1;
}
+ /* Debian BTS #853804 */
+ if (w->direction == WIRE_DECODE
+ && req.value_type == SANE_TYPE_STRING
+ && req.action == SANE_ACTION_GET_VALUE)
+ {
+ if (req.value)
+ {
+ /* FIXME: If req.value contained embedded NUL
+ * characters, this is wrong.
+ */
+ w->allocated_memory -= (1 + strlen (req.value));
+ free (req.value);
+ }
+ req.value = malloc (req.value_size);
+ if (!req.value)
+ {
+ w->status = ENOMEM;
+ DBG (DBG_ERR,
+ "process_request: (control_option) "
+ "h=%d (%s)\n", req.handle, strerror (w->status));
+ return 1;
+ }
+ memset (req.value, 0, req.value_size);
+ w->allocated_memory += req.value_size;
+ }
+
can_authorize = 1;
memset (&reply, 0, sizeof (reply)); /* avoid leaking bits */
--
2.11.0
--
sane-devel mailing list: sane-devel@lists.alioth.debian.org
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/sane-devel
Unsubscribe: Send mail with subject "unsubscribe your_password"
to sane-devel-requ...@lists.alioth.debian.org
