Skip to content

Validate key material length when deserializing crypto tokens - #6552

Open
Alearner12 wants to merge 1 commit into
eProsima:masterfrom
Alearner12:fix/key-material-cdr-bounds
Open

Alearner12 wants to merge 1 commit into
eProsima:masterfrom
Alearner12:fix/key-material-cdr-bounds

Conversation

@Alearner12

Copy link
Copy Markdown

Description

KeyMaterialCDRDeserialize() reads dds.cryp.keymat from remote crypto tokens without checking the input size or the sequence lengths before copying into 32-byte arrays. Malformed input can read past the input or overwrite a key array.

This change:

  • Bounds every parser read and validates the transformation kind and key lengths (16 bytes for AES-128, 32 for AES-256), matching the local serializer.
  • Returns a parse result so participant, data reader, and data writer token handlers reject malformed key material before storing it.
  • Adds round-trip and malformed-input tests, including truncation with and without a receiver-specific key, plus a participant-token rejection check.

The original parser produced an AddressSanitizer stack-buffer-overflow when a length byte requested a 34-byte copy into a 32-byte key array; empty input also read out of bounds. With the fix, all 17 cryptography tests pass under AddressSanitizer. The full fastdds library target builds, and the three changed files pass eProsima's uncrustify configuration.

Fixes #6345

Potential backports for affected branches: 3.6.x, 3.5.x, and 2.14.x. Each still has the unchecked parser; maintainers can choose the supported targets.

Contributor Checklist

  • Commit messages follow the project guidelines (DCO sign-off).
  • The code follows the style guidelines of this project.
  • Regression tests have been added and pass locally.
  • New or modified methods have Doxygen documentation.
  • N/A No new configuration API or XML setting.
  • Changes are backport compatible and do not change the public API.
  • Changes are API compatible.
  • N/A No new feature for versions.md.
  • N/A No user-facing documentation change is needed.
  • Potential backport branches are listed above.

Signed-off-by: Alearner12 <siddharthtiwari155@yahoo.com>

This branch has not been deployed

No deployments
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.

Remote memory corruption in DDS Security key material deserialization (dds.sec.participant_crypto_tokens) in Fast DDS

1 participant