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

1 stars 0 forks source link

## [M-07] ERC20 return values not checked #344

Closed code423n4 closed 11 months ago

code423n4 commented 12 months ago

Lines of code

https://github.com/code-423n4/2023-07-moonwell/blob/4aaa7d6767da3bc42e31c18ea2e75736a4ea53d4/src/core/Comptroller.sol#L965 https://github.com/code-423n4/2023-07-moonwell/blob/4aaa7d6767da3bc42e31c18ea2e75736a4ea53d4/src/core/Comptroller.sol#L967

Vulnerability details

Impact

Tokens that don’t actually perform the transfer and return false are still counted as a correct transfer and the tokens remain in the SingleNativeTokenExitV2 contract and could potentially be stolen by someone else.

Proof of Concept

The ERC20.transfer() and ERC20.transferFrom() functions return a boolean value indicating success. This parameter needs to be checked for success. Some tokens do not revert if the transfer failed but return false instead.

Tools Used

Recommended Mitigation Steps

We recommend using OpenZeppelin’sSafeERC20 versions with thesafeTransfer andsafeTransferFrom functions that handle the return value check as well as non-standard-compliant tokens.

Assessed type

ERC20

c4-pre-sort commented 11 months ago

0xSorryNotSorry marked the issue as primary issue

ElliotFriedman commented 11 months ago

this was specifically marked as out of scope in the contest description

Comptroller _rescueFunds function does not check the return value of transfer. This is expected.

https://github.com/code-423n4/2023-07-moonwell/blob/4aaa7d6767da3bc42e31c18ea2e75736a4ea53d4/README.md

c4-sponsor commented 11 months ago

ElliotFriedman marked the issue as sponsor disputed

alcueca commented 11 months ago

Out of scope

c4-judge commented 11 months ago

alcueca marked the issue as unsatisfactory: Invalid