BathBuddy contract should implement methods to pause and unpause contract

[code423n4]

Lines of code

https://github.com/code-423n4/2023-04-rubicon/blob/main/contracts/periphery/BathBuddy.sol#L38

Vulnerability details

Impact

The BathBuddy contract inherits from OpenZeppelin Pausable contract with the intention of adding pausing features to the contract.

https://github.com/code-423n4/2023-04-rubicon/blob/main/contracts/periphery/BathBuddy.sol#L38

38: contract BathBuddy is ReentrancyGuard, IBathBuddy, Pausable {

The Pausable implementation contains all the logic to implement pausing, but doesn't include any external or public functionality to actually trigger the pause or resume, this task is left to the derived contract.

The BathBuddy contract fails to implement these functions, as there is no callable function from the outside that modifies the pause state. The pausing mechanism is intended to be used in the getReward function, as this function includes the whenNotPaused:

https://github.com/code-423n4/2023-04-rubicon/blob/main/contracts/periphery/BathBuddy.sol#L168-L185

168:     function getReward(
169:         IERC20 rewardsToken,
170:         address holderRecipient
171:     )
172:         external
173:         override
174:         nonReentrant
175:         whenNotPaused
176:         updateReward(holderRecipient, address(rewardsToken))
177:         onlyBathHouse
178:     {
179:         uint256 reward = tokenRewards[address(rewardsToken)][holderRecipient];
180:         if (reward > 0) {
181:             tokenRewards[address(rewardsToken)][holderRecipient] = 0;
182:             rewardsToken.safeTransfer(holderRecipient, reward);
183:             emit RewardPaid(holderRecipient, reward);
184:         }
185:     }

This means that protocol admin won't be able to pause this function if needed as there is no accessible function to enable the mechanism at all.

Proof of Concept

The BathBuddy contract code doesn't include any external or public function to pause or resume the contract. The OpenZeppelin implementations only contains internal functions to provide support so that the derived contract implements the public interface:

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/security/Pausable.sol#L82-L104

/**
  * @dev Triggers stopped state.
  *
  * Requirements:
  *
  * - The contract must not be paused.
  */
function _pause() internal virtual whenNotPaused {
    _paused = true;
    emit Paused(_msgSender());
}

/**
  * @dev Returns to normal state.
  *
  * Requirements:
  *
  * - The contract must be paused.
  */
function _unpause() internal virtual whenPaused {
    _paused = false;
    emit Unpaused(_msgSender());
}

Recommendation

The BathBuddy contract should implement the functions to expose the pausing mechanism. These functions should only be accessible to the owner of the contract.

function pause() external onlyOwner {
    _pause();
}

function unpause() external onlyOwner {
    _unpause();
}

[c4-pre-sort]

0xSorryNotSorry marked the issue as primary issue

[c4-sponsor]

daoio marked the issue as sponsor confirmed

[c4-judge]

HickupHH3 marked the issue as selected for report