setUnboundedKerosineVault not called during deployment, causing reverts when querying for Kerosene value after adding it as a Kerosene vault

mediumCode4rena

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/script/deploy/Deploy.V2.s.sol#L78-L82 https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/Vault.kerosine.bounded.sol#L23-L30 https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/README.md?plain=1#L66-L68

Vulnerability details

Root Cause

The setUnboundedKerosineVault function was never called during deployment, nor was it planned to be called post deployment.

Impact

Without setting the unboundedKerosineVault, any attempt to get the asset price of a dNFT that has uses the bounded Kerosene vault will result in a revert.

Note Regarding Vault Licenser

VaultManagerV2's addKerosene() erroneously does a vault license check using keroseneManager.isLicensed(vault) at VaultManagerV2.sol#L88 making it impossible to add Kerosene vaults to Notes. As clarified by the sponsor in this video DYAD V2- Kerosene - Code4rena Audit #2, the vaults in keroseneManager are intended to be used for kerosene value calculation and kerosene vaults are not supposed to be added there. We updated the relevant Kerosene vault license checks to use vaultLicenser.isLicensed(vault) instead as it is aligned with the deployment script at Deploy.V2.s.sol#L95 since unboundedKerosineVault is added as a licensed vault with vaultLicenser.add(address(unboundedKerosineVault))

The two following code changes were made to VaultManagerV2.sol so that the unbounded kerosene vault can be added as a kerosene vault without further changes in the vaults, the vault manager, or the deployment script.

VaultManagerV2.sol#L88 From:

solidity📋
if (!keroseneManager.isLicensed(vault))                 revert VaultNotLicensed();

To:

solidity📋
if (!vaultLicenser.isLicensed(vault))                   revert VaultNotLicensed();

and VaultManagerV2.sol#L280 From:

solidity📋
if (keroseneManager.isLicensed(address(vault))) {

To:

solidity📋
if (vaultLicenser.isLicensed(address(vault))) {

Proof of Concept

The following test script demonstrates that the getKeroseneValue function reverts when the unboundedKerosineVault is not set during deployment.

forge t --mt test_boundedVaultValueRevert --fork-url <MAINNET_RPC_URL> -vv

solidity📋
// SPDX-License-Identifier: MIT
pragma solidity =0.8.17;

import "forge-std/console.sol";
import "forge-std/Test.sol";

import {DeployV2, Contracts} from "../../script/deploy/Deploy.V2.s.sol";
import {Licenser} from "../../src/core/Licenser.sol";
import {Parameters} from "../../src/params/Parameters.sol";
import {WETH} from "../WETH.sol";
import {DNft} from "../../src/core/DNft.sol";

contract V2TestBoundedKeroseneVault is Test, Parameters {

  Contracts contracts;

  function setUp() public {
    contracts = new DeployV2().run();
  }

  function test_boundedVaultValueRevert() public {
    Licenser licenser = Licenser(MAINNET_VAULT_MANAGER_LICENSER);
    vm.prank(MAINNET_OWNER);
    licenser.add(address(contracts.vaultManager));

    address alice = makeAddr("alice");
    vm.prank(MAINNET_OWNER);
    uint aliceTokenId = DNft(MAINNET_DNFT).mintInsiderNft(alice);    

    // drop 1000 eth into ethVault for initial TVL used for calculating kerosene price
    vm.deal(address(contracts.ethVault), 1000 ether);
    vm.prank(address(contracts.ethVault));
    WETH(payable(MAINNET_WETH)).deposit{value: 1000 ether}();

    // add boundedKerosineVault as a licensed vault since it was commented out in the deploy script
    vm.prank(MAINNET_OWNER);
    contracts.vaultLicenser.add(address(contracts.boundedKerosineVault));

    // add boundedKerosineVault to kerosene vault
    vm.prank(alice);
    contracts.vaultManager.addKerosene(aliceTokenId, address(contracts.boundedKerosineVault));

    // getKeroseneValue now reverts
    vm.expectRevert();
    contracts.vaultManager.getKeroseneValue(aliceTokenId);

    // set the unbounded kerosine vault for the bounded kerosine vault
    vm.prank(MAINNET_OWNER);
    contracts.boundedKerosineVault.setUnboundedKerosineVault(contracts.unboundedKerosineVault);

    // this is fine now
    contracts.vaultManager.getKeroseneValue(aliceTokenId);

  }
}

Tools Used

Manual testing

Recommended Mitigation Steps

Set the unboundedKerosineVault during deployment.

Changes to DeployV2

Call the setUnboundedKerosineVault function during deployment after deploying the bounded Kerosene vault at Deploy.V2.s.sol#L78-L82:

solidity📋
boundedKerosineVault.setUnboundedKerosineVault(unboundedKerosineVault);

Assessed type

Other