fix: make Bytes.toNibbles allocation memory-safe

[Forostovec]

Bytes.toNibbles was updating the free memory pointer assuming a logical payload size of 2 * input.length, but the loop was actually writing 32 * ceil(length / 16) bytes. For non‑multiple-of-16 inputs this meant some mstore writes occurred above the new free memory pointer, which violates the assumptions of the memory-safe assembly dialect even though the visible bytes contents and length remained correct.

This change keeps the logical length of the returned buffer at 2 * input.length, but adjusts the allocation and free memory pointer to reserve 32 * ceil(length / 16) bytes for the internal 32‑byte chunk writes. This aligns the actual writes with the declared allocation, restoring compliance with memory-safe assembly and Yul IR expectations without changing the external behaviour of toNibbles.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 763665c

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[coderabbitai]

Walkthrough

The toNibbles function in contracts/utils/Bytes.sol is refactored to compute memory allocation more explicitly. Two new local variables are introduced: chunks (calculated via right-shift operations) and dataBytes (calculated via left-shift operations based on chunks). The memory allocation now uses dataBytes instead of the previous formula based on mul(length, 2). The output length value and nibble-writing loop operations remain unchanged. The functional behavior is preserved while the memory allocation calculation is updated.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'fix: make Bytes.toNibbles allocation memory-safe' directly and clearly summarizes the main change: addressing a memory-safety issue in the Bytes.toNibbles function.
Description check	✅ Passed	The description is directly related to the changeset, explaining the memory allocation issue in Bytes.toNibbles and how the fix aligns actual memory writes with declared allocation.
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.}

---

📜 Recent review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ad83688 and 763665c.

📒 Files selected for processing (1)

• contracts/utils/Bytes.sol

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)

• GitHub Check: Redirect rules - solidity-contracts
• GitHub Check: tests-foundry
• GitHub Check: tests
• GitHub Check: slither
• GitHub Check: coverage
• GitHub Check: tests-upgradeable
• GitHub Check: halmos
• GitHub Check: Header rules - solidity-contracts
• GitHub Check: Pages changed - solidity-contracts

🔇 Additional comments (1)

contracts/utils/Bytes.sol (1)

213-216: Excellent fix for memory-safe assembly compliance.

The changes correctly address the memory safety issue:

1. Line 213: chunks := shr(4, add(length, 0x0f)) computes ceil(length / 16), the number of loop iterations
2. Line 214: dataBytes := shl(5, chunks) computes chunks * 32, the actual bytes written by the loop
3. Line 216: Allocates dataBytes bytes instead of 2 * length bytes

This ensures that all 32-byte mstore operations at line 244 remain within allocated memory. For example, with length = 17:

• Old allocation: 34 bytes
• Loop writes 32 bytes at offsets 0 and 32 (the second write at 32-63 would exceed the 34-byte boundary)
• New allocation: 64 bytes (safely contains both writes)

The logical output length (line 217) correctly remains 2 * length, preserving the function's observable behavior.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[Amxx]

which violates the assumptions of the memory-safe assembly dialect

This is incorrect. See https://docs.soliditylang.org/en/latest/assembly.html#memory-safety

In particular, a memory-safe assembly block may only access the following memory ranges:

• [...]
• Temporary memory that is located after the value of the free memory pointer at the beginning of the assembly block, i.e. memory that is “allocated” at the free memory pointer without updating the free memory pointer.

Anything that is after the FMP can be accessed. If you read it, there is no guarantee that it contains zero. If you write to it and don't move the FMP accordingly, there is no guarantee that it won't be overwritten by another operation.

The current version is safe and doesn't require a fix.