#37660 [SC-High] incorrect tracking of `TOTAL_ACTIVE_STAKE` leads to permanent freezing of funds
Submitted on Dec 11th 2024 at 21:56:47 UTC by @A2Security for Audit Comp | Folks: Liquid Staking
Report ID: #37660
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
Description
Brief/Intro
There is a flow in the internal accounting for tracking the total active amount of staked Algo in the state variable TOTAL_ACTIVE_STAKE, this state variable is only incremented by the amount of algo deposited (increased in claim_delayed_mint() and immediate_mint()) when minting xAlgo.
The bug however arises from the fact that in the burn() function we reduce the TOTAL_ACTIVE_STAKE by the amount of the algo recieved from burning the xAlgo.
To simplify the bug:
User A deposits 100 ALGO recieves 100 xALGO, TOTAL_ACTIVE_STAKE = 100 ALGO
After a year the user, accrues some rewards 10 ALGO so the XALGO he owns is now worth 110 ALGO
when the user will try to burn hi 100 xAlgo, he will fail because in burn() we will try to reduce TOTAL_ACTIVE_STAKE (100 ALGO) by 110 ALGO which will underflow. (10 ALGO are locked in the contract)
Vulnerability Details
The vulnerability arises from the fact that Total Active Stake is not tracked correctly leading to an undersestimation which wil result in an underflow in the burn() function.
The TOTAL_ACTIVE_STAKE is only updated in the immediate_mint() and claim_delayed_mint() functions.
def claim_delayed_mint(receiver: abi.Address, nonce: abi.StaticBytes[L[2]]) -> Expr:
box_name = Concat(DelayMintBox.NAME_PREFIX, receiver.get(), nonce.get())
box = BoxGet(box_name)
delay_mint_receiver = Extract(box.value(), DelayMintBox.RECEIVER, Int(32))
delay_mint_stake = ExtractUint64(box.value(), DelayMintBox.STAKE)
delay_mint_round = ExtractUint64(box.value(), DelayMintBox.ROUND)
algo_balance = ScratchVar(TealType.uint64)
mint_amount = ScratchVar(TealType.uint64)
return Seq(
# callable by anyone
rekey_and_close_to_check(),
# ensure initialised
Assert(App.globalGet(initialised_key)),
# check nonce is 2 bytes
Assert(Len(nonce.get()) == Int(2)),
# check box
box,
Assert(box.hasValue()),
Assert(receiver.get() == delay_mint_receiver),
Assert(Global.round() >= delay_mint_round),
# update total stake and total rewards
App.globalPut(total_pending_stake_key, App.globalGet(total_pending_stake_key) - delay_mint_stake),
@>> App.globalPut(total_active_stake_key, App.globalGet(total_active_stake_key) + delay_mint_stake),The problem however arises because we don't increase the TOTAL_ACTIVE_STAKE when we sync rewards and unclaimed fees for the protocol. This leads to underestimating the TOTAL_ACTIVE_STAKE by the accumalted amounts
This will lead to the actual withdrawable assets of users to be significantly higher than the value stored in the state variable TOTAL_ACTIVE_STAKE. Which will lead to an underflow in the burn() function
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(
rekey_and_close_to_check(),
# ensure initialised
Assert(App.globalGet(initialised_key)),
# check xALGO sent
check_x_algo_sent(send_xalgo),
# update total rewards
update_total_rewards_and_unclaimed_fees(),
# 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
)
),
# check amount and send ALGO to user
Assert(algo_to_send.load()),
Assert(algo_to_send.load() >= min_received.get()),
send_algo_from_proposers(Txn.sender(), algo_to_send.load()),
# @audit underflow here
@>> App.globalPut(total_active_stake_key, App.globalGet(total_active_stake_key) - algo_to_send.load()),N.B algo to
Impact Details
Permanent locked funds for the user who withdraws last.
A simplified example to showcase this vulnerability. 2 users A and B.
User A deposits 100 ALGO recieves 100 xALGO, TOTAL_ACTIVE_STAKE = 100 ALGO w8 a year and his 100 xAlgo will be worth 110 ALGO
User B deposits 10 ALGO recieves ~8 xALGO, TOTAL_ACTIVE_STAKE = 110 ALGO (100 from User A and 10 from User B)
User A burns his xAlgo (worth 110 ALGO), TOTAL_ACTIVE_STAKE = 110 - 110 = 0 ALGO
User B can't withdraw his ALGO because when he tries to withdraw reducing 10 from 0 will underflow.
Proof of Concept
Proof of Concept
xpected Result:
> algo-liquid-staking-contracts@0.0.1 test
> PYTHONPATH='./contracts' jest --runInBand
console.log
totoal pending stake 0n
at Object.log (test/xAlgoConsensusV2.test.ts:1590:15)
console.log
total active stake 182352954n
at Object.log (test/xAlgoConsensusV2.test.ts:1591:15)
console.log
total unclaimed fees 0n
at Object.log (test/xAlgoConsensusV2.test.ts:1592:15)
console.log
0n 182352954n 63000000n 0n
at Object.log (test/xAlgoConsensusV2.test.ts:1593:15)
console.log
proposers algo balance 245352954n
at Object.log (test/xAlgoConsensusV2.test.ts:1597:15)
console.log
unclaimable balance 63000000n
at Object.log (test/xAlgoConsensusV2.test.ts:1599:15)Please add the following modifications:
diff --git a/test/xAlgoConsensusV2.test.ts b/test/xAlgoConsensusV2.test.ts
index 662887b..563887e 100644
--- a/test/xAlgoConsensusV2.test.ts
+++ b/test/xAlgoConsensusV2.test.ts
@@ -1528,6 +1528,45 @@ describe("Algo Consensus V2", () => {
});
describe("claim fee", () => {
+ // test("succeeds", async () => {
+ // // airdrop 10 ALGO rewards (%fee of which will be claimable by admin)
+ // const additionalRewards = BigInt(10e6);
+ // await fundAccountWithAlgo(algodClient, proposer0.addr, additionalRewards / BigInt(2), await getParams(algodClient));
+ // await fundAccountWithAlgo(algodClient, proposer1.addr, additionalRewards / BigInt(2), await getParams(algodClient));
+ // const additionalRewardsFee = mulScale(additionalRewards, fee, ONE_4_DP);
+
+ // // balances before
+ // const adminAlgoBalanceB = await getAlgoBalance(algodClient, admin.addr);
+ // const { algoBalance: oldAlgoBalance, xAlgoCirculatingSupply: oldXAlgoCirculatingSupply, proposersBalances: oldProposersBalance } = await getXAlgoRate();
+
+ // // state before
+ // let state = await parseXAlgoConsensusV2GlobalState(algodClient, xAlgoAppId);
+ // const { totalRewards: oldTotalRewards, totalUnclaimedFees: oldTotalUnclaimedFees } = state;
+
+ // // claim fee
+ // const proposerAddrs = [proposer0.addr, proposer1.addr];
+ // const tx = prepareClaimXAlgoConsensusV2Fee(xAlgoConsensusABI, xAlgoAppId, user1.addr, admin.addr, proposerAddrs, await getParams(algodClient));
+ // const txId = await submitTransaction(algodClient, tx, user1.sk);
+ // const txInfo = await algodClient.pendingTransactionInformation(txId).do();
+ // const { txn: transfer } = txInfo['inner-txns'][txInfo['inner-txns'].length - 1].txn;
+ // state = await parseXAlgoConsensusV2GlobalState(algodClient, xAlgoAppId);
+ // expect(state.totalRewards).toEqual(oldTotalRewards + additionalRewards - (oldTotalUnclaimedFees + additionalRewardsFee));
+ // expect(state.totalUnclaimedFees).toEqual(BigInt(0));
+
+ // // balances after
+ // const adminAlgoBalanceA = await getAlgoBalance(algodClient, admin.addr);
+ // const { algoBalance, xAlgoCirculatingSupply, proposersBalances } = await getXAlgoRate();
+ // expect(adminAlgoBalanceA).toEqual(adminAlgoBalanceB + oldTotalUnclaimedFees + additionalRewardsFee);
+ // expect(algoBalance).toEqual(oldAlgoBalance);
+ // expect(xAlgoCirculatingSupply).toEqual(oldXAlgoCirculatingSupply);
+ // expect(proposersBalances[0] + proposersBalances[1]).toEqual(oldProposersBalance[0] + oldProposersBalance[1] - (oldTotalUnclaimedFees + additionalRewardsFee));
+ // expect(proposersBalances[0] - BigInt(1)).toBeLessThanOrEqual(proposersBalances[1]);
+ // expect(proposersBalances[0] + BigInt(1)).toBeGreaterThanOrEqual(proposersBalances[1]);
+ // expect(transfer.type).toEqual("pay");
+ // expect(transfer.amt).toEqual(Number(oldTotalUnclaimedFees + additionalRewardsFee));
+ // expect(transfer.snd).toEqual(decodeAddress(getApplicationAddress(xAlgoAppId)).publicKey);
+ // expect(transfer.rcv).toEqual(decodeAddress(admin.addr).publicKey);
+ // });
test("succeeds", async () => {
// airdrop 10 ALGO rewards (%fee of which will be claimable by admin)
const additionalRewards = BigInt(10e6);
@@ -1537,11 +1576,7 @@ describe("Algo Consensus V2", () => {
// balances before
const adminAlgoBalanceB = await getAlgoBalance(algodClient, admin.addr);
- const { algoBalance: oldAlgoBalance, xAlgoCirculatingSupply: oldXAlgoCirculatingSupply, proposersBalances: oldProposersBalance } = await getXAlgoRate();
- // state before
- let state = await parseXAlgoConsensusV2GlobalState(algodClient, xAlgoAppId);
- const { totalRewards: oldTotalRewards, totalUnclaimedFees: oldTotalUnclaimedFees } = state;
// claim fee
const proposerAddrs = [proposer0.addr, proposer1.addr];
@@ -1549,24 +1584,21 @@ describe("Algo Consensus V2", () => {
const txId = await submitTransaction(algodClient, tx, user1.sk);
const txInfo = await algodClient.pendingTransactionInformation(txId).do();
const { txn: transfer } = txInfo['inner-txns'][txInfo['inner-txns'].length - 1].txn;
- state = await parseXAlgoConsensusV2GlobalState(algodClient, xAlgoAppId);
- expect(state.totalRewards).toEqual(oldTotalRewards + additionalRewards - (oldTotalUnclaimedFees + additionalRewardsFee));
- expect(state.totalUnclaimedFees).toEqual(BigInt(0));
+ let state = await parseXAlgoConsensusV2GlobalState(algodClient, xAlgoAppId);
+ const { totalRewards, totalUnclaimedFees,totalActiveStake, totalPendingStake } = state;
+ console.log("totoal pending stake", totalPendingStake);
+ console.log("total active stake", totalActiveStake);
+ console.log("total unclaimed fees", totalUnclaimedFees);
+ console.log(totalPendingStake, totalActiveStake, totalRewards, totalUnclaimedFees);
// balances after
- const adminAlgoBalanceA = await getAlgoBalance(algodClient, admin.addr);
- const { algoBalance, xAlgoCirculatingSupply, proposersBalances } = await getXAlgoRate();
- expect(adminAlgoBalanceA).toEqual(adminAlgoBalanceB + oldTotalUnclaimedFees + additionalRewardsFee);
- expect(algoBalance).toEqual(oldAlgoBalance);
- expect(xAlgoCirculatingSupply).toEqual(oldXAlgoCirculatingSupply);
- expect(proposersBalances[0] + proposersBalances[1]).toEqual(oldProposersBalance[0] + oldProposersBalance[1] - (oldTotalUnclaimedFees + additionalRewardsFee));
- expect(proposersBalances[0] - BigInt(1)).toBeLessThanOrEqual(proposersBalances[1]);
- expect(proposersBalances[0] + BigInt(1)).toBeGreaterThanOrEqual(proposersBalances[1]);
- expect(transfer.type).toEqual("pay");
- expect(transfer.amt).toEqual(Number(oldTotalUnclaimedFees + additionalRewardsFee));
- expect(transfer.snd).toEqual(decodeAddress(getApplicationAddress(xAlgoAppId)).publicKey);
- expect(transfer.rcv).toEqual(decodeAddress(admin.addr).publicKey);
+
+ const { algoBalance } = await getXAlgoRate();
+ console.log("proposers algo balance", algoBalance );
+
+ console.log("unclaimable balance", algoBalance - totalActiveStake)
});
+
});
describe("update smart contract", () => {Last updated
Was this helpful?