Conversation
There was a problem hiding this comment.
Pull request overview
Adds bounds checking to os_parse_ndef by introducing an in_buffer_length parameter, fixing an out-of-bounds read vulnerability. Also adds a libFuzzer harness to fuzz the NFC NDEF parsing path.
Changes:
- Added
in_buffer_lengthparameter toos_parse_ndefwith input validation checks before each buffer access - Added fuzzing harness (
fuzzer_nfc_ndef.c) and CMake config for NFC NDEF parsing
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| lib_nfc/src/nfc_ndef.c | Added length parameter and bounds checks to prevent OOB reads |
| lib_nfc/include/nfc_ndef.h | Updated function signature |
| fuzzing/harness/fuzzer_nfc_ndef.c | New libFuzzer harness for os_parse_ndef and os_ndef_to_string |
| fuzzing/extra/nfc_ndef.cmake | CMake build config for the fuzzer |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| memset(out_string, 0, sizeof(out_string)); | ||
| memcpy(in_buffer, data, size); | ||
|
|
||
| os_parse_ndef(in_buffer, (uint16_t) size, &parsed); |
| @@ -134,24 +134,43 @@ uint16_t os_get_uri_header(uint8_t uri_id, char *uri_header) | |||
| * @brief deserializes an encoded NDEF message to an ndef_struct_t | |||
| * | |||
| * @param in_buffer input buffer to deseialize | |||
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1463 +/- ##
=======================================
Coverage 91.82% 91.82%
=======================================
Files 37 37
Lines 4244 4244
Branches 521 521
=======================================
Hits 3897 3897
Misses 257 257
Partials 90 90
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Description
Add a fuzzing harness for NFC NDEF parsing and fix an out-of-bounds read in
os_parse_ndefby validating input buffer length before parsing.Changes include
Breaking changes
os_parse_ndefnow takes an additionalin_buffer_lengthparameter.Auto cherry-pick in API_LEVEL
If requested to port the commits from this PR on a dedicated API_LEVEL branch,
select the targeted one(s), or add new references if not listed:
[ ] TARGET_API_LEVEL: API_LEVEL_25
[ ] TARGET_API_LEVEL: API_LEVEL_26
This will only create the PR with cherry-picks, ready to be reviewed and merged.
Remember: