Skip to content

Conversation

@mdh1418
Copy link
Member

@mdh1418 mdh1418 commented Dec 6, 2025

While investigating for a way to check whether arbitrary .NET processes support emitting userevents, the idea of probing for DS_IPC_E_UNKNOWN_COMMAND 0x80131385 with an empty payload uncovered a problem with truncated payloads. Additionally, there were two other places to improve the resiliency of IPC protocol parsing.

@mdh1418 mdh1418 requested a review from jkotas December 6, 2025 03:24
Copilot AI review requested due to automatic review settings December 6, 2025 03:24
@github-actions github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Dec 6, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR improves the resiliency of IPC message parsing in the Diagnostic Server by addressing issues with truncated payloads. The changes add defensive checks to prevent buffer underruns and ensure proper data alignment when parsing UTF-16 strings.

Key Changes:

  • Added alignment validation for UTF-16 string parsing to ensure buffer pointers are properly aligned
  • Replaced an ineffective underflow assertion with both a clearer assertion and runtime validation in ds_ipc_message_try_parse_value
  • Fixed comparison logic in attach_profiler_command_try_parse_payload to correctly validate sufficient buffer space

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/native/eventpipe/ds-protocol.c Adds buffer alignment check for UTF-16 parsing and improves buffer length validation with runtime check
src/native/eventpipe/ds-profiler-protocol.c Corrects buffer size validation from <= to >= to ensure adequate remaining buffer

@mdh1418 mdh1418 force-pushed the impove_ipc_message_parsing_resiliency branch from e54397b to 93ea92f Compare December 6, 2025 03:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant