#46592 [SC-High] The return value of redeemFromAgent/redeemFromAgentInCollateral in the selfCloseExitTo is not checked
Submitted on Jun 2nd 2025 at 05:53:32 UTC by @ox9527 for Audit Comp | Flare | FAssets
Report ID: #46592
Report Type: Smart Contract
Report severity: High
Target: https://github.com/flare-foundation/fassets/blob/main/contracts/assetManager/implementation/CollateralPool.sol
Impacts:
Permanent freezing of funds
Description
Brief/Intro
In CollateralPool.sol::selfCloseExitTo(), the user spends an amount of requiredFAssets to redeem collateral from the agent. However, the actual value redeemed may vary, and the return value is not validated. This could potentially lead to a loss of funds for the user.
Vulnerability Details
From the code CollateralPool.sol->selfCloseExitTo->_selfCloseExitTo()->assetManager.redeemFromAgent()
// redeem f-assets if necessary
if (requiredFAssets > 0) {
if (requiredFAssets < assetManager.lotSize() || _redeemToCollateral) {
assetManager.redeemFromAgentInCollateral(
agentVault, _recipient, requiredFAssets);
} else {
// automatically pass `msg.value` to `redeemFromAgent` for the executor fee
assetManager.redeemFromAgent{ value: msg.value }( //@audit the return value is not checked.
agentVault, _recipient, requiredFAssets, _redeemerUnderlyingAddress, _executor); <@
}
}
And then RedemptionRequests.sol->Redemptions.closeTickets()
AssetManagerState.State storage state = AssetManagerState.get();
// redemption tickets
uint256 maxRedeemedTickets = Globals.getSettings().maxRedeemedTickets;
uint64 lotSize = Globals.getSettings().lotSizeAMG;
for (uint256 i = 0; i < maxRedeemedTickets && _closedAMG < _amountAMG; i++) {
// each loop, firstTicketId will change since we delete the first ticket
uint64 ticketId = state.redemptionQueue.agents[_agent.vaultAddress()].firstTicketId;
if (ticketId == 0) {
break; // no more tickets for this agent
}
RedemptionQueue.Ticket storage ticket = state.redemptionQueue.getTicket(ticketId);
uint64 maxTicketRedeemAMG = ticket.valueAMG + _agent.dustAMG;
maxTicketRedeemAMG -= maxTicketRedeemAMG % lotSize; // round down to whole lots
uint64 ticketRedeemAMG = SafeMath64.min64(_amountAMG - _closedAMG, maxTicketRedeemAMG); <@
// only remove from tickets and add to total, do everything else after the loop
removeFromTicket(ticketId, ticketRedeemAMG);
_closedAMG += ticketRedeemAMG;
}
From the code above, we can see that the loop stops early in two cases: when maxRedeemedTickets
is reached, or when there are no more tickets available for this agent.
Both of these cases can result in the actual redeemed amount being smaller than expected.
However, the final closedUBA
obtained is not actually verified, which may result in the actual closedUBA
being smaller than expected. As a result, the user may end up overpaying in requiredFAssets
.
Impact Details
User paid more FAssets then expected
References
if (requiredFAssets > 0) {
if (requiredFAssets < assetManager.lotSize() || _redeemToCollateral) {
assetManager.redeemFromAgentInCollateral(
agentVault, _recipient, requiredFAssets);
} else {
// automatically pass `msg.value` to `redeemFromAgent` for the executor fee
assetManager.redeemFromAgent{ value: msg.value }( //@audit the return value is not checked.
agentVault, _recipient, requiredFAssets, _redeemerUnderlyingAddress, _executor);
}
}
Proof of Concept
Proof of Concept
First add console.log to file:RedemptionRequests.sol::redeemFromAgent()
// burn the closed assets
console.log("closedUBA:",closedUBA);
Redemptions.burnFAssets(msg.sender, closedUBA);
}
Track the actually closedUBA amount.
add test to Redemption.ts:
it.only("mint and redeem from agent and redeem from agent in collateral branch test", async () => {
const agentVault = await createAgent(agentOwner1, underlyingAgent1);
await depositAndMakeAgentAvailable(agentVault, agentOwner1, toWei(3e10));
// minter
chain.mint(underlyingMinter1, toBNExp(10000000, 18));
await updateUnderlyingBlock();
const lots = 1;
const agentInfo = await assetManager.getAgentInfo(agentVault.address);
const crFee = await assetManager.collateralReservationFee(lots);
const resAg = await assetManager.reserveCollateral(agentVault.address, lots, agentInfo.feeBIPS, ZERO_ADDRESS, [], { from: minterAddress1, value: crFee });
const crt = requiredEventArgs(resAg, 'CollateralReserved');
const paymentAmount = crt.valueUBA.add(crt.feeUBA);
const txHash = await wallet.addTransaction(underlyingMinter1, crt.paymentAddress, paymentAmount, crt.paymentReference);
const proof = await attestationProvider.provePayment(txHash, underlyingMinter1, crt.paymentAddress);
const res = await assetManager.executeMinting(proof, crt.collateralReservationId, { from: minterAddress1 });
const minted = requiredEventArgs(res, 'MintingExecuted');
// redeemer "buys" f-assets
await fAsset.transfer(redeemerAddress1, minted.mintedAmountUBA, { from: minterAddress1 });
// redemption request
collateralPool = await CollateralPool.at(await assetManager.getCollateralPool(agentVault.address));
await impersonateContract(collateralPool.address, toBN(512526332000000000), accounts[0]);
//Only collateral pool can redeem from agent
const rs = await assetManager.redeemFromAgent(agentVault.address, redeemerAddress1, toBN(40000000000000000000), underlyingRedeemer1, ZERO_ADDRESS, { from: collateralPool.address });
});
We use the amount is 40 ETH, and the output closedUBA is not this value:
npx hardhat test test/unit/fasset/library/Redemption.ts
...
closedUBA: 31200000000000000000
...
Was this helpful?