code-423n4 / 2023-07-tapioca-findings

13 stars 9 forks source link

borrowInternal() of BaseTOFTMarketModule.sol has phantom permit functions #1677

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/Tapioca-DAO/tapiocaz-audit/blob/master/contracts/tOFT/modules/BaseTOFTMarketModule.sol#L293-#L303

Vulnerability details

Impact

A malicious actor could steal funds from a User who has already done his first deposit.

Proof of Concept

Consider the case where attacker uses a token with phantom permit function as collateral, the most famous ones being WETH, BNB, HEX etc. Let’s consider WETH in this example.
Attack Scenario: Attacker calls borrowInternal() of BaseTOFTMarketModule.sol, with carefully inputted approvals parameters to skip the first 2 “if” conditions of _callApproval function. Subsequently, performing IERC20Permit(approvals[i].target).permit() functionality with Attacker inputted variables where attacker inputs approvals[i].owner to an account who has already deposited some tokens through the functionality in order BaseTOFTMarketModule.sol:borrowInternal()->MagnetarMarketModule.sol: _depositAddCollateralAndBorrowFromMarket -> MagnetarMarketModule.sol: _extractTokens

Thereby confirming Users approval of the token for the MagnetarMarketModule.sol

3) As Weth doesn’t have a permit function , permit() call will get directed to it’ fallback
function. Weth has a fallback functionality as below:


function() public payable { 

deposit(); 

} 

function deposit() public payable { 

balanceOf[msg.sender] += msg.value; 

Deposit(msg.sender, msg.value); 

} 

This function is called when receiving ETH (and just deposits it, to mint wrapped ETH with it) but, crucially, is also called when an undefined function is invoked on the WETH contract. All WETH of all such users can be stolen and deposited onBehalf of the Attacker.

Another Vulnerability of the Tokens with Phantom Permits: Unexpected reverts are caused by IERC20Permit(approvals[i].target).permit() by tokens like stETH which does not have a permit function.

Tools Used

Manual Review

Reference

https://media.dedaub.com/phantom-functions-and-the-billion-dollar-no-op-c56f062ae49f

Recommended Mitigation Steps

Setting a precedent of explicitly reverting when calling unsupported permit functions can help ensure Protocol and it’s Trusted Users do not suffer from calling phantom permits.

Assessed type

Other

minhquanym commented 1 year ago

Insufficient proof

c4-pre-sort commented 1 year ago

minhquanym marked the issue as low quality report

c4-judge commented 1 year ago

dmvt marked the issue as unsatisfactory: Insufficient proof