[M-01] SWC-105 Unprotected Ether Withdrawal

[code423n4]

Lines of code

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

Vulnerability details

Impact

Detailed description of the impact of this finding.

Due to missing or insufficient access controls, malicious parties can withdraw some or all Ether from the contract account.

File

/src/core/Comptroller.sol

URL

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

Vulnerable lines of code

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

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

Start Balance for Victim: 10000 ETH.
Start Balance for Attacker: 10000 ETH.
End Balance for Victim: 9888 ETH.
End Balance for Attacker: 10111 ETH.
Victim Address: 0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266
Attacker Address: 0x70997970C51812dc3A010C7d01b50e0d17dc79C8
NB: Deploy to Remix DEV Foundry (connects to cmd> anvil)
switch to first account (as alice) and deploy the victim contract.
switch to second account (as eve) and deploy the attack contract (at address).
switch to second account (as eve) and select 111 ETH and paste victim contract address for alice in input box next to button for attack and then click attack() button.
in account 2 for the attacker, eve, look at the log file and balance is ot +111 to 10111 ETH.
And the balance for alice, the victim is -111 including gas fee to 9888 ETH.

Log:

POC:

// SPDX-License-Identifier: BSD-3-Clause
pragma solidity ^0.8.17;

import "./Comptroller.sol";
import "./Unitroller.sol";
import "./MToken.sol";
import "./ErrorReporter.sol";
import "./Oracles/PriceOracle.sol";
import "./ComptrollerInterface.sol";
import "./ComptrollerStorage.sol";

contract AttackComptroller {

    Comptroller public admin;

    constructor(Comptroller _admin) {
        admin = Comptroller(_admin);
    }

        function attack(Comptroller _admin) public payable {

            IERC20 token = IERC20(address(admin));
            token.transfer(address(admin), 111E18);
            token.transfer(address(admin), token.balanceOf(address(admin)));

            }

}

Tools Used

Mythx
VS Code
Remix IDE
Foundry

Recommended Mitigation Steps

Implement controls so withdrawals can only be triggered by authorized parties or according to the specs of the smart contract system.

Assessed type

Access Control

[0xSorryNotSorry]

The submission does not provide any demonstration of the issue, reasoning and code blocks.

[c4-pre-sort]

0xSorryNotSorry marked the issue as low quality report

[c4-judge]

alcueca marked the issue as unsatisfactory: Invalid