Skip to content

Conversation

@IntelCaisui
Copy link
Contributor

@IntelCaisui IntelCaisui commented Nov 21, 2025

Enhance the payload size check in SPDM receive function to prevent
buffer overflow and interger overflow.

Fix problem 2 of issue #603

@IntelCaisui IntelCaisui requested a review from jyao1 as a code owner November 21, 2025 05:50
@IntelCaisui IntelCaisui force-pushed the 251121_SPDM_RECEIVE_PAYLOAD_SIZE_CHECK branch from 70b4040 to 227199c Compare November 21, 2025 06:30
return Err(0_usize);
}

let mut buffer = buffer.lock();
Copy link
Contributor

Choose a reason for hiding this comment

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

Please don't lock() twice.

The above check should be put below the lock().

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated. sorry for such incautious problem.

@liuw1 liuw1 requested a review from Copilot November 21, 2025 08:17
Copy link

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 enhances payload size validation in the SPDM receive function to prevent buffer overflow and integer overflow vulnerabilities. The changes add an early buffer size check and refactor the payload size comparison to avoid potential integer underflow.

  • Added early validation to ensure the buffer can accommodate the SPDM message header
  • Refactored payload size comparison to prevent integer underflow when buffer length is smaller than header size

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

let payload_size = vmcall_msg_header.length as usize;

if buffer.len() < payload_size + VMCALL_SPDM_MESSAGE_HEADER_SIZE {
if payload_size > buffer.len() - VMCALL_SPDM_MESSAGE_HEADER_SIZE {
Copy link

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

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

This check can still underflow if buffer.len() is less than VMCALL_SPDM_MESSAGE_HEADER_SIZE. While the early check at line 65 prevents this for the initial buffer lock, the buffer reference at line 69 is a new mutable lock that could potentially have a different length. Consider using checked_sub() or restructuring to use the result from the early check: if payload_size > buffer.len().saturating_sub(VMCALL_SPDM_MESSAGE_HEADER_SIZE)

Suggested change
if payload_size > buffer.len() - VMCALL_SPDM_MESSAGE_HEADER_SIZE {
if payload_size > buffer.len().saturating_sub(VMCALL_SPDM_MESSAGE_HEADER_SIZE) {

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated.

Enhance the payload size check in SPDM receive function to prevent
  buffer overflow and interger overflow.
@IntelCaisui IntelCaisui force-pushed the 251121_SPDM_RECEIVE_PAYLOAD_SIZE_CHECK branch from 227199c to dc533b0 Compare November 21, 2025 08:26
@jyao1 jyao1 merged commit 9b2d101 into intel:main Nov 24, 2025
50 checks passed
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.

2 participants