View::queryAssetBalances does not account for JIT penalties
| Title | View::queryAssetBalances does not account for JIT penalties |
|---|---|
| Reward | $886, Unique |
| Contest | Ammplify - 2 Sep 2025 on Sherlock |
| Author | tedox |
| Context | Missing Logic |
View::queryAssetBalances reports how many assets a user would receive if they withdrew right now. The value is wrong, because it does not account for the JIT penalty the position may incur.
When a user removes a maker position, the contract calculates the assets to return and then applies the JIT penalty afterwards.
/// @inheritdoc IMaker
function removeMaker(
address recipient,
uint256 assetId,
uint160 minSqrtPriceX96,
uint160 maxSqrtPriceX96,
bytes calldata rftData
) external nonReentrant returns (address token0, address token1, uint256 removedX, uint256 removedY) {
Asset storage asset = AssetLib.getAsset(assetId);
require(asset.owner == msg.sender, NotMakerOwner(asset.owner, msg.sender));
require(asset.liqType == LiqType.MAKER || asset.liqType == LiqType.MAKER_NC, NotMaker(assetId));
PoolInfo memory pInfo = PoolLib.getPoolInfo(asset.poolAddr);
Data memory data = DataImpl.make(pInfo, asset, minSqrtPriceX96, maxSqrtPriceX96, 0);
WalkerLib.modify(pInfo, asset.lowTick, asset.highTick, data);
// Settle balances.
PoolWalker.settle(pInfo, asset.lowTick, asset.highTick, data);
removedX = uint256(-data.xBalance); // These are definitely negative.
removedY = uint256(-data.yBalance);
-> (removedX, removedY) = FeeLib.applyJITPenalties(asset, removedX, removedY);
//@audit JIT penalty applied to the full amount returned
AssetLib.removeAsset(assetId);
address[] memory tokens = pInfo.tokens();
int256[] memory balances = new int256[](2);
balances[0] = -int256(removedX); // We know they fit since they can only be less (in magnitude) than before.
balances[1] = -int256(removedY);
RFTLib.settle(recipient, tokens, balances, rftData);
// Return values
token0 = tokens[0];
token1 = tokens[1];
}However, when the view function is used it does not properly account for the JIT penalty.
function queryAssetBalances(
uint256 assetId
) external view returns (int256 netBalance0, int256 netBalance1, uint256 fees0, uint256 fees1) {
Asset storage asset = AssetLib.getAsset(assetId);
PoolInfo memory pInfo = PoolLib.getPoolInfo(asset.poolAddr);
ViewData memory data = ViewDataImpl.make(pInfo, asset);
ViewWalkerLib.viewAsset(pInfo, asset.lowTick, asset.highTick, data);
if (asset.liqType == LiqType.TAKER) {
uint256 vaultX = VaultLib.balanceOf(pInfo.token0, asset.xVaultIndex, assetId, false);
uint256 vaultY = VaultLib.balanceOf(pInfo.token1, asset.yVaultIndex, assetId, false);
// Balance and fees are owed, and vault balance is owned.
netBalance0 = int256(vaultX) - int256(data.liqBalanceX);
netBalance1 = int256(vaultY) - int256(data.liqBalanceY);
fees0 = data.earningsX;
fees1 = data.earningsY;
} else { //@audit missing JIT penalty
netBalance0 = int256(data.liqBalanceX);
netBalance1 = int256(data.liqBalanceY);
fees0 = data.earningsX;
fees1 = data.earningsY;
}
}The two values then differ by more than 0.01% whenever the JIT penalty exceeds 0.01%, which it usually does.
From the README: Issues that lead to getting incorrect return values (i.e. deviates from the withdrawal value of the asset by more than 0.01%) from the queryAssetBalance function (even if the appropriate input is used), which will lead to issues when executing other functions, may be considered valid with Medium severity at max.
Alpha: view functions are not usually worth a medium, but this protocol explicitly asked for them to be considered.
Conclusion
This finding would earn you $886. The easiest way to find these is to compare the state-changing function with the view function and check they agree — and to read the README.