Skip to content

Fix heap-buffer-overflow in legacy maps section parsing#1052

Merged
elazarg merged 1 commit intovbpf:mainfrom
mikeagun:map-count-fix
Mar 11, 2026
Merged

Fix heap-buffer-overflow in legacy maps section parsing#1052
elazarg merged 1 commit intovbpf:mainfrom
mikeagun:map-count-fix

Conversation

@mikeagun
Copy link
Contributor

@mikeagun mikeagun commented Mar 11, 2026

Problem

parse_map_sections in elf_map_parser.cpp computes map_count using ceiling division when the section size isn't evenly
divisible by map_record_size:

map_count = (max_record_end + map_record_size - 1) / map_record_size;

This can produce a count where the last record's byte range exceeds the section buffer. The platform parse_maps_section
callback then performs an out-of-bounds read via memcpy.

Example: section=85 bytes, map_record_size=28 → ceiling gives 4 records (needs 112 bytes), but only 85 are available.

Fix

Two changes:

  1. Floor division for the map_count computation. Since max_record_end <= section_size is already enforced, floor division guarantees all records fit within the buffer.
  2. Pre-call bounds check (map_count * map_record_size <= section_size) as a safety invariant before invoking the platform callback to help avoid future regression.

Validation

  • Reproduced the ebpf-for-windows verifier fuzzer ASAN crash locally.
  • Confirmed the fix eliminates the crash (exit code 0, no ASAN errors)

Impact

Both the Windows (_parse_maps_section_windows) and Linux (parse_maps_section_linux) callbacks are affected — neither receives the buffer size, so both rely on a correct map_count from the caller.

A follow-up PR will add a regression test fixture using the fuzzer crash input.

Fixes #1051

Summary by CodeRabbit

Bug Fixes

  • Improved validation and error detection for legacy map data sections to properly identify and safely handle malformed or corrupted entries during import
  • Added comprehensive bounds checking mechanisms to ensure all map data conforms to expected size requirements and prevent parsing errors
  • Enhanced error reporting and validation messages to better inform users when invalid, corrupted, or non-conforming map data is encountered

The map_count computation used ceiling division when the section size
was not evenly divisible by map_record_size, which can produce a count
whose last record extends past the section data buffer. This causes a
heap-buffer-overflow when the platform's parse_maps_section callback
iterates all records.

Fix by using floor division, and add a pre-call bounds check as a
safety invariant to prevent future regressions.

Signed-off-by: Michael Agun <danielagun@microsoft.com>
@coderabbitai
Copy link

coderabbitai bot commented Mar 11, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: d198ffe0-839c-48b5-816b-0c05177d5d59

📥 Commits

Reviewing files that changed from the base of the PR and between 6f0384e and 10de1d4.

📒 Files selected for processing (1)
  • src/io/elf_map_parser.cpp

Walkthrough

Modified src/io/elf_map_parser.cpp to replace ceiling-style map_count computation with floor division and added a safety invariant check to prevent map_count * map_record_size from exceeding section size, throwing UnmarshalError on violation to prevent heap-buffer-overflow.

Changes

Cohort / File(s) Summary
ELF Map Parsing Safety
src/io/elf_map_parser.cpp
Changed map_count calculation from ceiling division to floor division and added invariant check with UnmarshalError throw condition if computed size exceeds section bounds; includes clarifying comments.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

Possibly related PRs

  • Handle invalid BTF map metadata #1034: Complements this change by catching UnmarshalError exceptions from BTF/map parsing to fall back to section-based parsing, which now has stricter validation with this PR's new safety checks.
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: fixing a heap-buffer-overflow vulnerability in legacy maps section parsing, which is the core issue addressed in the PR.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@elazarg elazarg merged commit 4790e81 into vbpf:main Mar 11, 2026
16 checks passed
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.

Heap-buffer-overflow in legacy maps section parsing due to ceiling division in map_count

2 participants