Upgraded Q -> 2 from #1121 [1716977907927]

[c4-judge]

Judge has assessed an item in Issue #1121 as 2 risk. The relevant finding follows:

[L-01] BoundedKerosineVault::setUnboundedKerosineVault is not fired when deploying, leading to DoS when they are added.
For BoundedKerosineVault to return the assetPrice, it depends on the UnBoundedkerosineVault asset price, and multiply it by 2.

Vault.kerosine.bounded.sol#L44-L50

function assetPrice()
public
view
override
returns (uint) {
return unboundedKerosineVault.assetPrice() * 2;
}
And unboundedKerosineVault should be set by the owner of the contract.

Vault.kerosine.bounded.sol#L23-L30

function setUnboundedKerosineVault(
UnboundedKerosineVault _unboundedKerosineVault
)
external
onlyOwner
{
unboundedKerosineVault = _unboundedKerosineVault;
}
In Deploy.V2.s.sol, the vault is not set before transferring the ownership from the Deployer to the MAINNET_OWNER (multi-sig). which makes adding this vault leads to reverting when calling different functions in the VaultManagerV2, and DoS the protocol.

Deploy.V2.s.sol#L78-L91

function run() public returns (Contracts memory) {
...

UnboundedKerosineVault unboundedKerosineVault = new UnboundedKerosineVault( ... );

BoundedKerosineVault boundedKerosineVault     = new BoundedKerosineVault( ... );

KerosineDenominator kerosineDenominator       = new KerosineDenominator( ... );

unboundedKerosineVault.setDenominator(kerosineDenominator);

// @audit we did not set the unBoundedVault in `boundedKerosineVault` before transfering the ownership

unboundedKerosineVault.transferOwnership(MAINNET_OWNER);
boundedKerosineVault.  transferOwnership(MAINNET_OWNER);

}
PoC
The protocol Deployed into the mainnet.
The ownership of the boundedKerosineVault transferred to multi-sig.
boundedKerosineVault was added as licensed vaults.
The owner of the dNFT owners added that vault.
VaultManagerV2::getNonKeroseneValue, VaultManagerV2::getKeroseneValue, and VaultManagerV2::getTotalUsdValue will revert when getting called, as they call getUsdValue which calls assetPrice, which will make a call to the address(0), and it will revert.
If the vault is added as a NonKerosene vault getNonKeroseneValue() and getTotalUsdValue() will revert.
If the vault is added as Kerosene vault getKeroseneValue() and getTotalUsdValue() will revert
If the vault is added to both then all functions will get reverted (if it is a licensed vault as kerosene vault and normal vault).
This will make all the protocols in DoS, as these functions are called when minting, withdrawing, and liquidating.

In the Deployment script, all settings and configurations occur to all newly deployed contracts before transferring the ownership to MAINNET_OWNER, So we believe that this is an issue.

Recommendations
Set the unBoundedKeroseneVault before transferring the ownership.

Deploy.V2.s.sol L:89

function run() public returns (Contracts memory) {
...

unboundedKerosineVault.setDenominator(kerosineDenominator);

• boundedKerosineVault.setUnboundedKerosineVault(unboundedKerosineVault);

  unboundedKerosineVault.transferOwnership(MAINNET_OWNER);
  boundedKerosineVault. transferOwnership(MAINNET_OWNER);

  ...
  }

[c4-judge]

koolexcrypto marked the issue as duplicate of #829

[c4-judge]

koolexcrypto marked the issue as satisfactory