Updates
160,000 USDC
View results
Submission Details
Severity: medium
Valid

Contract interfaces allow nonpayable implementations of payable functions

Summary

When a contract interface is implemented, the compiler checks that each function in the interface has a corresponding public function in the contract. However, it does not check that the functions have the same visibility, which can lead to dangerous situations.

Vulnerability Details

When performing semantic analysis on a contract that implements an interface, the compiler calls type_.validate_implements(node) to confirm that the interface is correctly implemented.

This function iterates through all public functions on the interface, checks that we have implemented a function with the same name, and then verifies that all the arguments and return types are of the same type. Finally, it checks that the state mutability of our function is not greater than the interface.

def implements(self, other: "ContractFunctionT") -> bool:
"""
Checks if this function implements the signature of another
function.
Used when determining if an interface has been implemented. This method
should not be directly implemented by any inherited classes.
"""
if not self.is_external:
return False
arguments, return_type = self._iface_sig
other_arguments, other_return_type = other._iface_sig
if len(arguments) != len(other_arguments):
return False
for atyp, btyp in zip(arguments, other_arguments):
if not atyp.compare_type(btyp):
return False
if return_type and not return_type.compare_type(other_return_type): # type: ignore
return False
if self.mutability > other.mutability:
return False
return True

If we look at the mutability enum, we can see that "greater than" represents a less restrictive mutability:

class StateMutability(_StringEnum):
PURE = _StringEnum.auto()
VIEW = _StringEnum.auto()
NONPAYABLE = _StringEnum.auto()
PAYABLE = _StringEnum.auto()

This means that, although we cannot take a view function on the interface and implement it as a nonpayable function, we can do the inverse and implement any function as a more restrictive type.

While for some types this may make sense, it can lead to problems with payable functions.

Interfaces are intended to define the behavior that is required for a contract to perform. If an interface defines a function as payable, it is safe for interacting contracts to send ETH to that function. However, if a contract that implements that interface changes that function to nonpayable (or to view), it could cause the interacting contracts to revert.

Impact

Contracts that Vyper considers to be correctly implementing an interface may not reflect the expectations of the interface, and interacting contracts may end up locked because they expect to be able to send ETH to a function that is not payable.

Note that Solidity has a similar check that "lower" mutabilities are acceptable when implementing an interface, but has a specific carveout for payable functions to avoid this risk. See the table below for a breakdown of the similarities and differences.

------------------------- Solidity ------------ Vyper
view => nonpayable NO NO ✓
view => payable NO NO ✓
nonpayable => view/getter YES YES ✓
nonpayable => payable NO NO ✓
payable => view/getter NO YES <== this is the issue
payable => nonpayable NO YES <== this is the issue

Tools Used

Manual Review

Recommendations

In the implements() function, check whether the mutability of the function on the interface is payable. If it is, require the implementing contract to make the function payable as well.

Updates

Lead Judging Commences

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

interface payable ignored

check with charles

Support

FAQs

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