Skip to content

Make advert parsing robust to malformed advert packets#2459

Draft
mtlynch wants to merge 17 commits intomeshcore-dev:devfrom
mtlynch:test-gps
Draft

Make advert parsing robust to malformed advert packets#2459
mtlynch wants to merge 17 commits intomeshcore-dev:devfrom
mtlynch:test-gps

Conversation

@mtlynch
Copy link
Copy Markdown

@mtlynch mtlynch commented May 1, 2026

This continues #925 in getting more of MeshCore under automated test coverage.

This adds unit tests for AdvertDataParser::AdvertDataParser and fixes memory corruption issues that currently exists when MeshCore receives a malformed advert packet (e.g., too short a packet).

To better catch memory issues in unit tests, I've added ASAN and UBSAN to the build config. These are compiler flags that change runtime execution only for unit tests to identify places in code that read or write beyond allocated memory buffers or rely on undefined compiler behavior.

@mtlynch
Copy link
Copy Markdown
Author

mtlynch commented May 1, 2026

This is still a work in progress and is not yet ready for review. Just sharing early progress now that #925 is in.

Comment thread test/mocks/SHA256.h Outdated
// Mock SHA256 class for testing
// Provides minimal interface to allow Utils.cpp to compile
// Mock SHA256 class for native testing.
// Provides a deterministic stand-in so code can hash data without Arduino crypto deps.
Copy link
Copy Markdown
Contributor

@nextgens nextgens May 1, 2026

Choose a reason for hiding this comment

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

That's evil.

We shouldn't depend on a mock implementation that is not doing what it says on the tin!

How do you propose we do crypto tests (regression tests in particular)? We need a way to test interfaces including those that have arduino deps (like rweather's Crypto lib).

Copy link
Copy Markdown
Author

@mtlynch mtlynch May 1, 2026

Choose a reason for hiding this comment

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

This is still a WIP, and I haven't decided for sure on this implementation yet.

That said, it's pretty normal for mocks to not literally do what they say for the sake of testing. I imagine it's uncontroversial to mock out a clock and have it always claim to be a fixed time like 2000-01-01T00:00:00Z.

Another possibility is to let the client hardcode the inputs and outputs, so the test can say, "if you get the input "hello meshcore", return 17f4227296ecf3043173ab392f5496ad8ebe86a061a4c7ac015ec38831a3a0a2."

Another option is to re-route it to a real SHA256 implementation.

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