Skip to content

bound key material lengths in KeyMaterialCDRDeserialize#6425

Open
alhudz wants to merge 1 commit into
eProsima:masterfrom
alhudz:keymat-deserialize-bounds
Open

bound key material lengths in KeyMaterialCDRDeserialize#6425
alhudz wants to merge 1 commit into
eProsima:masterfrom
alhudz:keymat-deserialize-bounds

Conversation

@alhudz

@alhudz alhudz commented Jun 9, 2026

Copy link
Copy Markdown

Description

KeyMaterialCDRDeserialize parses the dds.cryp.keymat binary property carried in a CryptoToken sent by a remote participant, so every length byte in it crosses a trust boundary.

Repro: join a secured domain and send a participant/datawriter/datareader CryptoToken whose first sequence length byte (master_salt) is 0xFF followed by enough bytes.
Cause: each key_len is read straight off the wire and handed to memcpy into the 32-byte std::array members (master_salt, master_sender_key, master_receiver_specific_key) with no check, and the CDR buffer size is never validated before indexing, so a key_len up to 255 writes past the destination (ASan reports a stack-buffer-overflow, WRITE of size 255).
Fix: validate the available CDR size at each step and reject any key_len larger than its destination array, returning false so the three set_remote_*_crypto_tokens callers drop the malformed token.

The validation lives in the deserialiser itself rather than the callers, since the callers only have the opaque token bytes.

Contributor Checklist

  • Commit messages follow the project guidelines.
  • The code follows the style guidelines of this project.
  • Tests that thoroughly check the new feature have been added/Regression tests checking the bug and its fix have been added; the added tests pass locally
  • N/A Any new/modified methods have been properly documented using Doxygen.
  • N/A Any new configuration API has an equivalent XML API (with the corresponding XSD extension)
  • Changes are backport compatible: they do NOT break ABI nor change library core behavior.
  • Changes are API compatible.
  • N/A New feature has been added to the versions.md file (if applicable).
  • N/A New feature has been documented/Current behavior is correctly described in the documentation.
  • Applicable backports have been included in the description.

Signed-off-by: alhudz <al.hudz.k@gmail.com>
@alhudz

alhudz commented Jun 18, 2026

Copy link
Copy Markdown
Author

gentle ping

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.

1 participant