CentOS - Feb 2017 - Serious attack vector on pkcheck ignored by Red Hat

On Thu, 2017-02-02 at 07:16 -0800, Gordon Messmer wrote:
> On 02/02/2017 06:51 AM, Leonard den Ottolander wrote:
> > pkcheck might not be directly vulnerable. However, pkexec is.
> 
> 
> If that's so, why are you supplying patches to pkcheck rather than 
> fixing pkexec?
The patch has a fix for three memory leaks. One memory leak that allows
heap spraying in pkexec.c that according to the aforementioned article
is *directly* exploitable and has been fixed upstream. (Check references
I provided.)

Two similar memory leaks exist in pkcheck.c, for which I also provided
patches. Even though these might not be so easily exploitable the memory
leaks in themselves allow a local attacker to "spray the heap", which
makes it easier for him to leverage an attack. You do not want to allow
an attacker to have such potent tools readily available.

Memory leaks are always bad, but these are seriously bad because they
are attacker controlled.

Regards,
Leonard.

-- 
mount -t life -o ro /dev/dna /genetic/research

On 02/02/2017 07:35 AM, Leonard den Ottolander wrote:
>> If that's so, why are you supplying patches to pkcheck rather than
>> fixing pkexec?
> The patch has a fix for three memory leaks. One memory leak that allows
> heap spraying in pkexec.c that according to the aforementioned article
> is*directly*  exploitable and has been fixed upstream.


It took me a while to find the patch that you mentioned, which is 
probably why your bugs are being disregarded.  Entirely too much of your 
existing bug reports is spent discussing a non-issue.

If you want this issue to be taken seriously, I have a couple of 
pointers:  First, drop the bug reports that have been closed. Those 
tickets are now convoluted and clouded by misguided discussion of a bug 
in pkcheck.c, which isn't expoitable. Continued arguing in those bug 
reports will be counter-productive.

Open a new bug report and focus on this patch, exclusively:
https://cgit.freedesktop.org/polkit/commit/src/programs/pkexec.c?id=6c992bc8aefa195a41eaa41c07f46f17de18e25c

The upstream developer has disallowed multiple --user specifications in 
order to close a memory leak.  That memory leak can be used to cause the 
heap and the stack to run in to each other, and that flaw has previously 
been combined with bugs in glibc to produce an exploit.  The glibc bug 
is now fixed, but there is still a risk that collision could be 
exploitable in combination with other, as yet undiscovered bugs.  If Red 
Hat is concerned with changing the behavior of pkexec in scripts, then 
they can still fix the memory leak without otherwise changing the 
behavior of the program by adding:

if (opt_user != NULL)
   {
     g_free(opt_user);
   }

..instead of the upstream solution of failing on multiple --user 
specifications.  This will correct the leak and won't break any scripts 
that call --user multiple times.

That's it.  Keep your bug report simple.  Focus on the program that 
presents a security vulnerability due to being SUID root. Offer a 
solution that doesn't break any existing user applications.  Since the 
problem has been fixed upstream already, you don't need any bug reports 
with freedesktop.org, just with Red Hat for the polkit-112 package.

On Thu, 2017-02-02 at 10:39 -0800, Gordon Messmer wrote:
> It took me a while to find the patch that you mentioned, which is 
> probably why your bugs are being disregarded.
It is beyond my control where patches are listed in the Red Hat bugzilla
pages. I don't think the Red Hat employee involved should have a hard
time finding it in my report.
> Open a new bug report and focus on this patch, exclusively:
>
https://cgit.freedesktop.org/polkit/commit/src/programs/pkexec.c?id=6c992bc8aefa195a41eaa41c07f46f17de18e25c
> 
> The upstream developer has disallowed multiple --user specifications in 
> order to close a memory leak.
Yes. This seems to be a better solution than the one I came up with
initially and which you mention below, because multiple invocations of
--user are meaningless in this context.
> That memory leak can be used to cause the 
> heap and the stack to run in to each other, and that flaw has previously 
> been combined with bugs in glibc to produce an exploit.  The glibc bug 
> is now fixed, but there is still a risk that collision could be 
> exploitable in combination with other, as yet undiscovered bugs.
Yes. I understand perfectly well how this works, which is why I am so
concerned. And that is exactly why I also want to fix those memory leaks
in pkcheck.c. There might not be a known exploit for pkcheck.c but the
vector ("heap spraying" because of a memory leak) is the same. That
"heap spraying" will make it easier for an attacker to leverage any
attack including a privilege escalation so it is worrisome even if the
binary itself is not setuid.
> If Red 
> Hat is concerned with changing the behavior of pkexec in scripts, then 
> they can still fix the memory leak without otherwise changing the 
> behavior of the program by adding:
> 
> if (opt_user != NULL)
>    {
>      g_free(opt_user);
>    }
That is the initial fix I proposed, but I changed it to use the upstream
fix of not allowing multiple invocations of --user. Multiple invocations
of --user are pointless in this context, so I believe the upstream fix
is just fine. And probably *more* acceptable for "midstream", i.e. Red
Hat. But either will do.

I applied a similar fix to pkcheck.c, because the memory leaks are
identical to the one in pkexec.c, so even though not quite as
exploitable that (very powerful) vector is in that binary too.

The argument that the binary must be setuid to make this worrisome is
bogus. The vector might enable a privilege escalation because (at least
on a 32-bit system) a shell user is able to initialize the entire heap
with data of his liking making it way easier to mount any attack.
> ..instead of the upstream solution of failing on multiple --user 
> specifications.  This will correct the leak and won't break any scripts
> that call --user multiple times.
Scripts shouldn't call --user multiple times. When using the fix with
g_free() only the last specification will be used.

I think proposing a fix different from the one applied upstream (as in
freedesktop.org) might also be considered "convoluting the issue".
> That's it.  Keep your bug report simple.  Focus on the program that 
> presents a security vulnerability due to being SUID root. Offer a 
> solution that doesn't break any existing user applications.  Since the 
> problem has been fixed upstream already, you don't need any bug reports
> with freedesktop.org, just with Red Hat for the polkit-112 package.
The fixes I provided for upstream are for pkcheck.c only (because
pkexec.c has been fixed there). Again, even though not directly
(knowingly) exploitable allowing a shell user to heap spray *any* binary
is bad. Just the fact that I have to argue that fact so extensively is
troubling. Any memory leak should be fixed. A memory leak that allows a
(local) user to entirely control the contents of the heap should be
fixed immediately.

I have no indications the fixes I supplied for pkcheck.c will not be
applied upstream.

Regards,
Leonard.

-- 
mount -t life -o ro /dev/dna /genetic/research

On Thu, 2017-02-02 at 10:39 -0800, Gordon Messmer wrote:
> Open a new bug report and focus on this patch, exclusively:
>
https://cgit.freedesktop.org/polkit/commit/src/programs/pkexec.c?id=6c992bc8aefa195a41eaa41c07f46f17de18e25c
By the way, the comment for that commit starts with:

This usage is clearly errorneous, so we should tell the users they are
making a mistake. Besides, this allows an attacker to cause a high
number of heap allocations with attacker-controlled sizes
(
http://googleprojectzero.blogspot.cz/2014/08/the-poisoned-nul-byte-2014-edition.html
), making some exploits easier.

Regards,
Leonard.

-- 
mount -t life -o ro /dev/dna /genetic/research