016.md

Clean Fossilized Stallion

Medium

notifyRewardAmounts does not properly enforce max brides per week due to incorrect period check

Summary

notifyRewardAmounts keeps track of the current active period to prevent more bribes from being placed than the weekly limit. The problem is that the current active period is not modified correctly and the bride count is reset to 0 every time this function is called.

Root Cause

When notifyRewardAmounts it checks that the current period is greater than the active period. The intention here is to reset the amount of bids for the current period back to 0 as the week has passed, however it will always be reset back to 0 because currentPeriod is always going to be greater than the activePeriod for each subsequent call to this function.

currentPeriod is set to the current block timestamp. If it is greater than activePeriod, the activePeriod is set to the current timestamp. Since active period is just set to whatever the current timestamp is, it will always be less than the current period. Therefore the amount of bribes for the active period will always be reset back to zero and the if condition to check the amount of bribes per week will not accurately represent the amount of bribes in a week span.

 uint256 currentPeriod = (block.timestamp / 1 weeks) * 1 weeks;

        if (currentPeriod > activePeriod) {
            activePeriod = currentPeriod;
            bribeTimes[activePeriod] = 0;
        }

        if (bribeTimes[activePeriod] + bribes.length > bribeTimesPerWeek) revert BribeTimesPerWeekLimitExceeded();

Internal pre-conditions

1. Operator roles are set
2. Contract is funded with the appropriate reward token

External pre-conditions

1. Exchange has valid bribe contracts deployed

Attack Path

1. Operator calls notifyRewardAmounts with bribes
2. Can continuously call it as bribe weekly limit is not properly enforced

Impact

No limit on the amount of bribes per week

PoC

No response

Mitigation

Properly enforce the weekly limit by making the active period 1 week long

if (currentPeriod > activePeriod) {
-            activePeriod = currentPeriod;
+.          activePeriod = currentPeriod + 1 weeks;
            bribeTimes[activePeriod] = 0;
        }