Wise Lending - unix515's results

Lines of code

https://github.com/code-423n4/2024-02-wise-lending/blob/main/contracts/TransferHub/CallOptionalReturn.sol#L12-L38

Vulnerability details

Description

1. CallOptionalReturn#_callOptionalReturn() is not reverted and returns false even if the low level call returned false.

function _callOptionalReturn(
	address token,
	bytes memory data
)
	internal
	returns (bool call)
{
	(
		bool success,
		bytes memory returndata   ///<-------------- @audit
	) = token.call(          
		data
	);
        
	bool results = returndata.length == 0 || abi.decode(  ///<------------ @audit
		returndata,
		(bool)
	);
	...
	call = success       
		&& results         ///<--------------- @audit
		&& token.code.length > 0;
}

2. The following functions does not check if CallOptionalReturn#_callOptionalReturn() returned false.
   • TransferHelper#_safeTransfer()
   • TransferHelper#_safeTransferFrom()
   • ApprovalHelper#_safeApprove()

function _safeTransfer(
	address _token,
	address _to,
	uint256 _value
)
	internal
{
	_callOptionalReturn(    ///<-------------- @audit
		_token,
		abi.encodeWithSelector(
			IERC20.transfer.selector,
			_to,
			_value
		)
	);
}

Impact

So even if the input _token's transfer() returned false, the following functions are not reverted and works as if calling were successful .

• TransferHelper#_safeTransfer()
• TransferHelper#_safeTransferFrom()
• ApprovalHelper#_safeApprove()

This may result in some funds being recorded as transferred in the ledger but not actually transferred.

Reference

As reference, SafeERC20#_callOptionalReturn() of openzeppelin is reverted with SafeERC20FailedOperation error when the low level call returned false.

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC20/utils/SafeERC20.sol#L146-L155

function _callOptionalReturn(IERC20 token, bytes memory data) private {
	bytes memory returndata = address(token).functionCall(data);
	if (returndata.length != 0 && !abi.decode(returndata, (bool))) {  ///<---- @audit
		revert SafeERC20FailedOperation(address(token));
	}
}

Proof of Concept

• Assumption
  1. Let's assume one token _poolToken's transfer() returns false (not reverted) if it fell in some circumstances.
  2. Let's assume one user Bob has some amount of poolToken.
• Scenario

  1. Bob calls WiseLending#withdrawExactAmount() to withdraw some amount of _poolToken.

  2. WiseLending#withdrawExactAmount() updates the size of pool with the following internal call flows.

     • WiseLending#_handleWithdrawAmount() ->
     • WiseCore#_coreWithdrawToken() ->
     • WiseCore#_coreWithdrawBare()

     function _coreWithdrawBare(
     	uint256 _nftId,
     	address _poolToken,
     	uint256 _amount,
     	uint256 _shares
     )
     	private
     {
     	_updatePoolStorage(
     		_poolToken,
     		_amount,             ///<---------------- @audit
     		_shares,
     		_decreaseTotalPool,         ///<---------------- @audit
     		_decreasePseudoTotalPool,   ///<---------------- @audit
     		_decreaseTotalDepositShares
     	);
     	...
     }
     

  3. WiseLending#withdrawExactAmount() then calls TransferHelper#_safeTransfer() to transfer funds to Bob finally.

     function withdrawExactAmount(
     	uint256 _nftId,
     	address _poolToken,
     	uint256 _withdrawAmount
     )
     	external
     	syncPool(_poolToken)
     	healthStateCheck(_nftId)
     	returns (uint256)
     {    
     	...
     	_safeTransfer(     ///<------------------- @audit
     		_poolToken,
     		msg.sender,
     		_withdrawAmount
     	);
     	return withdrawShares;
     }
     

  4. Even if _poolToken's transfer() returned false, TransferHelper#_safeTransfer() is not reverted and then WiseLending#withdrawExactAmount() works as if calling were successful.

  5. So the pool size of poolToken was already updated, but some amount (_withdrawAmount) of _poolToken were not really transferred to Bob.

Tools Used

Manual Review

Recommened Mitigation Steps

Please update CallOptionaReturn#_callOptionalReturn() as following.

function _callOptionalReturn(
	address token,
	bytes memory data
)
	internal
	returns (bool call)
{
	(
		bool success,
		bytes memory returndata
	) = token.call(
		data
	);

	bool results = returndata.length == 0 || abi.decode(
		returndata,
		(bool)
	);

--	if (success == false) {
++	if (success == false || results == false) {
		revert();
	}

	call = success
		&& results
		&& token.code.length > 0;
}

Assessed type

Invalid Validation