Summary
Same validation fail will lead to different errors.
Vulnerability Details
In the liquidateVault function, a try/catch block is utilized.
function liquidateVault(uint256 _tokenId) external onlyLiquidator {
ISmartVault vault = ISmartVault(smartVaultIndex.getVaultAddress(_tokenId));
try vault.undercollateralised() returns (bool _undercollateralised) {
require(_undercollateralised, "vault-not-undercollateralised");
vault.liquidate();
IEUROs(euros).revokeRole(IEUROs(euros).MINTER_ROLE(), address(vault));
IEUROs(euros).revokeRole(IEUROs(euros).BURNER_ROLE(), address(vault));
emit VaultLiquidated(address(vault));
} catch {
revert("other-liquidation-error");
}
}
The problem lies in these require statements:
require(_undercollateralised, "vault-not-undercollateralised");
and
require(undercollateralised(), "err-not-liquidatable");
As we can see, the validation is the same, while the error messages are different. Also the catch block is not supposed to be there as it will never be reached. That is because vault.liquidate() will never revert because the require above it has the same validation.
revert("other-liquidation-error");
Impact
Wrong error message will be thrown.
Tools Used
Manual Review
Recommendations
Remove the try/catch and the require.
function liquidateVault(uint256 _tokenId) external onlyLiquidator {
ISmartVault vault = ISmartVault(smartVaultIndex.getVaultAddress(_tokenId));
vault.liquidate();
IEUROs(euros).revokeRole(IEUROs(euros).MINTER_ROLE(), address(vault));
IEUROs(euros).revokeRole(IEUROs(euros).BURNER_ROLE(), address(vault));
emit VaultLiquidated(address(vault));
}