Skip to content

Guard stackwalking in IXClrDataStackWalk codepaths to account for underlying max size of context#125819

Merged
hoyosjs merged 1 commit into
mainfrom
juhoyosa/guard-dac-context-copy
Apr 10, 2026
Merged

Guard stackwalking in IXClrDataStackWalk codepaths to account for underlying max size of context#125819
hoyosjs merged 1 commit into
mainfrom
juhoyosa/guard-dac-context-copy

Conversation

@hoyosjs

@hoyosjs hoyosjs commented Mar 20, 2026

Copy link
Copy Markdown
Member

IXClrDataStackWalk methods don't take into account that the Windows context size is the one of the Windows SDK CONTEXT on AMD64. It does not include inline XSTATE. If some customer passes in XSTATE, ContextSizeForFlags always returns 0x4D0 regardless of context flag. So CheckContextSizeForBuffer always returns true. Then CopyMemory(&m_context, context, ...) will overrun for any XSTATE input and overwrite other fields.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @steveisok, @tommcdon, @dotnet/dotnet-diag
See info in area-owners.md if you want to be subscribed.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a memory-safety issue in CoreCLR DAC stackwalking (IXCLRDataStackWalk / IXCLRDataFrame) when callers provide a CONTEXT buffer larger than the underlying T_CONTEXT (e.g., AMD64 CONTEXT + appended XSTATE), which could previously cause out-of-bounds reads/writes during CopyMemory.

Changes:

  • Cap GetContext copies to sizeof(T_CONTEXT) to avoid reading past the local context struct when callers supply oversized output buffers.
  • Cap SetContext2 copies to sizeof(m_context) to avoid overwriting adjacent fields when callers supply oversized input buffers (e.g., with XSTATE).

@max-charlamb max-charlamb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change looks good

@hoyosjs hoyosjs merged commit 2383f46 into main Apr 10, 2026
104 of 107 checks passed
@hoyosjs hoyosjs deleted the juhoyosa/guard-dac-context-copy branch April 10, 2026 04:15
@github-actions github-actions Bot locked and limited conversation to collaborators May 10, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants