Skip to content

Conversation

austinderek
Copy link

When parsing DWARF information, the applyRelocations functions are called to process relocations. A malformed ELF file can contain a relocation entry with an offset that points outside the destination buffer. This leads to an out-of-bounds access when attempting to apply the relocation, causing a panic.

This change adds bounds checks to all 'applyRelocations' functions to validate the offset before writing. If an offset is invalid, the relocation entry is skipped, preventing the panic when parsing a malformed ELF file.

Fixes golang#75516


🔄 This is a mirror of upstream PR golang#75522

@austinderek austinderek force-pushed the master branch 30 times, most recently from 37c78b5 to 0ab038a Compare September 19, 2025 04:01
@austinderek austinderek force-pushed the master branch 27 times, most recently from 5500cbf to d17a28a Compare September 29, 2025 05:02
@austinderek austinderek deleted the elf_range_check branch September 29, 2025 05:03
Copy link

staging bot commented Sep 29, 2025

HackerOne Code Security Review

🟢 Scan Complete: 1 Issue(s)
🟠 Validation Complete: One or more Issues looked potentially actionable, so this was escalated to our network of engineers for manual review. Once this is complete you'll see an update posted.

Here's how the code changes were interpreted and info about the tools used for scanning.

📖 Summary of Changes The changes in the ELF file handling code focus on enhancing relocation safety by introducing a new `putUint` function. This function provides more robust handling of byte manipulations during relocation processes, with improved overflow and bounds checking across different architectures. The modification aims to increase the reliability of ELF file parsing and manipulation. | File | Summary | | --- | --- | | src/debug/elf/file.go | The changes include adding a new function `putUint` to handle relocations more safely, with additional checks for overflow and bounds. This function replaces direct byte manipulation in relocation methods across various architectures, improving robustness when applying relocations. |
ℹ️ Issues Detected

NOTE: These may not require action!

Below are unvalidated results from the Analysis Tools that ran during the latest scan for transparency. We investigate each of these for accuracy and relevance before surfacing them as a potential problem.

How will I know if something is a problem?
When validation completes, any concerns that warrant attention prior to merge will be posted as inline comments. These will show up in 2 ways:

  • Expert review (most cases): Issues will be posted by experts who manually reviewed and validated them. These are real HackerOne engineers (not bots) reviewing through an integrated IDE-like tool. You can communicate with them like any other reviewer. They'll stay assigned and get notified with commit & comment updates.
  • Automatically: In cases where our validation checks have highest confidence the problem is legitimate and urgent. These will include a description of contextual reasoning why & actionable next steps.
File & Line Issue
src/debug/elf/file.go Line 1834 The new putUint function has a potential integer overflow vulnerability. In line 1834, the check math.MaxUint64-start < length is intended to prevent integer overflow when calculating start+length, but this check itself could overflow if start is very large. The proper way to check for overflow would be to first check if start > math.MaxUint64-length before performing the addition.
🧰 Analysis tools - [ ✅ ] [HackerOne AI Code Analysis](https://www.pullrequest.com/blog/harnessing-ai-to-pinpoint-security-hotspots-in-code-review-a-deep-dive/) - [ ✅ ] [HackerOne AI Code Validation](https://www.hackerone.com/blog/ai-triage-code-validation-security) - [ ✅ ] [semgrep](https://semgrep.dev?&utm_source=hackerone&utm_campaign=pullrequest) - [ ✅ ] gosec - [ ✅ ] bandit

⏱️ Latest scan covered changes up to commit 4914431 (latest)

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