search

code-423n4 / 2021-12-pooltogether-findings

0 stars 0 forks source link

safeTransferFrom call inconsistency #124

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

sirhashalot

Vulnerability details

Impact

The safeTransferFrom function is called twice, but in different ways. When a promotion is create, the product _tokensPerEpoch * _numberOfEpochs is placed in the function call directly. In contrast, when a promotion is extended, this product is stored in a variable _amount and the variable is placed in the function call.

Proof of Concept

The createPromotion function safeTransferFrom call: https://github.com/pooltogether/v4-periphery/blob/b520faea26bcf60371012f6cb246aa149abd3c7d/contracts/TwabRewards.sol#L111 The extendPromotion function safeTransferFrom call: https://github.com/pooltogether/v4-periphery/blob/b520faea26bcf60371012f6cb246aa149abd3c7d/contracts/TwabRewards.sol#L153-L154

Recommended Mitigation Steps

Keep code consistent by using the same approach in both calls of safeTransferFrom

PierrickGT commented 2 years ago

This isn't a bug and we actually save on gas by not storing _tokensPerEpoch * _numberOfEpochs in a variable if we don't plan to reuse it. For this reason, I've disputed the issue.

dmvt commented 2 years ago

Agree with sponsor