You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Unlike the attacker, the defender's moves aren't currently being checked for the proper length or range in defenderRevealMoves.
If the defender's doesn't supply enough moves or the first MOVE_LENGTH moves aren't valid, then this will always cause an out of bounds exception in defenderRevealMoves -> _executeMoves if there are not enough moves and in defenderRevealMoves -> _executeMoves -> _applyBonuses -> getPartRarity if any of the first MOVE_LENGTH moves is not 0-3.
If the defender supplies more than MOVE_LENGTH moves, then there seem to be no ill effects, as only the first MOVE_LENGTH are considered.
Due to the nature of the commit/reveal scheme, we cannot check the validity of the defender's moves in _defenderCommitMoves, so if the defender originally committed with invalid moves, then they will be forced to either cancel the battle or let it time out.
Fix:
When the defender does reveal their moves, it might be worth considering nullifying the duel if the moves weren't valid to begin with.
Even if this approach isn't taken, this can still be a lurking bug for future battle implementations and it would save gas if require(_isValidMoves(_moves)) was added to defenderRevealMoves to avoid exceptions.
The text was updated successfully, but these errors were encountered:
Unlike the attacker, the defender's moves aren't currently being checked for the proper length or range in
defenderRevealMoves
.If the defender's doesn't supply enough moves or the first MOVE_LENGTH moves aren't valid, then this will always cause an out of bounds exception in
defenderRevealMoves
->_executeMoves
if there are not enough moves and indefenderRevealMoves
->_executeMoves
->_applyBonuses
->getPartRarity
if any of the first MOVE_LENGTH moves is not 0-3.If the defender supplies more than MOVE_LENGTH moves, then there seem to be no ill effects, as only the first MOVE_LENGTH are considered.
Due to the nature of the commit/reveal scheme, we cannot check the validity of the defender's moves in
_defenderCommitMoves
, so if the defender originally committed with invalid moves, then they will be forced to either cancel the battle or let it time out.Fix:
When the defender does reveal their moves, it might be worth considering nullifying the duel if the moves weren't valid to begin with.
Even if this approach isn't taken, this can still be a lurking bug for future battle implementations and it would save gas if
require(_isValidMoves(_moves))
was added todefenderRevealMoves
to avoid exceptions.The text was updated successfully, but these errors were encountered: