Redacted Cartel contest - KingNFT's results

Lines of code

https://github.com/code-423n4/2022-11-redactedcartel/blob/03b71a8d395c02324cb9fdaf92401357da5b19d1/src/PirexGmx.sol#L615 https://github.com/code-423n4/2022-11-redactedcartel/blob/03b71a8d395c02324cb9fdaf92401357da5b19d1/src/PirexGmx.sol#L685 https://github.com/code-423n4/2022-11-redactedcartel/blob/03b71a8d395c02324cb9fdaf92401357da5b19d1/src/PirexGmx.sol#L712

Vulnerability details

Impact

The following 'redeem' related functions are likely to be blocked, users will not be able to retrieve their funds.

function _redeemPxGlp(
    address token,
    uint256 amount,
    uint256 minOut,
    address receiver
);
function redeemPxGlpETH(
    uint256 amount,
    uint256 minOut,
    address receiver
);
function redeemPxGlp(
    address token,
    uint256 amount,
    uint256 minOut,
    address receiver
);

Proof of Concept

The 'GlpManager' contract of GMX has a 'cooldownDuration' limit on redeem/unstake ('_removeLiquidity()'). While there is at least one deposit/stake ('_addLiquidity()') operation in the past 'cooldownDuration' time, redemption would fail. Obviously this limitation is user-based, and 'PirexGmx' contract is one such user.

https://github.com/gmx-io/gmx-contracts/blob/c3618b0d6fc1b88819393dc7e6c785e32e78c72b/contracts/core/GlpManager.sol#L234

Current setting of 'cooldownDuration' is 15 minutes, the max value is 2 days.

https://arbiscan.io/address/0x321f653eed006ad1c29d174e17d96351bde22649#readContract

Due to the above limit, there are 3 risks that can block redemption for Pirex users.

1.The normal case Let's say there is 10% GMX users will use Pirex to manage their GLP. By checking recent history of GMX router contract, we can find the average stake interval is smaller than 1 minute https://arbiscan.io/address/0xa906f338cb21815cbc4bc87ace9e68c87ef8d8f1 Let's take

averageStakeIntervalOfGMX = 30 seconds

So if Pirex has 10% of GMX users, then

averageStakeIntervalOfPirex = 30 ÷ 10% = 300 seconds

The probability of successfully redeeming is a typical Poisson distribution: https://en.wikipedia.org/wiki/Poisson_distribution With

λ = cooldownDuration ÷ averageStakeIntervalOfPirex = 15 * 60 ÷ 300 = 3
k = 0

So we get

P ≈ 1 ÷ (2.718 * 2.718 * 2.718) ≈ 0.05 

Conclusion

If Pirex has 10 % of GMX users, then the redemption will fail with 95% probability.

A full list of % of GMX users versus failure probability of redemption

1% : 26%
5% : 78%
10% : 95%
20% : 99.75%
30% : 99.98%

2.The attack case If an attacker, such as bad competitors of similar projects, try to exploit this vulnerability. Let's estimate the cost for attack.

As attacker can deposit a very small GLP, such as 1 wei, so we can ignore the GLP cost and only focus on GAS cost.

By checking the explorer history https://arbiscan.io We are safe to assume the cost for calling 'depositGlpETH()' or 'depositGlp' is

txCost = 0.1 USD

To block redemption, attacker have to execute a deposit call every 15 minutes, so

dailyCost = 24 * (60 / 15) * 0.1 = 9.6 USD
yearCost = 365 * 9.6 = 3504 USD

Conclusion

If an attacker want to block Pirex users funds, his yearly cost is only about 3.5k USD.

3.GMX adjusts protocol parameters If GMX increases 'cooldownDuration' to 2 days, it will obviously cause redemption not working.

Tools Used

VS Code

Recommended Mitigation Steps

Reserve some time range for redemption only. e.g. 1 of every 7 days.

Lines of code

https://github.com/code-423n4/2022-11-redactedcartel/blob/03b71a8d395c02324cb9fdaf92401357da5b19d1/src/vaults/PirexERC4626.sol#L60 https://github.com/code-423n4/2022-11-redactedcartel/blob/03b71a8d395c02324cb9fdaf92401357da5b19d1/src/vaults/PirexERC4626.sol#L80

Vulnerability details

Impact

There is a front running attack vector on users who call 'deposit()' or 'mint()' of 'PirexERC4626' based contracts ( 'AutoPxGlp' and 'AutoPxGmx') while the initial share of contract is 0. Attack can steal up to half of the user's deposit amount.

Proof of Concept

Let's take an example of 'AutoPxGmx' contract to explain the attack steps.

1.Victim user call 'deposit()' with parameter

assets = 2000 pxGMX = 2000e18 wei pxGMX

and the transaction is cached in the memory pool of node.

2.Attacker front running the following 2 transactions (1) send 1000 pxGMX to contract directly, now

totalAssets = 1000 pxGMX = 1000e18 wei pxGMX

(2) call 'deposit()' with parameter

assets = 1e-18 pxGMX = 1 wei pxGMX

Now

apxGMXBalanceOfAttacker = 1 wei apxGMX
totalAssets = 1000e18 + 1 wei pxGMX
totalSupply = 1 wei apxGMX

3.Victim's transaction executes, then

apxGMXBalanceOfVictim = assets * totalSupply / totalAssets = 2000e18 * 1 / (1000e18 + 1) = 1 wei apxGMX
totalAssets = 3000e18 + 1 wei pxGMX
totalSupply = 2 wei apxGMX

4.Attacker call 'redeem()' with

shares = 1 wei apxGMX

then

assets = shares * totalAssets / totalSupply = 1 * (3000e18 + 1) / 2 = 1500e18 = 1500 pxGMX
penalty = assets * 300 / 10000 = 45 pxGMX

received

assets - penalty = 1500 - 45 = 1455 pxGMX

and profit

 1455 - 1000 = 455 pxGMX

Attacker can draw out more token from victim by increasing deposit and shares, the up limit is half of the user's deposit amount.

Related functions:

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);
}

function convertToShares(uint256 assets)
    public
    view
    virtual
    returns (uint256)
{
    uint256 supply = totalSupply; // Saves an extra SLOAD if totalSupply is non-zero.

    return supply == 0 ? assets : assets.mulDivDown(supply, totalAssets());
}

Tools Used

VS Code

Recommended Mitigation Steps

Admin deposits some token in 'constructor' and never redeem.

constructor(
    address _gmxBaseReward,
    address _gmx,
    address _asset,
    string memory _name,
    string memory _symbol,
    address _platform,
    address _rewardsModule
) Owned(msg.sender) PirexERC4626(ERC20(_asset), _name, _symbol) {
    // ...

    deposit(0.01 ether, address(this)); // @fix
}