Skip to content

fix(nebula_continental): reject truncated ARS548 object list packets - #505

Open
mojomex wants to merge 2 commits into
refactor/ars548-decoder-loggerfrom
fix/ars548-object-list-bounds
Open

mojomex wants to merge 2 commits into
refactor/ars548-decoder-loggerfrom
fix/ars548-object-list-bounds

Conversation

@mojomex

@mojomex mojomex commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

PR Type

  • Bug Fix

Related Links

  • GHSA-vfxx-jpg9-gqcp

Description

The common-format object list selector in ContinentalARS548Decoder::process_packet() accepted a packet when either the received payload size or the advertised PDU length matched. A packet with a matching PDU length but a short payload therefore reached parse_objects_list_packet(), which copies a fixed number of bytes and read out of bounds (CWE-125).

  • Require both the payload size and the PDU length to match, as the FW40 branch already does.
  • Reject packets shorter than header + max_objects * sizeof(ObjectPacket) in parse_objects_list_packet() as defense in depth.
  • Add a ground-truth bag recorded from an ARS540DI corner radar (firmware 5.40.4, 200-object list format), so both object list formats are now covered by recorded packets.
  • Add regression tests: header-only, truncated, oversized, mismatched-format and off-by-one PDU length packets of both object list formats are rejected, correctly sized packets of both formats are decoded, and truncated detection list and sensor status packets are rejected.

Stacked on #515.

Review Procedure

Review the diff and the new test cases in continental_ros_decoder_test_main_ars548.cpp.

Remarks

The test fixture gained a toggle to skip ground-truth comparison and a callback counter so that synthetic packets not present in the test bag can be asserted on.

Pre-Review Checklist for the PR Author

PR Author should check the checkboxes below when creating the PR.

  • Assign PR to reviewer

Checklist for the PR Reviewer

Reviewers should check the checkboxes below before approval.

  • Commits are properly organized and messages are according to the guideline
  • (Optional) Unit tests have been written for new behavior
  • PR title describes the changes

Post-Review Checklist for the PR Author

PR Author should check the checkboxes below before merging.

  • All open points are addressed and tracked via issues or tickets

CI Checks

  • Build and test for PR: Required to pass before the merge.

🤖 Generated with Claude Code

@mojomex mojomex self-assigned this Oct 21, 2026
@mojomex
mojomex force-pushed the fix/ars548-object-list-bounds branch 2 times, most recently from a6b9b9a to 6db75bf Compare October 22, 2026 06:33
@mojomex
mojomex changed the base branch from main to refactor/ars548-decoder-logger October 22, 2026 06:33
@mojomex
mojomex added this pull request to stack #516 October 22, 2026 06:53
@mojomex
mojomex force-pushed the fix/ars548-object-list-bounds branch from 6db75bf to 9d05cdf Compare October 22, 2026 06:56
@mojomex
mojomex marked this pull request as ready for review October 22, 2026 08:05
mojomex and others added 2 commits October 26, 2026 15:14
…(GHSA-vfxx-jpg9-gqcp)

The common-format object list selector accepted a packet when either the
received payload size or the advertised PDU length matched, so a packet
with a matching PDU length but a short payload reached
parse_objects_list_packet() and was read out of bounds (CWE-125).

- Require both the payload size and the PDU length to match, like the FW40 branch already does
- Reject packets shorter than header + max_objects * sizeof(ObjectPacket) in parse_objects_list_packet() as defense in depth
- Add regression tests: header-only, truncated, oversized, mismatched-format and off-by-one PDU length packets of both object list formats are rejected, and correctly sized packets of both formats are decoded; truncated detection list and sensor status packets are rejected

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Add a ground-truth bag recorded from an ARS548 flashed with the
ARS540DI firmware (5.40.4), mounted as a corner radar. It exercises the
200-object list format (37452-byte payload, PDU length 37444) with real
sensor data; previously only the 50-object ARS548 format had a recording.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@mojomex
mojomex force-pushed the fix/ars548-object-list-bounds branch from 6d41215 to 75a4d76 Compare October 26, 2026 06:14
@codecov

codecov Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 42.85714% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 47.01%. Comparing base (40b7d2d) to head (75a4d76).

Additional details and impacted files
@@                        Coverage Diff                         @@
##           refactor/ars548-decoder-logger     #505      +/-   ##
==================================================================
+ Coverage                           45.72%   47.01%   +1.28%     
==================================================================
  Files                                 137      137              
  Lines                               10359    10365       +6     
  Branches                             1282     1284       +2     
==================================================================
+ Hits                                 4737     4873     +136     
+ Misses                               5620     5490     -130     
  Partials                                2        2              
Flag Coverage Δ
nebula_continental 37.85% <42.85%> (+4.90%) ⬆️
nebula_continental_decoders 37.85% <42.85%> (+4.90%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

1 participant