code-423n4 / 2022-12-backed-findings

1 stars 3 forks source link

SOLMATE SAFETRANSFER AND SAFETRANSFERFROM DOES NOT CHECK THE CODESIZE OF THE TOKEN ADDRESS, WHICH MAY LEAD TO FUND LOSS #269

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/with-backed/papr/blob/9528f2711ff0c1522076b9f93fba13f88d5bd5e6/src/PaprController.sol#L253 https://github.com/with-backed/papr/blob/9528f2711ff0c1522076b9f93fba13f88d5bd5e6/src/PaprController.sol#L383

Vulnerability details

Impact

Reference from a previous Contrest : https://code4rena.com/reports/2022-08-olympus/#m-02-solmate-safetransfer-and-safetransferfrom-does-not-check-the-codesize-of-the-token-address-which-may-lead-to-fund-loss

In uniswapV3SwapCallback() and sendPaprFromAuctionFees() the safetransfer and safetransferfrom doesn’t check the existence of code at the token address. This is a known issue while using solmate’s libraries.
Hence this may lead to miscalculation of funds and may lead to loss of funds , because if safetransfer() and safetransferfrom() are called on a token address that doesn’t have contract in it, it will always return success, bypassing the return value check. Due to this protocol will think that funds has been transferred and successful , and records will be accordingly calculated, but in reality funds were never transferred.
So this will lead to miscalculation and possibly loss of funds

Proof of Concept

https://github.com/with-backed/papr/blob/9528f2711ff0c1522076b9f93fba13f88d5bd5e6/src/PaprController.sol#L253 https://github.com/with-backed/papr/blob/9528f2711ff0c1522076b9f93fba13f88d5bd5e6/src/PaprController.sol#L383

Recommended Mitigation Steps

Use openzeppelin’s safeERC20 or implement a code existence check.

c4-judge commented 1 year ago

trust1995 marked the issue as unsatisfactory: Insufficient quality