code-423n4 / 2022-03-lifinance-findings

6 stars 4 forks source link

low-level calls returning success although it doesn't succeeded #47

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Libraries/LibSwap.sol#L42-L46 https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Libraries/LibAsset.sol#L49-L50

Vulnerability details

Impact

Low level calls (call, delegate call and static call) return success if the called contract doesn’t exist (not deployed or destructed).

That means that in LibSwap, if _swapData.callTo doesn't exist the call will fail but success will be set to true, which will act like the call was successful.

This bug can also be seen in the transferNativeAsset function of the LibAsset library:

function transferNativeAsset(address payable recipient, uint256 amount) internal {
    // solhint-disable-next-line avoid-low-level-calls
    (bool success, ) = recipient.call{ value: amount }("");
    require(success, "#TNA:028");
}

Proof of Concept

Tools Used

Remix & VS Code

Recommended Mitigation Steps

Check address existence before the low level call

H3xept commented 2 years ago

Duplicate of #101