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

[Bug] contract reports ether as locked when ether is sent in Yul #1686

Closed
0xalpharush opened this issue Feb 24, 2023 · 1 comment
Closed
Labels
bug Something isn't working yul

Comments

@0xalpharush
Copy link
Contributor

The following contract reports ether as locked despite it being sent in a Yul block

contract FPLockedEther {
  receive() payable external {}

  function yulSendEther() external {
    bool success;
    assembly {
      success := call(gas(), caller(), balance(address()), 0,0,0,0)
    }
  }
}
Contract locking ether found:
	Contract FPLockedEther (locked-ether.sol#1-13) has payable functions:
	 - FPLockedEther.receive() (locked-ether.sol#2-3)
	But does not have a function to withdraw the ether
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#contracts-that-lock-ether

It could be that the IR is incorrect here as it should not be a SOLIDITY_CALL

Contract FPLockedEther
	Function FPLockedEther.receive() (*)
	Function FPLockedEther.yulSendEther() (*)
		Expression: success = call(uint256,uint256,uint256,uint256,uint256,uint256,uint256)(gas()(),caller()(),balance(uint256)(address()()),0,0,0,0)
		IRs:
			TMP_0(uint256) = SOLIDITY_CALL gas()()
			TMP_1(address) := msg.sender(address)
			TMP_2 = CONVERT this to address
			TMP_3(uint256) = SOLIDITY_CALL balance(uint256)(TMP_2)
			TMP_4(uint256) = SOLIDITY_CALL call(uint256,uint256,uint256,uint256,uint256,uint256,uint256)(TMP_0,TMP_1,TMP_3,0,0,0,0)
			success(bool) := TMP_4(uint256)
@0xalpharush 0xalpharush added bug Something isn't working yul labels Feb 24, 2023
@bart1e
Copy link
Contributor

bart1e commented Mar 1, 2023

The following code seems to cause this bug:

else:
ident = Identifier(SolidityFunction(format_function_descriptor(ident.value.name)))

It probably should be LowLevelCall instead. The problem is, that LowLevelCall requires destination in its constructor to be of type Union[LocalVariable, LocalIRVariable, TemporaryVariableSSA, TemporaryVariable], but at the point the code is invoked, none of these variable types is created (we only have CallExpressions, at least for the provided yulSendEther implementation).

I may try to fix this issue, but I need more information about it:

  • should I use LowLevelCall here?
  • if I should, then what to provide as destination and result parameters?

So far, LowLevelCall objects are only created using convert_to_low_level invoked on a HighLevelCall object, so they aren't created for Yul code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working yul
Projects
None yet
Development

No branches or pull requests

2 participants