Olympus DAO contest - sorrynotsorry's results

Lines of code

https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/modules/TRSRY.sol#L64-L72

Vulnerability details

Impact

Multiple Withdrawal Attack Vector in TRSRY.sol

Proof of Concept

The TRSRY.sol has setApprovalFor method to set approval for specific withdrawer addresses. The function is as below;

function setApprovalFor(
        address withdrawer_,
        ERC20 token_,
        uint256 amount_
    ) external permissioned {
        withdrawApproval[withdrawer_][token_] = amount_;

        emit ApprovedForWithdrawal(withdrawer_, token_, amount_);
    }

https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/modules/TRSRY.sol#L64-L72

However, a malicious/compromised withdrawer may monitor the mempool for this new setting and can frontrun it by withdrawing the existing approval amount.So the account will have a fresh approval and a executed withdrawal.

Tools Used

Manual Review

Recommended Mitigation Steps

The team migh consider to add require(withdrawApproval[withdrawer_][token_] = 0);, which will prevent accounts from hitting the problem.

Lines of code

https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/modules/TRSRY.sol#L122-L135

Vulnerability details

Impact

setDebt function of TRSRY.sol has no comparison of financial values and sets/erase debts for arbitrary addresses for arbitrary amounts.

Proof of Concept

The TRSRY.sol has setDebt function to renew the debtof an address to a new ERC20 token in special cases. However, it's not clear or in guidance to which value it will be renewed. It only compares the amount of both tokens only. It can be abused very easily once any human error is engaged by miscalculation.One more concern is that the function can erase/increase any debt of any addres arbitrarily.

The function is as below;

function setDebt(
        ERC20 token_,
        address debtor_,
        uint256 amount_
    ) external permissioned {
        uint256 oldDebt = reserveDebt[token_][debtor_];

        reserveDebt[token_][debtor_] = amount_;

        if (oldDebt < amount_) totalDebt[token_] += amount_ - oldDebt;
        else totalDebt[token_] -= oldDebt - amount_;

        emit DebtSet(token_, debtor_, amount_);
    }

https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/modules/TRSRY.sol#L122-L135

Tools Used

Manual Review

Recommended Mitigation Steps

The team migh consider to add more checks/conditions to minimize the risk.