Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add slow VM validation for calldata offset checks #107

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mininny
Copy link
Collaborator

@mininny mininny commented Dec 15, 2024

Description

The slow VM implementation is supposed to replicate the Solidity implementation. It takes as input the same calldata and process them.

However, the slow VM implementation lacks calldata offset checks for the _stateData and _proof parameters.
This can lead the same calldata payload to succeed on the slow implementation but revert on the Solidity implementation.

The Solidity implementation ensures that the _stateData.offset == 132 and that _proof.offset == 548.

            //
            // Initial EVM memory / calldata checks
            //
            if iszero(eq(mload(0x40), 0x80)) {
                // expected memory check: no allocated memory (start after scratch + free-mem-ptr + zero slot = 0x80)
                revert(0, 0)
            }
            if iszero(eq(_stateData.offset, 132)) { // @POC: check _stateData offset
                // 32*4+4 = 132 expected state data offset
                revert(0, 0)
            }
            if iszero(eq(calldataload(sub(_stateData.offset, 32)), stateSize())) {
                // user-provided state size must match expected state size
                revert(0, 0)
            }
            function paddedLen(v) -> out {
                // padded to multiple of 32 bytes
                let padding := mod(sub(32, mod(v, 32)), 32)
                out := add(v, padding)
            }
            if iszero(eq(_proof.offset, add(add(_stateData.offset, paddedLen(stateSize())), 32))) {
                // 132+stateSize+padding+32 = expected proof offset
                revert(0, 0)
            }
            function proofContentOffset() -> out {
                // since we can't reference proof.offset in functions, blame Yul
                // 132+362+(32-362%32)+32=548
                out := 548
            }
            if iszero(eq(_proof.offset, proofContentOffset())) { revert(0, 0) } // @POC: check _proof offset

But, Go slow implementation does not check those.

	//
	// Initial EVM memory / calldata checks
	//
	calldataload := func(offset U64) (out [32]byte) {
		copy(out[:], calldata[offset.val():])
		return
	}

	stateContentOffset := uint8(4 + 32 + 32 + 32 + 32)
	if iszero(eq(b32asBEWord(calldataload(toU64(4+32*3))), shortToU256(stateSize))) {
		// user-provided state size must match expected state size
		panic("invalid state size input")
	}

	proofContentOffset := shortToU64(uint16(stateContentOffset) + paddedStateSize + 32)

	if and(b32asBEWord(calldataload(shortToU64(uint16(stateContentOffset)+paddedStateSize))), shortToU256(60-1)) != (U256{}) {
		// proof offset must be stateContentOffset+paddedStateSize+32
		// proof size: 64-5+1=60 * 32 byte leaf,
		// but multiple memProof can be used, so the proofSize must be a multiple of 60
		panic("invalid proof offset input")
	}

Thus, this implements the same offsets check in the Go slow implementation.

@mininny mininny force-pushed the feature/mininny/audit-21 branch from 1712602 to 7b6a8ee Compare December 15, 2024 12:20
@codecov-commenter
Copy link

codecov-commenter commented Dec 15, 2024

Codecov Report

Attention: Patch coverage is 20.00000% with 8 lines in your changes missing coverage. Please review.

Project coverage is 61.78%. Comparing base (8ff9318) to head (4bb1569).
Report is 18 commits behind head on master.

Files with missing lines Patch % Lines
rvgo/slow/vm.go 20.00% 5 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #107      +/-   ##
==========================================
- Coverage   61.89%   61.78%   -0.11%     
==========================================
  Files          27       27              
  Lines        4091     4098       +7     
==========================================
  Hits         2532     2532              
- Misses       1427     1432       +5     
- Partials      132      134       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@BlocksOnAChain BlocksOnAChain added the Audit finding grouping for our audit findings label Dec 16, 2024
rvgo/slow/vm.go Outdated
@@ -122,6 +122,12 @@ func Step(calldata []byte, po PreimageOracle) (stateHash common.Hash, outErr err
}

stateContentOffset := uint8(4 + 32 + 32 + 32 + 32)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we make stateContentOffset a uint16 by default? or is it used elsewhere as a uint8 that I'm missing

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No other intent! I changed stateContentOffset to uint16 by default :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Audit finding grouping for our audit findings
Development

Successfully merging this pull request may close these issues.

4 participants