Page MenuHomeFreeBSD

Fix memory disclosures from ptrace(PT_GET*REGS).
ClosedPublic

Authored by markj on Mon, Dec 3, 4:29 PM.

Details

Summary

Zero the structures before calling the machdep routines that fill them
out. For good measure, ensure that those routines zero unused fields:
they might end up getting used in some other sensitive context (e.g.,
Linux ptrace emulation).

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

markj created this revision.Mon, Dec 3, 4:29 PM
markj added reviewers: jhb, kib.Mon, Dec 3, 4:33 PM
kib added inline comments.Mon, Dec 3, 5:18 PM
sys/kern/sys_process.c
597 ↗(On Diff #51533)

So you zero-out struct reg before calling into MD code. Then why would we need to explicitly set r_err and r_trapno ? Might be for procfs, but isn't it better to clear at top-level in procfs too ?

getcontext() seems to be immune to trapno/err leak for the same reason, top-level code clears the ucontext_t at start, and same is done for sigcontext.

markj marked an inline comment as done.Mon, Dec 3, 5:27 PM
markj added inline comments.
sys/kern/sys_process.c
597 ↗(On Diff #51533)

I tried to explain the motivation in the description: I wanted to try and protect other (future) consumers of the interface. I did not catch procfs, for instance. I agree that the explicit zeroing is preferable there, though, and will fix it too. If you still prefer to limit the zeroing to sys_ptrace() itself and procfs, I don't insist on modifying the machdep code.

markj updated this revision to Diff 51538.Mon, Dec 3, 5:40 PM
  • Zero out reg structs in procfs too.
markj updated this revision to Diff 51539.Mon, Dec 3, 5:45 PM

Explicitly bzero 32bit compat register structures in sys_ptrace().

kib accepted this revision.Mon, Dec 3, 5:52 PM
This revision is now accepted and ready to land.Mon, Dec 3, 5:52 PM
This revision was automatically updated to reflect the committed changes.