QA Report #20
Labels
bug
Something isn't working
QA (Quality Assurance)
Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax
https://github.com/code-423n4/2022-05-runes/blob/main/contracts/ForgottenRunesWarriorsMinter.sol#L287
Below calculation can be done in sinle line. Variables elapsed and steps can be removed.
Replace
uint256 elapsed = block.timestamp - daStartTime;
uint256 steps = elapsed / daDropInterval;
uint256 stepDeduction = steps * dropPerStep;
With
uint stepDeduction = ((block.timestamp - daStartTime) / daDropInterval) * dropPerStep
https://github.com/code-423n4/2022-05-runes/blob/main/contracts/ForgottenRunesWarriorsGuild.sol#L99
Add a check if receipient is not same as msg.sender(). Although only owner can call this function, if attacker can get hold of contract, they can mint all token and trasnfer to seld.
https://github.com/code-423n4/2022-05-runes/blob/main/contracts/ForgottenRunesWarriorsGuild.sol#L140
Define a immutable constant MAX_WARRIORS = 20 and use it in below check. Also use revert() function instead of require to save gas fees.
require(
numWarriors > 0 && numWarriors <= 20,
'You can summon no more than 20 Warriors at a time'
);
The text was updated successfully, but these errors were encountered: