Vyper - Compiler
Vyper
Updates
160,000 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

slice() of msg.data, self.code and address.code can be manipulated to return 0

obront

Summary

When the built-in slice() function is called, there are three special values that have their own logic:

if src.value in ADHOC_SLICE_NODE_MACROS:
return _build_adhoc_slice_node(src, start, length, context)

The _build_adhoc_slice_node() function implements custom logic on the msg.data, self.code, and address.code types to perform custom bounds checks and return the requested slice. The bounds checks implemented in the fetch_call_return() function are skipped for these types.

Let's look at msg.data as an example (although the same issue applies to all three):

# `msg.data` by `calldatacopy`
if sub.value == "~calldata":
node = [
"seq",
["assert", ["le", ["add", start, length], "calldatasize"]], # runtime bounds check
["mstore", np, length],
["calldatacopy", np + 32, start, length],
np,
]

This IR code performs a runtime bounds check to ensure that start + length < calldatasize, in order to ensure that we can only access memory inside of the calldata.

However, there is no overflow check on the addition. As a result, it is possible for a user who has the ability to specify either the start or the length to cause an overflow and bypass the bounds check. The result will be a slice of memory that is outside of the calldata region, and will return 0.

Vulnerability Details

Consider the following Vyper contract, which allows a user to specify an index into msg.data that meets the criteria idx % 32 == 4 and returns 32 bytes from that index. This should only allow a user to choose indexes that correspond to the starts of the function arguments. Any index beyond 68 should throw an out of bound error.

# @version ^0.3.9
​
@external
def choose_value(_x: uint256, _y: uint256, _z: uint256, idx: uint256) -> Bytes[32]:
assert idx % 32 == 4
return slice(msg.data, idx, 32)
​

The following Foundry test case demonstrates that the bound check can be violated by using a sufficiently high idx value:

function test__sliceOverflow() public {
c = SuperContract(deployer.deploy("src/loose/", "slicebreak", ""));
​
// passing an index of 4 works and returns the first value
uint idx = 4;
uint ret = abi.decode(c.choose_value(1, 2, 3, idx), (uint));
assert(ret == 1);
​
// passing an index of 36 works and returns the second value
idx = 36;
ret = abi.decode(c.choose_value(1, 2, 3, idx), (uint));
assert(ret == 2);
​
// passing an index that doesn't line up with idx % 32 == 4 reverts
idx = 20;
vm.expectRevert();
c.choose_value(1, 2, 3, idx);
​
// passing an index outside of calldata bounds reverts
idx = 132;
vm.expectRevert();
c.choose_value(1, 2, 3, idx);
​
// but a large enough value overflows and returns zero
idx = 115792089237316195423570985008687907853269984665640564039457584007913129639908;
ret = abi.decode(c.choose_value(1, 2, 3, idx), (uint));
assert(ret == 0);
}

Impact

Vyper functions that are written in order to restrict the possible values that could be returned can be abused by a malicious user to return an unexpected value.

Tools Used

Manual Review, Foundry

Recommendation

The bounds check in the _build_adhoc_slice_node() function should be updated to ensure that the addition of start and length does not overflow:

# `msg.data` by `calldatacopy`
if sub.value == "~calldata":
node = [
+ "with",
+ "_final_idx",
+ ["add", start, length],
+ [
"seq",
- ["assert", ["le", ["add", start, length], "calldatasize"]], # runtime bounds check
+ ["assert", ["and", ["ge", "_final_idx", start], ["le", "_final_idx", "calldatasize"]]], # runtime bounds check
["mstore", np, length],
["calldatacopy", np + 32, start, length],
np,
+ ]
]
Updates

Lead Judging Commences

patrickalphac Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

slice overflow calldata,code

severity to be re-evaluated

patrickalphac Auditor
over 1 year ago
patrickalphac Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

integer slice overflow

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.