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
_______________________________________________
CentOS mailing list
CentOS@xxxxxxxxxx
https://lists.centos.org/mailman/listinfo/centos
