The original implementation had a flaw where a slashing could fail due to a check in handleSlashing() that would revert when amount to slash equals zero. This could happen if during a time gap between slashing request and its finalization totalAssets of the vault were reduced to 0 either due another DSS slashing (since vaults can be staked to multiple DSSs) or because pending withdrawals were finalized.
As a result, if the slashing reverted for such vault, the entire transaction would also revert. Since slashing request can include multiple slashes for different vaults, this would block other vault slashes as well.
Mitigation
FIX
The mitigation modifies handleSlashing() function so it does not revert when amount to slash equals zero. Instead the function now skips such vaults, which results in transaction no longer reverting, so it does not block other vault slashes anymore. The mitigation resolves the original issue.
Lines of code
Vulnerability details
C4 Issue:
M-05: https://github.com/code-423n4/2024-07-karak-findings/issues/15
Comments
The original implementation had a flaw where a slashing could fail due to a check in
handleSlashing()
that would revert when amount to slash equals zero. This could happen if during a time gap between slashing request and its finalizationtotalAssets
of the vault were reduced to 0 either due another DSS slashing (since vaults can be staked to multiple DSSs) or because pending withdrawals were finalized. As a result, if the slashing reverted for such vault, the entire transaction would also revert. Since slashing request can include multiple slashes for different vaults, this would block other vault slashes as well.Mitigation
FIX The mitigation modifies
handleSlashing()
function so it does not revert when amount to slash equals zero. Instead the function now skips such vaults, which results in transaction no longer reverting, so it does not block other vault slashes anymore. The mitigation resolves the original issue.Conclusion
LGTM