Fix unvalidated TerminalProfileSize during distribution import - #41495
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a potential out-of-bounds read in LxssUserSessionImpl::_ProcessImportResultMessage when constructing a std::string_view for Windows Terminal profile data during distribution import, by validating TerminalProfileSize against the received buffer length via gsl::span.
Changes:
- Switch
Span.subspan(Message.TerminalProfileIndex)toSpan.subspan(Message.TerminalProfileIndex, Message.TerminalProfileSize)sooffset + countis validated. - Build the
std::string_viewusing the resulting span’s actual size (terminalProfileSpan.size()) rather than trusting the message size field directly.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
56b8937 to
493d601
Compare
There was a problem hiding this comment.
🟢 Approval recommended
The change is minimal, matches the existing local pattern for other payload slices, and directly addresses the described out-of-bounds risk.
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0 new
- Review effort level: Lite
_ProcessImportResultMessage constructed the terminal profile string_view using the message-supplied TerminalProfileSize without validating it against the received buffer length. Use the bounds-checked two-argument span::subspan() overload (matching the existing ShortcutIconSize handling a few lines above) so an inconsistent size value throws instead of producing a string_view that runs past the end of the buffer. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 35281c30-3d08-4f05-8c84-2ce4711023d5
493d601 to
2af9b69
Compare
There was a problem hiding this comment.
🟢 Approval recommended
The change is minimal, matches an existing validated pattern in the same function, and directly addresses the out-of-bounds string_view risk.
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0 new
- Review effort level: Lite
Summary
LxssUserSessionImpl::_ProcessImportResultMessage builds a string_view over the terminal profile data using the message's TerminalProfileSize field directly, without validating it against the size of the received buffer. If the value is inconsistent with the actual buffer length, the resulting string_view extends past the end of the buffer.
Fix
Use the bounds-checked two-argument gsl::span::subspan(offset, count) overload instead of the single-argument form, matching the pattern already used a few lines above for ShortcutIconIndex/ShortcutIconSize. This ensures offset + count is validated against the span size before it's used to build the string_view.