Auction.sol amendAccountParticipation has no zero division check

[code423n4]

Handle

harleythedog

Vulnerability details

Impact

In Auction.sol, there is a privileged function amendAccountParticipation. Presumably, this function allows a trusted party to decrease a user's participation in an auction for acting maliciously (maybe there are other reasons for this function as well). However, if the auction is not currently finalized, then auction.finalPrice will be 0, and amendAccountParticipation will revert due to a division by zero on line 800. This means that the user's participation cannot be amended until the auction is finalized. This restricts the time frame that the admins can reduce the user's participation, which increases the chances that the user will get away with some vested reward tokens that are potentially malicious.

Proof of Concept

See amendAccountParticipation here: https://github.com/code-423n4/2021-11-malt/blob/c3a204a2c0f7c653c6c2dda9f4563fd1dc1cecf3/src/contracts/Auction.sol#L786

Notice that there may be a division by zero on line 800 if the auction is not finalized, which will always revert.

Tools Used

Inspection

Recommended Mitigation Steps

In the case where auction.finalPrice is zero, just let the function return early, since unclaimedArbTokens does not need to be altered in this case. This is already what is done in other functions, like userClaimableArbTokens.

[0xScotch]

amendAccountParticipation is only called by the auction escape hatch contract which enforces the auction is over. Adding a guard against finalPrice being 0 is worth doing though as a fail safe.

[GalloDaSballo]

I don't think the warden has identified any particular vulnerability. Just a hedge case for the logic. The logic is also inconsistent with other parts of the code where the 0 value is handled without a revert. For this reasons I think the finding is valid, but low severity (inconsistent code)