Skip to content

Protocol parser fixes and ASAN - #3524

Closed
neilalexander wants to merge 1 commit into
meshcore-dev:devfrom
neilalexander:neil/parser2
Closed

neilalexander wants to merge 1 commit into
meshcore-dev:devfrom
neilalexander:neil/parser2

Conversation

@neilalexander

Copy link
Copy Markdown
Contributor

A number of severe problems existed in the protocol parser:

  • Null/out-of-bounds reads in Packet::readFrom for empty or truncated frames
  • Unsafe logic duplicated into Dispatcher::tryParsePacket
  • Short typed payloads could expose stale/uninitialised memory
  • Partial AES ciphertext could cause block-sized overreads
  • Decrypted PATH payloads could advance beyond their valid length
  • Invalid TRACE packets, zero-hop CONTROL routes and oversized advert data were accepted
  • AdvertDataParser::AdvertDataParser failed to reject null, empty, oversized, truncated inputs
  • Multi-ack overflow in plain and multipart ACKs outside of supported byte ranges
  • Anonymous reply path overread without considering decrypted length

These are now fixed, along with added tests and ASAN testing target.

A number of severe problems existed in the protocol parser:

* Null/out-of-bounds reads in `Packet::readFrom` for empty or truncated frames
* Unsafe logic duplicated into `Dispatcher::tryParsePacket`
* Short typed payloads could expose stale/uninitialised memory
* Partial AES ciphertext could cause block-sized overreads
* Decrypted PATH payloads could advance beyond their valid length
* Invalid TRACE packets, zero-hop CONTROL routes and oversized advert data were accepted
* `AdvertDataParser::AdvertDataParser` failed to reject null, empty, oversized, truncated inputs
* Multi-ack overflow in plain and multipart ACKs outside of supported byte ranges
* Anonymous reply path overread without considering decrypted length

These are now fixed, along with added tests and ASAN testing target.
@neilalexander

Copy link
Copy Markdown
Contributor Author

Apologies, was trying to open the PR on my own fork to check CI and hit the wrong base repo.

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