search

code-423n4 / 2023-01-rabbithole-findings

1 stars 2 forks source link

Immutable varibles should be checked to there default values #649

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/Erc20Quest.sol#L13-L15 https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/Quest.sol#L17-L18 https://github.com/rabbitholegg/quest-protocol/blob/8c4c1f71221570b14a0479c216583342bd652d8d/contracts/Quest.sol#L13

Vulnerability details

Impact

 It is very important to check whether the immutable variables are not equal to the default values because if  Quest is created and when we pass a 
 default value to a variable then it can't be changed and it can lead to a problem.

Proof of Concept

  Suppose while creating quests we have passed the total participants equal to 0 then  creating this quest is actually wasted because no one can mint 
  an NFT and claim the reward and also with the rewardAmountInWeiOrTokenId if it passed 0 even if they are claimers then the rewards will be 0 only.

Tools Used

  Pen and Paper

Recommended Mitigation Steps

  Always have the immutable variables check whether they are not equal to their default value.
waynehoover commented 1 year ago

This is describing the implementation. We are aware that they are immutable. Same thing could be said for implementing a wrong input also, not just zero address. This check feels like a nice to have.

c4-sponsor commented 1 year ago

waynehoover marked the issue as disagree with severity

c4-judge commented 1 year ago

kirk-baird marked the issue as unsatisfactory: Overinflated severity

kirk-baird commented 1 year ago

I agree with the sponsor and consider this to be over inflated severity ans it is clearly not a medium issue.