Platform: Code4rena
Start Date: 16/09/2021
Pot Size: $50,000 USDC
Total HM: 26
Participants: 30
Period: 7 days
Judge: GalloDaSballo
Total Solo HM: 17
Id: 36
League: ETH
Rank: 18/30
Findings: 1
Award: $667.96
🌟 Selected for report: 2
🚀 Solo Findings: 0
🌟 Selected for report: nikitastupin
333.9773 USDC - $333.98
nikitastupin
Here https://github.com/code-423n4/2021-09-defiProtocol/blob/e6dcf43a2f03aa65e04f0edc8ed1d7272677fabe/contracts/contracts/Auction.sol#L143-L143 the bounty variable is copied from Storage to Memory. Later it's assigned to false https://github.com/code-423n4/2021-09-defiProtocol/blob/e6dcf43a2f03aa65e04f0edc8ed1d7272677fabe/contracts/contracts/Auction.sol#L147. However, this assignment has no effect because bounty variable located at Memory so it's basically just thrown away when loop iteration finishes.
I think the intention was to make the bounty.active false so the same bounty isn't claimed twice or more times https://github.com/code-423n4/2021-09-defiProtocol/blob/e6dcf43a2f03aa65e04f0edc8ed1d7272677fabe/contracts/contracts/Auction.sol#L144. However, the bounty.active will always be true because it never changes to false except for https://github.com/code-423n4/2021-09-defiProtocol/blob/e6dcf43a2f03aa65e04f0edc8ed1d7272677fabe/contracts/contracts/Auction.sol#L147 (which has no effect).
I don't see the direct impact here, however it may arise with the future changes to the contracts.
I'll write a PoC if needed.
Do _bounties[bountyIds[i]].active = false instead of bounty.active = false if you need this check or just remove bounty.active = false and require(bounty.active) lines to save a gas otherwise.
#0 - frank-beard
2021-10-19T17:37:15Z
#1 - GalloDaSballo
2021-12-02T00:56:37Z
The finding details the fact that a storage variable will not be set, which violates the logic of the protocol Agree with finding and severity
#168 uses this specific property of the code to detail a DOS attack as such will be judged separately
#2 - GalloDaSballo
2021-12-19T15:50:43Z
After thinking about it, I revert back to low risk on this finding. The warden didn't mention re-entrancy nor any particular exploit
🌟 Selected for report: nikitastupin
333.9773 USDC - $333.98
nikitastupin
Timelock period of the protocol is declared to be 24 hours https://github.com/code-423n4/2021-09-defiProtocol/blob/e6dcf43a2f03aa65e04f0edc8ed1d7272677fabe/contracts/contracts/Auction.sol#L13-L13. However, timelock period depends on block.number instead of block.timestamp. Thus if the block mining time decreases, the timelock period will decrease proportionally.
Timelock bypass under certain conditions.
I'll write a PoC if needed.
Use block.timestamp instead of block.number in timelock calculations.
#0 - GalloDaSballo
2021-12-12T17:59:45Z
Finding is valid and correct, block number is not as reliable, however this is a rounding issue, similar to the rounding issue in ONE_DAY, I will downgrade this to Low Risk and highly recommend the sponsor to make it obvious for users that there are approximation issues when using block number