[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 17 Aug 2018 05:43:58 -0400 (EDT)
From: Frediano Ziglio <fziglio@...hat.com>
To: Florian Weimer <fweimer@...hat.com>
Cc: Doran Moppert <dmoppert@...hat.com>, oss-security@...ts.openwall.com,
Christophe Fergeau <cfergeau@...hat.com>
Subject: Re: spice CVE-2018-10873: post-auth crash or
potential heap corruption when demarshalling
> On 08/17/2018 02:51 AM, Doran Moppert wrote:
> > + if (SPICE_UNLIKELY((start + 2) > message_end)) {
> > + goto error;
> > + }
>
> These checks are still technically invalid because start + 2 is not a
> valid pointer if it points past the allocated object.
>
Technical but not real. Unless it wraps is correct. To wrap the memory
layout would have to have the kernel before the userspace and the
memory buffer at the very end of memory. Neither Linux nor Windows which
we support have these characteristics.
> This is more problematic here:
>
> > + if (SPICE_UNLIKELY((start2 + 2 + cursor_u__nw_size) >
> > message_end)) {
> > + goto error;
> > + }
>
> If cursor_u__nw_size results in pointer wraparound, the check might fail
> incorrectly.
>
cursor_u__nw_size is either 0 or 17, very hard to wrap, impossible in currently
supported systems as stated earlier.
> The commit message quotes the right pattern, nw_size > (uintptr_t)
> (message_end - start), but it is not used in the actual code AFAICS.
>
Yes, check is not perfect and can be modified to support any possible
technical systems. The check uses already present helpers, I think for the
fix is better to stick to the current code, I'll see if I can support
all cases as an improve.
> Thanks,
> Florian
>
Thanks,
Frediano
Powered by blists - more mailing lists
Please check out the
Open Source Software Security Wiki, which is counterpart to this
mailing list.
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
