The Standard
The Standard
DeFiHardhat
20,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

Variable name is confusing in SmartVaultManager

zigtur

Summary

A variable name in the SmartVaultManager contract is confusing.

Vulnerability Details

According to the test suite, the protocol variable in SmartVaultManager points to the LiquidationPoolManager.

But the protocol variable in the LiquidationPoolManager points to the Protocol multisig address.

It lets the user think that the SmartVault liquidated funds are sent to the Protocol multisig
but those should be sent to the LiquidationPoolManager.

Impact

The variable naming can be really confusing for users.

Tools Used

Scope:

  • https://github.com/Cyfrin/2023-12-the-standard/blob/main/contracts/SmartVaultManagerV5.sol#L21

  • https://github.com/Cyfrin/2023-12-the-standard/blob/main/contracts/SmartVaultV3.sol#L105

  • https://github.com/Cyfrin/2023-12-the-standard/blob/main/contracts/SmartVaultV3.sol#L111

  • https://github.com/Cyfrin/2023-12-the-standard/blob/main/contracts/SmartVaultV3.sol#L165

  • https://github.com/Cyfrin/2023-12-the-standard/blob/main/contracts/SmartVaultV3.sol#L173

  • https://github.com/Cyfrin/2023-12-the-standard/blob/main/contracts/SmartVaultV3.sol#L191

  • https://github.com/Cyfrin/2023-12-the-standard/blob/main/contracts/SmartVaultV3.sol#L197

Recommendations

Consider renaming the protocol variable into something like liquidationManager.

For backwards compatibility, a view function named protocol() can be used as an alias to liquidationManager.

The following diff implements this fix:

diff --git a/contracts/SmartVaultManagerV5.sol b/contracts/SmartVaultManagerV5.sol
index d54f06f..04a8b9d 100644
--- a/contracts/SmartVaultManagerV5.sol
+++ b/contracts/SmartVaultManagerV5.sol
@@ -18,7 +18,7 @@ contract SmartVaultManagerV5 is ISmartVaultManager, ISmartVaultManagerV2, Initia
uint256 public constant HUNDRED_PC = 1e5;
- address public protocol;
+ address public liquidationManager;
address public liquidator;
address public euros;
uint256 public collateralRate;
@@ -100,6 +100,10 @@ contract SmartVaultManagerV5 is ISmartVaultManager, ISmartVaultManagerV2, Initia
return lastToken;
}
+ function protocol() external view returns (address) {
+ return liquidationManager;
+ }
+
function setMintFeeRate(uint256 _rate) external onlyOwner {
mintFeeRate = _rate;
function liquidateERC20(IERC20 _token) private {
- if (_token.balanceOf(address(this)) != 0) _token.safeTransfer(ISmartVaultManagerV3(manager).protocol(), _token.balanceOf(address(this)));
+ if (_token.balanceOf(address(this)) != 0) _token.safeTransfer(ISmartVaultManagerV3(manager).liquidationManager(), _token.balanceOf(address(this)));
}
function liquidate() external onlyVaultManager {
@@ -162,7 +162,7 @@ contract SmartVaultV3 is ISmartVault {
require(fullyCollateralised(_amount + fee), UNDER_COLL);
minted = minted + _amount + fee;
EUROs.mint(_to, _amount);
- EUROs.mint(ISmartVaultManagerV3(manager).protocol(), fee);
+ EUROs.mint(ISmartVaultManagerV3(manager).liquidationManager(), fee);
emit EUROsMinted(_to, _amount, fee);
}
@@ -170,7 +170,7 @@ contract SmartVaultV3 is ISmartVault {
uint256 fee = _amount * ISmartVaultManagerV3(manager).burnFeeRate() / ISmartVaultManagerV3(manager).HUNDRED_PC();
minted = minted - _amount;
EUROs.burn(msg.sender, _amount);
- IERC20(address(EUROs)).safeTransferFrom(msg.sender, ISmartVaultManagerV3(manager).protocol(), fee);
+ IERC20(address(EUROs)).safeTransferFrom(msg.sender, ISmartVaultManagerV3(manager).liquidationManager(), fee);
emit EUROsBurned(_amount, fee);
}
@@ -188,13 +188,13 @@ contract SmartVaultV3 is ISmartVault {
}
function executeNativeSwapAndFee(ISwapRouter.ExactInputSingleParams memory _params, uint256 _swapFee) private {
- (bool sent,) = payable(ISmartVaultManagerV3(manager).protocol()).call{value: _swapFee}("");
+ (bool sent,) = payable(ISmartVaultManagerV3(manager).liquidationManager()).call{value: _swapFee}("");
require(sent, "err-swap-fee-native");
ISwapRouter(ISmartVaultManagerV3(manager).swapRouter2()).exactInputSingle{value: _params.amountIn}(_params);
}
function executeERC20SwapAndFee(ISwapRouter.ExactInputSingleParams memory _params, uint256 _swapFee) private {
- IERC20(_params.tokenIn).safeTransfer(ISmartVaultManagerV3(manager).protocol(), _swapFee);
+ IERC20(_params.tokenIn).safeTransfer(ISmartVaultManagerV3(manager).liquidationManager(), _swapFee);
IERC20(_params.tokenIn).safeApprove(ISmartVaultManagerV3(manager).swapRouter2(), _params.amountIn);
ISwapRouter(ISmartVaultManagerV3(manager).swapRouter2()).exactInputSingle(_params);
IWETH weth = IWETH(ISmartVaultManagerV3(manager).weth());
Updates

Lead Judging Commences

hrishibhat Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

informational/invalid

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.