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

[Zdenek Dohnal]

On 03/05/2017 10:40 AM, Olaf Meeuwissen wrote:
> Hi Zdenek,
>
> Zdenek Dohnal writes:
>
>> I tried to enhanced Olaf's patch and I posted it here:
>>
>> https://paste.fedoraproject.org/paste/qssgq4s0Vtqw6R5wkDWoEV5M1UNdIGYhyRLivL9gydE=
>>
>> Were my thoughts right and will it solve this issue?
> Thinking you just "backported" the patch (it applies with a fuzz but
> otherwise cleanly against 1.0.25) and removed the comments, I almost
> overlook your code change!
>
> I think it's my FIXME that misled you but you should *not* substract
> req.value_size.  Doing so is worse than what my code does because your
> code would substract too much, quite possibly making w->allocated_memory
> negative.  My code runs the risk of not substracting enough.
>
> In sanei/sanei_wire.c bytes are allocated to hold req.value based on a
> number provided by the network protocol.  This number is large enough to
> hold req.value plus terminating NUL and not larger than req.value_size.
>
> # In the original issue, req.value_size is 1024 and req.value = '\0'.
> # The code in sanei/sanei_wire.c allocates *1* byte.
>
> What the code in sanei/sanei_wire.c should do is allocate space for
> req.value_size bytes (it can't because where the allocation happens this
> information is not available).  My patch frees the incorrectly allocate
> memory and allocates a chunk that big enough.  It does that in saned.c
> to minimize its impact.
>
> # The sanei/sanei_wire.c code is used by saned *and* the net backend for
> # I/O in both directions.  To complicate matters, the code is meant to
> # transfer arrays and "abused" to transfer strings as if they are arrays
> # of characters.  My patch only affects saned's read logic.
> # A better patch would actually fix the issue(s) in sanei/sanei_wire.c.
>
> Doing this in saned.c though means that I no longer have access to the
> number provided by the network protocol.  I have to rely on the string
> length which may be less.  Hence my FIXME comment.
>
> # I was thinking about scenarios where backends might stuff a string in
> # a slightly larger buffer than strictly necessary and send the whole
> # buffer.
>
> Hope this clarifies a bit,
> --
> 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
Thank you so much for explanation, Olaf. I did not notice that fact
about req.value_size. So what about fetching string length from
sanei_w_array function by parameters sent by reference? Is it acceptable
to change number and type of parameters of functions? I created patch
proposal:

https://paste.fedoraproject.org/paste/KVJpdlIAMcxiovnYF4dhbV5M1UNdIGYhyRLivL9gydE=

It is probably not final version, but I hope I demonstrated my idea. It
was compiled without error.

-- 
Zdenek Dohnal
Associate Software Engineer
Brno, Purkyňova 99, Czech Republic
RED HAT | TRIED. TESTED. TRUSTED.

Every telecommunications Company in the Fortune Global 500 relies on Red Hat.

Find out why at Trusted | Red Hat


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: OpenPGP digital signature
URL: <http://lists.alioth.debian.org/pipermail/sane-devel/attachments/20170307/72f5ddc6/attachment.sig>