820573 - Security audit of breakpad wrt bug 820560

Status: RESOLVED WORKSFORME

(Core :: General, defect)

Description

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

This is the problem mentioned in bug 820560.  The gecko fix

>  - fix in gecko.  We need to inherited privileges past fork+exec,
> prctl(SET_DUMPABLE, 0), then drop rights.

works except that the breakpad handler prctl(SET_DUMPABLE, 1) in order for the crash generation server to create a minidump.  This creates the following *THEORETICAL* attack

 1. Compromised app Evil sends fatal signal to app Victim
 2. Victim's signal handler sets prctl(SET_DUMPABLE, 1) in preparation of the parent creating a minidump
 3. Evil ptrace(ATTACH)s to Victim, winning the race with the parent process

The kernel fix is not vulnerable to this attack because it will block the ptrace attempt in (3).

The question is whether (1) and (2) will allow Evil to ptrace(ATTACH) to Victim.

According to my reading of the breakpad client, there's a check that will prevent this

358   // Allow ourselves to be dumped if the signal is trusted.
359   bool signal_trusted = info->si_code > 0;
360   bool signal_pid_trusted = info->si_code == SI_USER ||
361       info->si_code == SI_TKILL;
362   if (signal_trusted || (signal_pid_trusted && info->si_pid == getpid())) {
363     sys_prctl(PR_SET_DUMPABLE, 1);
364   }

In this case, Evil would send the signal, so si_code would be SI_USER (which is 0).  So signal_trusted = false.  However, the pid of Evil would be different than Victim, so |info->si_pid == getpid()| would fail and we would not allow ptrace.

If everyone agrees with this analysis, we can close this out.

Comment 1

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

We have a fix in hand that's not vulnerable to this, and a big hammer in the bush.