Summary

The bounds check for slices does not account for the ability for start + length to overflow when the start value is not a literal. This creates the ability for an attacker to overflow the bounds check to use the slice() built-in to access either (a) an unrelated storage slot or (b) the previous word of memory.

Vulnerability Details

When calling slice() there are compile time bounds checks if the start and length values are literals, but of course this cannot happen if they are passed values:

At runtime, we perform the following equivalent check, but the runtime check does not account for overflows:

This same issue exists if the bytestring being sliced is in memory or storage:

The storage slice() function copies bytes directly from storage into memory and returns the memory value of the resulting slice. This means that, if a user is able to input the start value, they can force an overflow and access an unrelated storage slot. In most cases, this will mean they have the ability to forceably return 0 for the slice, even if this shouldn't be possible. In extreme cases, it will mean they can return another unrelated value from storage.

The memory slice() function returns the memory value of the resulting slice. There is a check as part of the process that start + 32 < length, which means that for the overflow to be possible start must be greater than max uint256 - 31. As a result, the returned slice can be any slice starting up to 32 bytes before the variable that is being sliced.

Proof of Concept

For simplicity, take the following Vyper contract, which takes an argument to determine where in a Bytes[64] bytestring should be sliced. It should only accept a value of zero, and should revert in all other cases.

We can use the following manual storage to demonstrate the vulnerability:

If we run the following test, passing max - 63 as the start value, we will overflow the bounds check, but access the storage slot at 1 + (2**256 - 63) / 32, which is what was set in the above storage layout:

The result is that we return the secret value from storage:

For a memory slice, see the following contract:

If we pass max uint256 - 31 as the start, we will be returned 2 (the length of the bytestring). If we pass max uint256 - 30, we will be returned 205 (the length plus the first element of the left aligned bytestring). If we pass max uint256 - 29, we will be returned 20505, etc.

Impact

The built-in slice() method can be used to read unrelated storage slots or memory locations by abusing a bounds check overflow.

Tools Used

Manual Review, Foundry

Recommendations

Update the bounds check to also include a check that start + length > start to ensure no overflow is possible.