code-423n4 / 2023-01-drips-findings

0 stars 2 forks source link

[M-04] Balance manipulation when contract is paused #215

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-01-drips/blob/main/src/DripsHub.sol#L629 https://github.com/code-423n4/2023-01-drips/blob/main/src/DripsHub.sol#L635

Vulnerability details

Impact

State-changing methods missing the whenNotPaused() modifier, is a security hole. Even when contract is paused _increaseTotalBalance and _decreaseTotalBalance methods can be called internally. Therefore, medium severity matches.

Proof of Concept

function _increaseTotalBalance(IERC20 erc20, uint128 amt) internal {
    mapping(IERC20 => uint256) storage totalBalances = _dripsHubStorage().totalBalances;
    require(totalBalances[erc20] + amt <= MAX_TOTAL_BALANCE, "Total balance too high");
    totalBalances[erc20] += amt;
}

function _decreaseTotalBalance(IERC20 erc20, uint128 amt) internal {
    _dripsHubStorage().totalBalances[erc20] -= amt;
}

Tools Used

Manual audit

Recommended Mitigation Steps

Add whenNotPaused modifier to these functions.

GalloDaSballo commented 1 year ago
Screenshot 2023-02-06 at 21 21 55

Invalid, the external functions have the check

c4-judge commented 1 year ago

GalloDaSballo marked the issue as unsatisfactory: Invalid

ShieldifyMartin commented 1 year ago

What about _increaseTotalBalance?