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