Bug 99626

Created attachment 129272 [details] [review]
Disallow multiple specification of parameters to avoid memory leaking.

FYI, patch vs. 0.112.

Thanks for the patch.

Note that pkcheck is not set-uid, so there is no privilege escalation; this only allows the user to waste memory under their own account, which they are perfectly capable of doing, and allowed to do, in many other ways.

pkcheck might not be setuid but it's still a potent attack vector that should be and easily is eliminated. I mean downstream also.

Quite a way to defend a blatant memory leak by the way ;-D

Oh I am not saying that this code is perfect, something like this patch, or at the very least freeing the old values, should indeed be applied upstream.

I _am_ saying that I can’t see why this should be a vulnerability, _and_ providing my rationale so that any mistake can be pointed out.

> pkcheck might not be setuid but it's still a potent attack vector

An attack on what? There is no privilege escalation, so there pretty much by definition is no attack.

Anyone able to run pkcheck with a hundred parameters is able to run something like
> while :; do a=a$a$a; done
in bash, or consume memory in many other ways in processes running under their UID. What makes pkcheck different? What is being attacked, and what is the attack mechanism?

Also, this is not a long-term memory leak where a single process running over hours accumulates unfreed memory during perfectly normal operation, and then crashes because of that = fails to serve its primary purpose.

pkcheck allocates all that memory, performs a single quick operation, and immediately quits, freeing the memory again. And this can only happen when the user intentionally causes it to allocate memory, so arguably allocating memory is, ridiculous as it may sound, the primary purpose of the operation (unlike a memory leak in a long-running process, which is almost always wanted by noone).

These memory leaks allow "heap spraying". Heap spraying is an attack vector that gives an adversary a serious edge, because they can essentially initialize large parts of the heap. This makes it easy for a local attacker to leverage an attack.

The author clearly states that in his example exploit he gives himself a break, takes the path of least resistance if you will, by working on a 32 bit system instead of a 64 system (smaller address space), and choosing a more easily exploitable binary so he does not have to add a privilege escalation.

However, the heap spraying is in itself a very potent local attack vector (bad boy with a shell on your system can use the exploit to "work the heap" before launching his actual attack).

The fact that the binary in the example is setuid is orthogonal to the fact that heap spraying is a very serious attack vector.

So even though pkcheck is not setuid, these memory leaks in the option parsing give a local attacker a big tool to try to attempt a privilege escalation.


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

These are very serious bugs so if my arguments are not convincing please consult one of your security advisors.

Or for that matter, mail the author of that post. I believe he will agree with me.

> The fact that the binary in the example is setuid is orthogonal to the fact that heap spraying is a very serious attack vector.

Again, what is being attacked?

(And heap spraying is not an attack vector; it is at best a technique to exploit _another_ vulnerability. Without that iconv off-by-one heap spraying pkexec would have been perfectly harmless.)

> So even though pkcheck is not setuid, these memory leaks in the option parsing give a local attacker a big tool to try to attempt a privilege escalation.

The point of the memory manipulation is that there is a process with _escalated privileges_ process which can be manipulated to corrupt its memory and then do what the attacker wants it to. That allows the attacker to perform an operation with those escalated privileges = escalate the attacker’s privileges.

When the target does not have escalated privileges (as is the case with pkcheck), manipulating the target’s memory does not give the user anything they don’t already have, in particular:

> bad boy with a shell on your system can use the exploit to "work the heap" before launching his actual attack

bady boy with a shell on my system can use vim+gcc, or perl, to craft a process with any memory layout they desire. pkcheck does not make _anything_ easier for them, it does not give them _any_ extra privileges; if anything, memory manipulation via pkcheck is way more difficult than any of the direct methods.

---

An attacker able to run an arbitrary command line is already capable to create a process with an arbitrary memory layout; there is no privilege escalation.

An attacker able to exhaust memory/swap in pkcheck is already capable to exhaust memory/swap with any other tool, including pure shell; there is no privilege escalation.

When there is no privilege escalation, there is no vulnerability.

-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/polkit/polkit/issues/4.