#46119 [SC-Low] Incorrect `msg.Value` check in `CoreVault` Transfer

Submitted on May 25th 2025 at 06:53:02 UTC by @aman for Audit Comp | Flare | FAssets

  • Report ID: #46119

  • Report Type: Smart Contract

  • Report severity: Low

  • Target: https://github.com/flare-foundation/fassets/blob/main/contracts/assetManager/library/CoreVault.sol

  • Impacts:

    • Permanent freezing of funds

Description

Brief/Intro

The transferToCoreVault function in CoreVault.sol incorrectly assumes that the TRANSFER_GAS_ALLOWANCE should be retained by the contract, when in fact the gas costs are paid by the agent (msg.sender) directly. This leads to unnecessary retention of funds that should be returned to the agent.

Vulnerability Details

The vulnerable code is in CoreVault.sol:

if (msg.value > transferFeeWei + Transfers.TRANSFER_GAS_ALLOWANCE) {
    Transfers.transferNAT(state.nativeAddress, transferFeeWei);
    Transfers.transferNATAllowFailure(payable(msg.sender), msg.value - transferFeeWei);
}

The issue is that the code:

  1. Checks if msg.value is greater than transferFeeWei + TRANSFER_GAS_ALLOWANCE

  2. But the TRANSFER_GAS_ALLOWANCE is unnecessary because:

    • Gas costs are paid by the agent (msg.sender) directly through the transaction

    • The contract doesn't need to retain any additional gas allowance

    • The only fee that should be retained is the transferFeeWei

Impact Details

  • Severity: Medium

  • Impact: Unnecessarily retains funds that should be returned to the agent

  • Likelihood: High - Affects every transfer to core vault where msg.value > transferFeeWei and msg.value <= transferFeeWei + TRANSFER_GAS_ALLOWANCE

References

  • File: contracts/assetManager/library/CoreVault.sol

  • Function: transferToCoreVault

Recommendations

  1. Remove the TRANSFER_GAS_ALLOWANCE check since gas costs are paid by the msg.sender:

if (msg.value > transferFeeWei) {
    Transfers.transferNAT(state.nativeAddress, transferFeeWei);
    Transfers.transferNATAllowFailure(payable(msg.sender), msg.value - transferFeeWei);
}

Proof of Concept

Proof of Concept

To proof my point I have added few changed to CoreVault.sol file :

+    uint256 balNativeAddressBefore=  payable(address(state.nativeAddress)).balance;
        if (msg.value > transferFeeWei + Transfers.TRANSFER_GAS_ALLOWANCE) {
            Transfers.transferNAT(state.nativeAddress, transferFeeWei);
            Transfers.transferNATAllowFailure(payable(msg.sender), msg.value - transferFeeWei);
        } else {
            Transfers.transferNAT(state.nativeAddress, msg.value);
        }
+        uint256 balNativeAddressAfter=  payable(address(state.nativeAddress)).balance;
+        console.log("state.nativeAddress balance After balance " , balNativeAddressAfter - balNativeAddressBefore - transferFeeWei);

And also update the Agent.ts:

diff --git a/test/integration/utils/Agent.ts b/test/integration/utils/Agent.ts
index 02980151..6ae8697a 100644
--- a/test/integration/utils/Agent.ts
+++ b/test/integration/utils/Agent.ts
@@ -529,7 +537,7 @@ export class Agent extends AssetContextClient {

     async transferToCoreVault(transferAmount: BNish) {
         const cbTransferFee = await this.assetManager.transferToCoreVaultFee(transferAmount);
-        const res = await this.assetManager.transferToCoreVault(this.vaultAddress, transferAmount, { from: this.ownerWorkAddress, value: cbTransferFee });
+        const res = await this.assetManager.transferToCoreVault(this.vaultAddress, transferAmount, { from: this.ownerWorkAddress, value: cbTransferFee.add(toBN(100_000)) });
         const rdreqs = filterEvents(res, "RedemptionRequested").map(evt => evt.args);
         assert.isAtLeast(rdreqs.length, 1);

And than add following test case to CoreVault.ts file:

    it.only("extra amount not return correctly", async () => {
        const agent = await Agent.createTest(context, agentOwner1, underlyingAgent1);
        const agent2 = await Agent.createTest(context, agentOwner2, underlyingAgent2);
        const minter = await Minter.createTest(context, minterAddress1, underlyingMinter1, context.underlyingAmount(10000000));
        const redeemer = await Redeemer.create(context, redeemerAddress1, underlyingRedeemer1);
        let agentInfo1 = await agent.getAgentInfo();

        await prefundCoreVault(minter.underlyingAddress, 1e6);
        // allow CV manager addresses
        await coreVaultManager.addAllowedDestinationAddresses([redeemer.underlyingAddress], { from: governance });
        // make agent available
        await agent.depositCollateralLotsAndMakeAvailable(50);

        // mint
        let [minted] = await minter.performMinting(agent.vaultAddress, 50);


        await minter.transferFAsset(redeemer.address, minted.mintedAmountUBA);
        // agent requests transfer for some backing to core vault
        const transferAmount = context.convertLotsToUBA(49);
        await agent.transferToCoreVault(transferAmount);
    });

Run with command yarn testHH console.logs of this test :

state.nativeAddress balance After balance  100000 # extra amount nativeAddress receives

Was this helpful?