#37661 [SC-High] Incorrect `total_active_stake` reduction causes loss of funds for the users and exc

Submitted on Dec 11th 2024 at 22:09:38 UTC by @holydevoti0n for Audit Comp | Folks: Liquid Staking

  • Report ID: #37661

  • Report Type: Smart Contract

  • Report severity: High

  • Target: https://github.com/Folks-Finance/algo-liquid-staking-contracts/blob/8bd890fde7981335e9b042a99db432e327681e1a/contracts/xalgo/consensus_v2.py

  • Impacts:

    • Permanent freezing of funds

    • Permanent freezing of unclaimed yield

Description

Brief/Intro

The burn operation incorrectly reduces total_active_stake by including rewards in the reduction amount. This leads to inflated reward calculations, causing the protocol to collect excessive fees from user funds.

Vulnerability Details

When users burn xALGO, the contract incorrectly reduces total_active_stake by the full withdrawal amount (stake + rewards): https://github.com/Folks-Finance/algo-liquid-staking-contracts/blob/8bd890fde7981335e9b042a99db432e327681e1a/contracts/xalgo/consensus_v2.py#L824

def burn(send_xalgo: abi.AssetTransferTransaction, min_received: abi.Uint64) -> Expr:
    burn_amount = send_xalgo.get().asset_amount()
    algo_balance = ScratchVar(TealType.uint64)
    algo_to_send = ScratchVar(TealType.uint64)

    return Seq(
        # calculate algo amount to send
@>        algo_balance.store(
            App.globalGet(total_active_stake_key)
@>            + App.globalGet(total_rewards_key)
            - App.globalGet(total_unclaimed_fees_key)
        ),
@>        algo_to_send.store(
            mul_scale(
                burn_amount,
                algo_balance.load(),
                get_x_algo_circulating_supply() + burn_amount
            )
        ),
       ...
        # update total active stake
@>        App.globalPut(total_active_stake_key, App.globalGet(total_active_stake_key) - algo_to_send.load()),
        ...
    )
  1. First burn distorts total_active_stake by including rewards in reduction.

  2. Subsequent burns use this distorted total_active_stake value.

  3. Creates compounding error where each burn further distorts the rate.

The second problem is that the total_active_stake is used to calculate the protocol fees and the total in rewards: https://github.com/Folks-Finance/algo-liquid-staking-contracts/blob/8bd890fde7981335e9b042a99db432e327681e1a/contracts/xalgo/consensus_v2.py#L144-L147

The artificially low total_active_stake causes:

  • Inflated new_total_rewards calculation

  • Excessive fee collection since fees are taken as a percentage of rewards

  • Compounding effect as more burns occur over time

Impact Details

  • Loss of user funds through excessive fee collection(especially for the users that will keep the funds after several burn operations)

  • Rewards and fees are calculated on stake that is incorrectly classified as rewards

  • Impact compounds over time with each burn operation

Recommendation

The burn operation should reduce total_active_stake proportionally based only on the staked amount. i.e:

Proof of Concept

The PoC shows the tracking of how total_active_stake decreases more than it should, which causes the protocol to misclassify staked funds as rewards.

Another observation is that the helper functions of the PoC is merely copy/paste code from the original test suite. I've done this so I could separate the test in a new file but still preserve the state and use the same logic.

Create a new test file called incorrect-burning.test.ts and paste the following code into it:

then run: PYTHONPATH="./contracts" npx jest test/incorrect-burning.test.ts --runInBand

Output:

Last updated

Was this helpful?