Platform: Code4rena
Start Date: 22/02/2022
Pot Size: $30,000 USDC
Total HM: 1
Participants: 22
Period: 3 days
Judge: leastwood
Id: 93
League: ETH
Rank: 13/22
Findings: 1
Award: $131.19
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: CertoraInc
Also found by: Dravee, Rhynorater, chunter, gzeon, jayjonah8, robee
131.1916 USDC - $131.19
https://github.com/pooltogether/v4-twab-delegator/blob/master/contracts/TWABDelegator.sol#L188 https://github.com/pooltogether/v4-twab-delegator/blob/master/contracts/TWABDelegator.sol#L204
In TWABDelegator.sol the stake() and unstake() functions handle the minting and burning of tickets to and from the caller. The _to address argument on both is the address to which the stake will be attributed but both functions do not make sure that the _to argument is not the TWABDelegator.sol contract itself. These checks should be added to avoid leaving open areas in which the protocol may be manipulated.
https://github.com/pooltogether/v4-twab-delegator/blob/master/contracts/TWABDelegator.sol#L188
https://github.com/pooltogether/v4-twab-delegator/blob/master/contracts/TWABDelegator.sol#L204
Manual code review
Add to stake() and unstake() functions: require(_to != address(this), "Cannot be TwabDelegator");
#0 - PierrickGT
2022-03-02T16:17:20Z
#1 - 0xleastwood
2022-03-07T11:54:49Z
I'm not convinced this should be rated as medium severity. The warden has not justified why this might cause the protocol issues. I think this could be better rated as low or non-critical. @PierrickGT
#2 - PierrickGT
2022-03-08T15:44:02Z
I'm not convinced this should be rated as
mediumseverity. The warden has not justified why this might cause the protocol issues. I think this could be better rated aslowornon-critical. @PierrickGT
I agree. Per Brendan comment, we are not going to implement this suggestion since only users interacting directly with the contract could make this mistake. So I've added the sponsor acknowledged badge.
As for the severity, it would result in a loss of fund if the user was to pass the contract address. But since this scenario is pretty unlikely, we could rate is as low or dispute the issue entirely.
#3 - 0xleastwood
2022-03-09T05:40:03Z
As per the above comment, I will mark this as low and have it be part of the QA report.
#4 - CloudEllie
2022-03-25T03:16:44Z
Since this issue was downgraded to a QA level, and the warden did not submit a separate QA report, we've renamed this one to "QA report" for consistency.
The original title, for the record, was "No check that _to address is not the contract itself."