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

1 stars 0 forks source link

Lack of Transfer Check in ERC20 Operations #303

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

It is not safe to use the ERC20 transfer function without checking the results. Use a safe transfer library like OpenZeppelin SafeERC20 to ensure consistent handling of ERC20 return values and abstract over inconsistent ERC20 implementations.

Proof of Concept

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

965:            token.transfer(admin, token.balanceOf(address(this)));

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

967:            token.transfer(admin, _amount);

Tools Used

Manual review

Recommended Mitigation Steps

Use a safe transfer library like OpenZeppelin SafeERC20 to check the values

Assessed type

ERC20

c4-pre-sort commented 1 year ago

0xSorryNotSorry marked the issue as duplicate of #344

c4-judge commented 1 year ago

alcueca marked the issue as unsatisfactory: Invalid