extendPromotion function should be access controlled by using onlyPromotionCreator

[code423n4]

Handle

hubble

Vulnerability details

Impact

In the current code, if any other address or user is extending the promotion via the extendPromotion, then there is a possibility of the original Promoter to cancel the promotion and take the remaining Rewards.

Proof of Concept

contract : TwabRewards line 141-145 : function extendPromotion(uint256 _promotionId, uint8 _numberOfEpochs) external override returns (bool) {

Tools Used

Manual review

Recommended Mitigation Steps

Add modifier as below, so that only the promoter can extend the promotion

line 141 : function extendPromotion(uint256 _promotionId, uint8 _numberOfEpochs) external override onlyPromotionCreator(_promotionId) returns (bool) {

[PierrickGT]

This is a design decision that we have taken. People calling this function without being the creator of the promotion they want to extend should know that they are giving out tokens that they can't get back. For this reason, I've acknowledged the issue but we won't actually make any changes.

[PierrickGT]

I also consider this not being a bug and no funds are directly at loss since people will still be able to claim their rewards. So it should be labelled as 1 (Low Risk) instead of 2 (Med Risk).

[dmvt]

I view this as a clarity issue, not a vulnerability. The expectation when extending a promotion is that the funds will be used for that promotion or for the benefit of the protocol. While the creator of the promotion could theoretically rug the promotion extending user, the net effect is the same, the extending user doesn't have the funds. I'm leaving this in place so that anyone doing due diligence by reading the audit will see that the protocol functions this way but don't see it as an abnormal risk or commenting issue at all. It is the design of the protocol.