Upstream fixed this by not allowing multiple opt_user specifications at the same location where I inserted the counter test.

      else if (strcmp (argv[n], "--user") == 0 || strcmp (argv[n], "-u") == 0)
        {
          n++;
          if (n >= (guint) argc)
            {
              usage (argc, argv);
              goto out;
            }

          if (opt_user != NULL)
            {
              g_printerr ("--user specified twice\n");
              goto out;
            }
          opt_user = g_strdup (argv[n]);
        }

Similar issues with pkcheck.c, you might want to fix those too!

https://bugs.freedesktop.org/show_bug.cgi?id=99626

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

Updated patch to fix both pkcheck.c and pkexec.c.

Thanks for your report.

There is nothing inherently exploitable about the attacker being able to allocate strings in memory _in pkexec_; the described vulnerability was in glibc, this was only a possible way to take advantage of it. That glibc vulnerability has been fixed in RHEL 7, see https://rhn.redhat.com/errata/RHSA-2015-0327.html .

Because these patches cause previously working commands to be rejected, they have a risk of breaking customers’ (admittedly misguided) scripts. On balance, I believe making the change is not worth this risk.

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.

The fact that the attack surface on pkcheck is small 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.

I dispute your WONTFIX.

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

No, it is essential; exploiting pkcheck does not give the attacker any privileges they don’t already have. pkcheck pretty much by definition cannot be vulnerable to the user running it because they run with exactly the same privilege [pkcheck _could_ be vulnerable to other users’ actions, in principle]. If there can’t be a vulnerability, there can’t be an attack vector either.

By the way, even though you might claim pkcheck might not be vulnerable,

pkexec *is* vulnerable as the article shows.

You should at least fix pkexec!