WhenNotPaused modifier in the CDPVault can be bypassed by users

[howlbot-integration]

Lines of code

https://github.com/code-423n4/2024-07-loopfi/blob/57871f64bdea450c1f04c9a53dc1a78223719164/src/CDPVault.sol#L223-L233
https://github.com/code-423n4/2024-07-loopfi/blob/main/src/CDPVault.sol#L239-L249

Vulnerability details

Impact

Users can still execute deposit and withdraw functions when the CDPVault.sol is paused as against the design expectation by just calling the modifyCollateralAndDebt(...) function with the necessary parameters.

Proof of Concept

By design it is expected that when the CDPVault.sol contract is paused by the Dao, the deposits and withdrawals cannot be made due to the whenNotPaused modifier on the deposit and withdraw functions. However, the pause mechanism can be bypassed by users to deposit or withdaraw on the CDPVault contract by directly calling the modifyCollateralAndDebt(...) function since it is public since it is the same function that is called by the deposit and withdraw functions.

Both the desposit and withdraw functions of the CDPVault.sol only make simple calculations before calling the public modifyCollateralAndDebt(...) function. This allows any user to directly call the modifyCollateralAndDebt(...) function when the CDPVault.sol contract is paused.

File: CDPVault.sol
function deposit(address to, uint256 amount) external whenNotPaused returns (uint256 tokenAmount) {
        tokenAmount = wdiv(amount, tokenScale);
        int256 deltaCollateral = toInt256(tokenAmount);
@>        modifyCollateralAndDebt({
            owner: to,
            collateralizer: msg.sender,
            creditor: msg.sender,
            deltaCollateral: deltaCollateral,
            deltaDebt: 0
        });
    }


function withdraw(address to, uint256 amount) external whenNotPaused returns (uint256 tokenAmount) {
        tokenAmount = wdiv(amount, tokenScale);
        int256 deltaCollateral = -toInt256(tokenAmount);
@>        modifyCollateralAndDebt({
            owner: to,
            collateralizer: msg.sender,
            creditor: msg.sender,
            deltaCollateral: deltaCollateral,
            deltaDebt: 0
        });
    }


 function modifyCollateralAndDebt(
        address owner,
        address collateralizer,
        address creditor,
        int256 deltaCollateral,
        int256 deltaDebt
    ) public {
...

  }

Tools Used

Manual Review

Recommended Mitigation Steps

Consider implementing either of the two solutions:

1. make the modifyCollateralAndDebt(...) internal instead of public
   OR
2. add the whenNotPaused modifier to the modifyCollateralAndDebt(...) function

Assessed type

Other

[c4-sponsor]

@amarcu Sponsors are not allowed to close, reopen, or assign issues or pull requests.

[c4-judge]

koolexcrypto marked the issue as satisfactory

[c4-judge]

koolexcrypto marked the issue as selected for report