Share price manipulation is possible for the first depositor of AutoPxGlp

[code423n4]

Lines of code

https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/vaults/AutoPxGlp.sol#L304-L356
https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/PirexGmx.sol#L422-L465

Vulnerability details

Impact

Share price manipulation is possible for the first depositor of AutoPxGlp by depositing small amount first to AutoPxGlp and then transferring some pxGMX tokens to AutoPxGlp directly.

Proof of Concept

When users deposit into AutoPxGlp, new pxGMX tokens are minted for the AutoPxGlp. To know how many assets are controlled by AutoPxGlp it uses totalAssets function.
https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/vaults/AutoPxGlp.sol#L142-L144

    function totalAssets() public view override returns (uint256) {
        return asset.balanceOf(address(this));
    }

When user provides tokens he wants to deposit they are sent to PirexGmx which then mints some amount of pxGMX tokens for AutoPxGlp. Later using that amount of minted tokens and totalAssets amount the amount of shares in AutoPxGlp is calculated.
https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/vaults/AutoPxGlp.sol#L304-L322

    function _deposit(uint256 assets, address receiver)
        internal
        returns (uint256 shares)
    {
        // Check for rounding error since we round down in previewDeposit.
        uint256 supply = totalSupply;


        if (
            (shares = supply == 0
                ? assets
                : assets.mulDivDown(supply, totalAssets() - assets)) == 0
        ) revert ZeroShares();


        _mint(receiver, shares);


        emit Deposit(msg.sender, receiver, assets, shares);


        afterDeposit(receiver, assets, shares);
    }

Also you should note that it's possible for anyone to mint pxGMX tokens directly through the PirexGmx contract to have some pxGMX.
https://github.com/code-423n4/2022-11-redactedcartel/blob/main/src/PirexGmx.sol#L422-L465

    function depositFsGlp(uint256 amount, address receiver)
        external
        whenNotPaused
        nonReentrant
        returns (
            uint256,
            uint256,
            uint256
        )
    {
        if (amount == 0) revert ZeroAmount();
        if (receiver == address(0)) revert ZeroAddress();


        // Transfer the caller's fsGLP (unstaked for the user, staked for this contract)
        stakedGlp.transferFrom(msg.sender, address(this), amount);


        // Get the pxGLP amounts for the receiver and the protocol (fees)
        (uint256 postFeeAmount, uint256 feeAmount) = _computeAssetAmounts(
            Fees.Deposit,
            amount
        );


        // Mint pxGLP for the receiver (excludes fees)
        pxGlp.mint(receiver, postFeeAmount);


        // Mint pxGLP for fee distribution contract
        if (feeAmount != 0) {
            pxGlp.mint(address(pirexFees), feeAmount);
        }


        emit DepositGlp(
            msg.sender,
            receiver,
            address(stakedGlp),
            0,
            0,
            0,
            amount,
            postFeeAmount,
            feeAmount
        );


        return (amount, postFeeAmount, feeAmount);
    }

This all allows first depositor to manipulate with share price.
1.Attacker deposit through PirexGmx.depositFsGlp(or any similar function) and mints huge amount of pxGMX tokens.
2.Attacker deposit(as first depositor) through AutoPxGlp.depositFsGlp(or any similar function) with minimum amount.
3.Attacker transfer his big amount of pxGMX tokens to AutoPxGlp.
4.Now because of big amount of totalAssets and small amount of totalSupply all next depositors will lose some tokens because of rounding while attacker will benefit as he will receive more tokens.

Tools Used

VsCode

Recommended Mitigation Steps

Restrict first depositor of AutoPxGlp to deposit big amount of assets to mint big amount of shares on start.

[c4-judge]

Picodes marked the issue as duplicate of #407

[c4-judge]

Picodes marked the issue as partial-50

[Picodes]

The mitigation is incorrect, and the root cause of the issue is not really identified

[C4-Staff]

JeeberC4 marked the issue as duplicate of #275