code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/Heart.sol#L106 https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/policies/Heart.sol#L112 https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/libraries/TransferHelper.sol#L23

Vulnerability details

Impact

If the Heart.sol contract's balance of rewardToken ever falls below the reward amount, the heartbeat will fail and the full protocol's metrics will be thrown off.

Proof of Concept

The beat() function contains a call to _issueReward(msg.sender).

This function uses the TransferHelper library to call rewardToken.safeTransfer(to_, reward);.

In the helper function, the transfer is made using a low level call and fails unless the call returns a success boolean.

(bool success, bytes memory data) = address(token).call(
    abi.encodeWithSelector(ERC20.transfer.selector, to, amount)
);

require(success && (data.length == 0 || abi.decode(data, (bool))), "TRANSFER_FAILED");

As a result, the full beat() function will fail.

Tools Used

VS Code

Recommended Mitigation Steps

Update the _issueReward() function to check the reward amount and the current balance, and send the minimum value out of these two options.

function _issueReward(address to_) internal {
    uint bal = rewardToken.balanceOf(address(this));
    uint toSend = reward > bal ? bal : reward;
    rewardToken.safeTransfer(to_, toSend);
    emit RewardIssued(to_, reward);
}

Oighty commented 2 years ago

Keepers will be able to check the contract balance and the reward amount prior to calling the contract to avoid failed transactions as a result of a low reward balance. We acknowledge that the fails could happen, but that is typical for reward contracts which do not mint tokens.

Oighty commented 2 years ago

After reviewing #378 , I changed my mind about this one due to the anti-DOS characteristics of using a min implementation.

0xean commented 2 years ago

closing as duplicate of #378

code-423n4 / 2022-08-olympus-findings