Platform: Code4rena
Start Date: 04/02/2022
Pot Size: $30,000 USDC
Total HM: 3
Participants: 37
Period: 3 days
Judge: leastwood
Id: 84
League: ETH
Rank: 25/37
Findings: 2
Award: $109.25
🌟 Selected for report: 0
🚀 Solo Findings: 0
74.2743 USDC - $74.27
The function getAmountOut() rounds down the result and therefore can return 0 also when _tokenInAmount > 0.
If a user calls buy() with a value that rounds down to zero in getAmountOut(), he/she will end up with zero amount of tokenOut, but with a non-zero value for daoVotedFor[user].
This scenario should never happen, because a user can’t vote for a DAO without being eligible for at least one tokenOut.
Manual code review.
Revert in buy() if the calculated amount of tokenOut (tokenOutAmount_) is zero.
#0 - GalloDaSballo
2022-02-10T01:51:29Z
I find the finding interesting, I don't think it can be of medium severity as by definition we're dealing with dust amounts. That said it seems like users could be able to vote while receiving a 0 amount out. I don't have a strong opinion on this and don't think it has any impact on user funds
#1 - 0xleastwood
2022-03-14T11:28:05Z
Duplicate of #50
#2 - 0xleastwood
2022-03-16T12:48:44Z
After further consideration, I've decided to downgrade this to low severity as it does not impact the availability of the protocol or leak funds. I think this just creates some interesting state handling.
#3 - CloudEllie
2022-03-25T03:24:17Z
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 "Users can vote for DAOs without being eligible for any tokenOut."
34.9843 USDC - $34.98
SLOADsReading variables that use storage as their memory location is an expensive process. When such a variable is read more than once, cache it in a local variable to avoid more SLOADs.
guestlist is read multiple times in buy().totalTokenIn is read multiple times in getTokenInLimitLeft().tokenInLimit is read multiple times in getTokenInLimitLeft().Reading immutable state variables isn't as expensive as reading mutable (regular) state variables. Use immutable state variables for every state variable which its value never changes.
tokenOut.tokenIn.The external call to tokenOut.decimals() in getAmountOut() is unnecessary because the return value will never change. Instead, it can be cached in a state variable to avoid more external calls. Moreover, the value 10**tokenOut.decimals() can be cached to save even more gas.