[framework] Inconsistent Maximum Supply Logic Between balance.move and coin_manager.move

[miker83z]

FROM THE AUDIT:

Summary

The balance.move module restricts the maximum supply of a coin to 18,446,744,073,709,551,614u64, but the coin_manager.move module allows a maximum supply of 18,446,744,073,709,551,615u64 when no specific limit is set. This inconsistency in the definition of the maximum supply can lead to logic conflicts.

Vulnerability Detail

In the balance.move module, the increase_supply function ensures that the total supply does not exceed 18,446,744,073,709,551,614u64 by using the following check:

assert!(value < (18446744073709551615u64 - self.value), EOverflow);

This effectively enforces a maximum total supply of 18,446,744,073,709,551,614u64 for any coin.

Conversely, in the coin_manager.move module, the maximum_supply function returns 18,446,744,073,709,551,615u64 when no specific maximum supply is set:

option::get_with_default(&manager.maximum_supply, 18_446_744_073_709_551_615u64)

This discrepancy affects multiple functions in coin_manager.move that rely on the maximum_supply value for minting operations, including:

• mint: Validates the total supply plus the minted value against maximum_supply.
• mint_balance: Performs a similar check before minting a Balance<T>.
• available_supply: Computes the remaining supply available for minting.

Impact

The two modules enforce different maximum supply limits, leading to confusion and potential errors in contract logic.

Code Snippet

• Balance.move

    /// Increase supply by `value` and create a new `Balance<T>` with this value.
    public fun increase_supply<T>(self: &mut Supply<T>, value: u64): Balance<T> {
@>>     assert!(value < (18446744073709551615u64 - self.value), EOverflow);
        self.value = self.value + value;
        Balance { value }
    }

• Coin_manager.move

    /// Get the maximum supply possible as a number. 
    /// If no maximum set it's the maximum u64 possible
    public fun maximum_supply<T>(manager: &CoinManager<T>): u64 {
@>>     option::get_with_default(&manager.maximum_supply, 18_446_744_073_709_551_615u64)
    }

    /// Convenience function returning the remaining supply that can be minted still
    public fun available_supply<T>(manager: &CoinManager<T>): u64 {
        maximum_supply(manager) - total_supply(manager)
    }

POC

    #[test]
    #[expected_failure(abort_code = balance::EOverflow)]
    fun test_coin_manager_incorrect_max_supply() {
        let sender = @0xA;
        let mut scenario = test_scenario::begin(sender);
        let witness = COIN_MANAGER_TESTS{};

        // Create a `Coin`.
        let (cap, meta) = coin::create_currency(
            witness,
            0,
            b"TEST",
            b"TEST",
            b"TEST",
            option::none(),
            scenario.ctx(),
        );

        let (cmcap, metacap, mut wrapper) = coin_manager::new(cap, meta, scenario.ctx());

        let max_supply = wrapper.maximum_supply();

        cmcap.mint_and_transfer(&mut wrapper, max_supply, sender, scenario.ctx());

        transfer::public_transfer(cmcap, scenario.ctx().sender());
        metacap.renounce_metadata_ownership(&mut wrapper);
        transfer::public_share_object(wrapper);

        scenario.end();
    }

Tool used

Manual Review

Recommendation

    /// Get the maximum supply possible as a number. 
    /// If no maximum set it's the maximum u64 possible
    public fun maximum_supply<T>(manager: &CoinManager<T>): u64 {
-        option::get_with_default(&manager.maximum_supply, 18_446_744_073_709_551_615u64)
+        option::get_with_default(&manager.maximum_supply, 18_446_744_073_709_551_614u64)
    }

Also enforce a check in enforce_maximum_supply function.