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

`raw_call` allows for kwarg `value` if `is_delegate_call` is `True`

pcaversaccio

Summary

The Vyper built-in function raw_call allows for the kwarg value if is_delegate_call is True. The Vyper compiler must throw in this context as the opcode DELEGATECALL does not support any CALLVALUE. Or in other words, the kwarg value is not added to the IR/bytecode.

Vulnerability Details

Let's make a simple example contract:

# pragma version 0.3.10
​
@external
@payable
def test(target: address) -> Bytes[32]:
success: bool = False
raw_data: Bytes[32] = b""
x: uint256 = 123
success, raw_data = raw_call(target, _abi_encode(x, method_id=method_id("foo(uint256)")), max_outsize=32,\
value=msg.value, is_delegate_call=True, revert_on_failure=False)
assert success
return raw_data

The issue here now is that value=msg.value is allowed while is_delegate_call=True is also enabled. As comparison, the Solidity compiler would correctly throw in this context with TypeError: Cannot set option "value" for delegatecall. If you check the compiler internals (e.g. here), you can see the following correct behaviour:

if delegate_call:
call_op = ["delegatecall", gas, to, *common_call_args]
elif static_call:
call_op = ["staticcall", gas, to, *common_call_args]
else:
call_op = ["call", gas, to, value, *common_call_args]

Thus, unfortunately, the compiler implies that this is correct behaviour and people could assume that msg.value is forwarded (which is not true).

Impact

The impact is that people might send ETH to the contract thinking the raw_call will forward it (from what they can read in the the code (the kwarg)) while instead the ETH stays here. In theory, this could be even used to create a honeypot with verified contracts, implying the users that if they send ETH to the main contract, that it gets forwarded to do some logic, but actually it stays within the proxy and the exploiter can simply withdraw it.

As a simple illustration why it can be problematic I deployed the above code to Goerli: https://goerli.etherscan.io/address/0xb2a04a1e78efa0b46e9e230cb690c9f2a23bf34c#code. I conduct here a simple test tx: https://goerli.etherscan.io/tx/0xbb9ba4b6ea43f39aa80239f2a05fb59039f9d973f9b008774669e378bbaeebe1, where I call the function test on target being an EOA, for which I know that it will return success = True and I could assume that the 0.2 ETH gets forwarded to the EOA. However, this is not the case, and the 0.2 ETH stays within the contract.

It's important to note that I haven't found any on-chain vulnerable contracts fortunately.

A more sophisticated example is where you use a multicall function combined with delegatecall:

# pragma version 0.3.10
​
​
struct BatchValueSelf:
allow_failure: bool
value: uint256
call_data: Bytes[max_value(uint16)]
​
​
struct Result:
success: bool
return_data: Bytes[max_value(uint8)]
​
​
@external
@payable
def multicall_value_self(data: DynArray[BatchValueSelf, max_value(uint8)]) -> DynArray[Result, max_value(uint8)]:
value_accumulator: uint256 = empty(uint256)
results: DynArray[Result, max_value(uint8)] = []
return_data: Bytes[max_value(uint8)] = b""
success: bool = empty(bool)
for batch in data:
msg_value: uint256 = batch.value
value_accumulator = unsafe_add(value_accumulator, msg_value)
if (batch.allow_failure == False):
return_data = raw_call(self, batch.call_data, max_outsize=255, value=msg_value, is_delegate_call=True)
success = True
results.append(Result({success: success, return_data: return_data}))
else:
success, return_data = \
raw_call(self, batch.call_data, max_outsize=255, value=msg_value, is_delegate_call=True, revert_on_failure=False)
results.append(Result({success: success, return_data: return_data}))
assert msg.value == value_accumulator, "Multicall: value mismatch"
return results

The line msg_value: uint256 = batch.value is implying a wrong msg.value for the raw_call call.

Tools Used

My brain :)

Recommendations

The Vyper compiler must disallow the combination of the kwarg value and is_delegate_call=True.

Updates

Lead Judging Commences

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

`raw_call` allows for kwarg `value` if `is_delegate_call` is `True`

Support

FAQs

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