Skip to content

prevent set overflow#12

Merged
thedavidmeister merged 2 commits intomainfrom
2026-02-11-audit
Feb 11, 2026
Merged

prevent set overflow#12
thedavidmeister merged 2 commits intomainfrom
2026-02-11-audit

Conversation

@thedavidmeister
Copy link
Contributor

@thedavidmeister thedavidmeister commented Feb 11, 2026

Motivation

Solution

Checks

By submitting this for review, I'm confirming I've done the following:

  • made this PR as small as possible
  • unit-tested any new functionality
  • linked any relevant issues or PRs
  • included screenshots (if this involves a front-end change)

Summary by CodeRabbit

  • New Features

    • Added runtime validation to prevent exceeding memory capacity when inserting key/value pairs; operations now revert with a specific overflow error if limits are exceeded.
  • Tests

    • Added tests covering memory overflow scenarios to verify the library rejects invalid insertions and triggers the new overflow error.

@coderabbitai
Copy link

coderabbitai bot commented Feb 11, 2026

Walkthrough

Adds overflow detection to LibMemoryKV by introducing a new public error and a runtime check in the set function to revert when the insertion pointer exceeds 0xFFFF; tests added to trigger and verify the overflow behavior.

Changes

Cohort / File(s) Summary
Library - Overflow check
src/lib/LibMemoryKV.sol
Added public error MemoryKVOverflow(uint256 pointer) and a runtime check in set to revert if the computed insertion pointer > 0xFFFF. Introduced a local pointer variable and adjusted assembly/assignment usage in set.
Tests - overflow trigger & assertion
test/src/lib/LibMemoryKV.getset.t.sol
Added setOverflowExternal(MemoryKV kv, MemoryKVKey key, MemoryKVVal value) helper to force a pointer state that causes overflow, and testSetOverflow() which expects MemoryKVOverflow when invoking the helper and set. Existing testSetGet0 unchanged.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'prevent set overflow' directly and clearly summarizes the main change: adding overflow prevention to the set function in LibMemoryKV.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ 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
  • Commit unit tests in branch 2026-02-11-audit

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.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@src/lib/LibMemoryKV.sol`:
- Around line 5-6: Remove the unused forge-std debug import by deleting the line
importing console2 (import {console2} from "forge-std/console2.sol";) from
LibMemoryKV.sol and ensure there are no remaining references to console2 in the
file; this eliminates a production dependency and reduces compilation footprint.

@thedavidmeister thedavidmeister merged commit 83e6079 into main Feb 11, 2026
3 of 4 checks passed
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/lib/LibMemoryKV.sol (1)

90-98: 🧹 Nitpick | 🔵 Trivial

No length-counter overflow check — but pointer overflow is always hit first.

Each insert allocates 0x60 (96) bytes, so starting from the default free-memory pointer (0x80), the pointer crosses 0xFFFF after roughly (0xFFFF - 0x80) / 0x60 ≈ 682 inserts. At that point the length counter is only ~1364, well within the 16-bit max of 65535. So the pointer guard implicitly protects the length as well.

If the free-memory pointer is already high when set is first called (due to other allocations), the headroom is even smaller, making the guard trigger earlier. This is worth documenting (e.g., a brief comment near line 101) so future maintainers don't wonder about a separate length-overflow check.

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