Submission Details
Severity:
Minting shares based on index token may inflate shares
Summary
PerpetualVault.sol:_mint() may mint shares in two ways - if _isLongOneLeverage is true, depending on amount of indexToken, or depending on amount of collateralToken. This may inflate shares in the first deposit through index token.
Vulnerability Details
In the first deposit depending on amount of collateralToken contract will multiply amount on 1e8. But then when will be first deposit through indexToken total shares amount will be multiplied on amount of indexToken , this will create an enormous shares amount because totalAmountBefore = 1.
/**
* @notice this function is an end of deposit flow.
* @dev should update all necessary global state variables
*
* @param depositId `depositId` of mint operation
* @param amount actual deposit amount. if `_isLongOneLeverage` is `true`, amount of `indexToken`, or amount of `collateralToken`
*/
function _mint(uint256 depositId, uint256 amount, bool refundFee, MarketPrices memory prices) internal {
uint256 _shares;
if (totalShares == 0) {
_shares = depositInfo[depositId].amount * 1e8;
} else {
uint256 totalAmountBefore;
if (positionIsClosed == false && _isLongOneLeverage(beenLong)) {
//@audit depending on index token
totalAmountBefore = IERC20(indexToken).balanceOf(address(this)) - amount;
} else {
totalAmountBefore = _totalAmount(prices) - amount;
}
//@audit if previous index token balance was zero, totalAmountBefore = balance - amount = 0
if (totalAmountBefore == 0) totalAmountBefore = 1;
_shares = amount * totalShares / totalAmountBefore;
}
depositInfo[depositId].shares = _shares;
totalShares = totalShares + _shares;
...
}
PoC
Without code
//formulas
totalAmountBefore = IERC20(indexToken).balanceOf(address(this)) - amount;
totalAmountBefore = totalAmountBefore == 0 ? 1 : totalAmountBefore;
_shares = amount * totalShares / totalAmountBefore;
//initial conditions
indexToken = ETH, 18 decimals
collateralToken = USDC, 6 decimals
//first deposit, token - collateral
collateralAmount = 1000e6
shares = collateralAmount * 1e8 = 1000e6 * 1e8 = 1e17
totalShares = 1e17
//second deposit, token - index
indexTokenBalance = 1e18
indexTokenAmount = 1e18
totalShares = 1e17
totalAmountBefore = 1e18 - 1e18 = 0 == 0 ? 1 : 0 = 1
shares = 1e18 * 1e17 / 1 = 1e35
Shares minted in second deposit is much larger than in first.
With code
Add this test to test/PerpetualVault.t.sol:
function test_WrongDecimalsDeposit() external {
address keeper = PerpetualVault(vault).keeper();
VaultReader vaultReader = VaultReader(address(PerpetualVault(vault).vaultReader()));
//collateral token has 6 decimals (USDC)
IERC20 collateralToken = PerpetualVault(vault).collateralToken();
//index token has 18 decimals (WETH)
IERC20 indexToken = IERC20(PerpetualVault(vault).indexToken());
//create actor
address alice = makeAddr("alice");
uint256 amount = 10000e6;
deal(address(collateralToken), alice, amount * 2);
//set how much shares should get user based on amount
//amount * 1e8
uint256 correctSharesAmount = 1e18;
uint256 executionFee = PerpetualVault(vault).getExecutionGasLimit(true);
//alice makes first deposit
vm.startPrank(alice);
collateralToken.approve(vault, amount * 2);
PerpetualVault(vault).deposit{value: executionFee * tx.gasprice}(amount);
//check if shares are ok
(, uint256 shares0,,,,) = PerpetualVault(vault).depositInfo(1);
assertEq(shares0, correctSharesAmount);
vm.stopPrank();
//create position
MarketPrices memory prices = mockData.getMarketPrices();
bytes[] memory data = new bytes[](1);
data[0] = abi.encode(3380000000000000);
vm.prank(keeper);
PerpetualVault(vault).run(true, false, prices, data);
PerpetualVault.FLOW flow = PerpetualVault(vault).flow();
assertEq(uint8(flow), 2);
assertEq(PerpetualVault(vault).positionIsClosed(), true);
(PerpetualVault.NextActionSelector selector, ) = PerpetualVault(vault).nextAction();
assertEq(uint8(selector), 0);
GmxOrderExecuted(true);
bytes32 curPositionKey = PerpetualVault(vault).curPositionKey();
assertTrue(curPositionKey != bytes32(0));
vm.stopPrank();
//set vault state to get to else block in deposit
PerpetualVault(vault).setVaultState(
PerpetualVault.FLOW.NONE, //NONE flow
uint256(0),
true, //been long
curPositionKey,
false, //positionIsClosed
false, //gmx lock
PerpetualVault.Action(
PerpetualVault.NextActionSelector.NO_ACTION,
abi.encode(true) //been long
) //no action
);
//another deposit, to get to else block
vm.deal(alice, 1 ether);
vm.startPrank(alice);
//already approved
uint256 executionFee2 = PerpetualVault(vault).getExecutionGasLimit(true);
PerpetualVault(vault).deposit{value: executionFee2 * tx.gasprice }(amount);
vm.stopPrank();
bytes[] memory metadata = new bytes[](1);
metadata[0] = abi.encode(PROTOCOL.DEX, mockData.getParaSwapData(vault));
vm.warp(1);
//set vault state to get to INCREASE_ACTION
PerpetualVault(vault).setVaultState(
PerpetualVault.FLOW.DEPOSIT,
uint256(0),
true, //been long
curPositionKey,
false, //positionIsClosed
false, //gmx lock
PerpetualVault.Action(
PerpetualVault.NextActionSelector.INCREASE_ACTION,
abi.encode(true) //been long
) //no action
);
vm.prank(keeper);
PerpetualVault(vault).runNextAction(prices, metadata);
vm.stopPrank();
(, uint256 shares1,,,,) = PerpetualVault(vault).depositInfo(2);
console.log("Shares by collateral deposit: ", shares0);
console.log("Shares by index deposit: ", shares1);
}
Run test in cmd:
forge test -vv --mt test_WrongDecimalsDeposit --via-ir --fork-url https://rpc.ankr.com/arbitrum
Output:
Ran 1 test for test/PerpetualVault.t.sol:PerpetualVaultTest
[PASS] test_WrongDecimalsDeposit() (gas: 5188186)
Logs:
Shares by collateral deposit: 1000000000000000000
Shares by index deposit: 2916781326862221805000000000000000000
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 53.66ms (47.52ms CPU time)
Impact
Shares minted through index token in the first deposit will be much larger than it should be.
Tools Used
Manual Review
Recommendations
Consider converting index token amount to collateral amount through __totalAmount() and mint shares depending on this.
Updates
Lead Judging Commences
n0kto 9 months ago
Submission Judgement Published Reason: Non-acceptable severity
Assigned finding tags:
invalid_totalAmountBefore_is_1_incorrect_calculation_supposition
No proof when this can happen: Most of the time totalAmountBefore equals 0 (balance minus amount sent), it means totalShares equals 0. If it could happen with very specific conditions, report with that tag didn't add the needed details to be validated.
Prize pool breakdown
Total prize
50,000 USDC
nSLOC
1,91026
USDC / LOC
47,500
2,500
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.