Platform: Code4rena
Start Date: 09/12/2021
Pot Size: $25,000 USDC
Total HM: 12
Participants: 25
Period: 4 days
Judge: LSDan
Total Solo HM: 4
Id: 64
League: ETH
Rank: 18/25
Findings: 2
Award: $313.30
🌟 Selected for report: 2
🚀 Solo Findings: 0
🌟 Selected for report: Meta0xNull
280.3604 USDC - $280.36
Meta0xNull
<code>@notice Cancel currently active promotion and send promotion tokens back to the creator.</code> The Notice Mention Token Send Back to the Creator.
<code>_promotion.token.safeTransfer(_to, _remainingRewards);</code> But in the code, Token Send to _to.
https://github.com/pooltogether/v4-periphery/blob/ceadb25844f95f19f33cb856222e461ed8edf005/contracts/interfaces/ITwabRewards.sol#L56 https://github.com/pooltogether/v4-periphery/blob/b520faea26bcf60371012f6cb246aa149abd3c7d/contracts/TwabRewards.sol#L133
Manual Review
Since the Creator is msg.sender, should Transfer Tokens back to msg.sender. <code>_promotion.token.safeTransfer(msg.sender, _remainingRewards);</code>
In cancelPromotion(), Remove Input address _to.
#0 - PierrickGT
2021-12-10T17:57:23Z
This function is only callable by the promotion creator, the _to address param is here to offer the possibility to the creator to send the remaining funds to an address of his choosing, wether he his in control of it or not.
For this reason, I've disputed the issue.
#1 - dmvt
2022-01-17T10:26:34Z
The sponsor's explanation is valid but also confirms that the code comment is wrong. This is a valid low risk issue.
1 — Low (L): vulns that have a risk of 1 are considered “Low” severity when assets are not at risk. Includes state handling, function incorrect as to spec, and issues with comments.
🌟 Selected for report: Meta0xNull
26.9701 USDC - $26.97
Meta0xNull
<code>require(_to != address(0), "TwabRewards/recipient-not-zero-address");</code>
Check Zero Address Before Function Call eg. _requirePromotionActive() Can Save Gas.
Manual Review
Move Zero Address Check to Line L125: https://github.com/pooltogether/v4-periphery/blob/b520faea26bcf60371012f6cb246aa149abd3c7d/contracts/TwabRewards.sol#L125
🌟 Selected for report: robee
Also found by: GiveMeTestEther, Jujic, Meta0xNull, WatchPug, defsec, sirhashalot, ye0lde
Meta0xNull
Shortening revert strings to fit in 32 bytes will decrease deployment time gas and will decrease runtime gas when the revert condition has been met.
Revert strings that are longer than 32 bytes require at least one additional mstore, along with additional overhead for computing memory offset, etc.
https://github.com/pooltogether/v4-periphery/blob/b520faea26bcf60371012f6cb246aa149abd3c7d/contracts/TwabRewards.sol#L80 https://github.com/pooltogether/v4-periphery/blob/b520faea26bcf60371012f6cb246aa149abd3c7d/contracts/TwabRewards.sol#L128 https://github.com/pooltogether/v4-periphery/blob/b520faea26bcf60371012f6cb246aa149abd3c7d/contracts/TwabRewards.sol#L177 https://github.com/pooltogether/v4-periphery/blob/b520faea26bcf60371012f6cb246aa149abd3c7d/contracts/TwabRewards.sol#L231
Manual Review
Shorten the revert strings to fit in 32 bytes.
Or consider using Custom Errors (solc >=0.8.4).
#0 - PierrickGT
2021-12-10T17:30:18Z