Skip to content

fix m_Extensions in ProcTerminated to avoid invalid log data issues#239

Open
ipduffy wants to merge 1 commit into
microsoft:mainfrom
ipduffy:fix-processterminate-sid-extension-zeroing
Open

fix m_Extensions in ProcTerminated to avoid invalid log data issues#239
ipduffy wants to merge 1 commit into
microsoft:mainfrom
ipduffy:fix-processterminate-sid-extension-zeroing

Conversation

@ipduffy

@ipduffy ipduffy commented Jul 22, 2026

Copy link
Copy Markdown

Summary

ProcTerminated() in ebpfKern/sysmonProcTerminated.c was the one eBPF
event handler that never zero-initialized event->m_Extensions before
conditionally populating the SID extension length. Every other handler
that emits a SID extension does this:

  • sysmonFileCreate.c
  • sysmonFileDelete.c
  • sysmonFileDeleteAt.c
  • sysmonFileDeleteAtCwd.c
  • sysmonFileOpen.c
  • sysmonProcAccessed.c
  • sysmonProcCreate.c
  • sysmonBPFLoad.c

Root cause

eventHdr is fetched from eventStorageMap, a per-CPU scratch buffer
reused across every eBPF event type on that core (getEventHdr() in
ebpfKern/sysmonHelpers.c). Without the memset, m_Extensions[PT_Sid]
could retain a stale nonzero length left over from a prior, unrelated
event on that CPU. Userspace's ExtGetPtr() / TranslateSid()
(sysmonCommon/eventsCommon.cpp) trusts that stale length and reads the
SID from memory adjacent to the actually-transmitted event, so a
ProcessTerminate (EventID 5) log entry could report a completely
unrelated user's UID in the User field.

This fix adds the same memset(event->m_Extensions, 0, sizeof(event->m_Extensions)) call that every sibling handler already
has, at the same point in the function (right after ptr is
initialized, before the SID is conditionally written).

Scope

This PR addresses only the missing zero-initialization. There is a
second, related defect in the same function (the if (uid != 0) check
around committing the SID extension silently drops the field for
root-owned terminations) that produces a different symptom — a missing
User field rather than a wrong one. That's tracked and will be
submitted as a separate PR per the "one PR per issue" guidance in
CONTRIBUTING.md, since it's a distinct code defect even though both
only manifest on root-owned terminations.

Fixes #238

Testing

Verified by code inspection against the sibling handlers' established
pattern. This specific defect depends on incidental per-CPU scratch
buffer state (which prior event last touched that memory), making it
impractical to assert deterministically in the existing integration
test suite (test/integrationtests/linux) or the rule-parsing tests in
test/rulestest (which only diff-test config parsing, not runtime
event contents) — happy to discuss an approach for regression coverage
here if maintainers have a preferred pattern for per-CPU-state-dependent
eBPF bugs.

@ipduffy ipduffy changed the title fix m_Extensions in ProcTerminated to avoid memory corruption issues fix m_Extensions in ProcTerminated to avoid invalid log data issues Jul 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ProcessTerminate events intermittently report an unrelated user's UID instead of the terminating process's UID

1 participant