Asymmetry contest - Viktor_Cortess's results

Lines of code

https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/derivatives/Reth.sol#L241 https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/derivatives/Reth.sol#L91-L101

Vulnerability details

Impact

I want to demonstrate how frontrunning with the flash loan can affect stake function causing errors and reverting transactions.

Proof of Concept

I chose WETH/rETH pool on Uniswap V3 because I'm more familiar with this exchange. But I think similar operations are possible with the other 2 derivatives.

First I tried to manipulate the price in the pool to see if it was possible to mint more shares during staking, but the result wasn't very significant. Staking 0.5 ETH with a flashloan of 1460 WETH (almost all WETH in the pool) could bring only about 0.006 WETH in profit (without expenses for gas and assuming the flashloan is free and could be borrowed without fees from Balancer).

During these attempts, I noticed 2 possible errors:

1)Error: VM Exception while processing transaction: reverted with panic code 0x11 (Arithmetic operation underflowed or overflowed outside of an unchecked block) in Reth.sol contract in this line :

241: return (sqrtPriceX96 * (uint(sqrtPriceX96)) * (1e18)) >> (96 * 2);

This error happens if the stake amount is 1 ETH and the size of flashloan is 1470 eth in my test. The sqrtPriceX96 = 1461446703485210103287273052203988822378723970341

it.only("WST Flash", async () => {
  let wethWhale = "0x0C4809bE72F9E117D75381438c5dAeC8AbE75BaD"
  let WETH = "0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2";
  let rETH = "0xae78736Cd615f374D3085123A210448E74Fc6393";
  const ethDepositAmount = "1";

  const weiDepositAmount = ethers.utils.parseEther(ethDepositAmount);
  const weiDepositAmount2 = ethers.utils.parseEther("1470");
  
  await network.provider.request({
    method: "hardhat_impersonateAccount",
    params: [wethWhale],
  });
  const whaleSigner = await ethers.getSigner(wethWhale);
  const erc20 = new ethers.Contract("0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2", ERC20.abi, whaleSigner);
  
  await erc20.transfer(derivatives[0].address, weiDepositAmount2); //I used swapExactInputSingleHop from Reth.sol to imitate trade with flashloan on Uniswap V3 to make the process faster.

  let amountOut = await derivatives[0].swapExactInputSingleHop(WETH,rETH,500,weiDepositAmount2,0)
  const tx1 = await safEthProxy.stake({ value: weiDepositAmount });

})

2) Error: VM Exception while processing transaction: reverted with reason string 'Too little received happens if I change amounts of stake and flashloan:

   const ethDepositAmount = "100";

  const weiDepositAmount = ethers.utils.parseEther(ethDepositAmount);
  const weiDepositAmount2 = ethers.utils.parseEther("1450"); 

Protection from slippage works, but the transaction reverts.

Tools Used

VC

Recommended Mitigation Steps

In one case we see that slippage protection works, but in the case of overflow, some changes should be made to the formula in the return statement of the poolPrice() function.

[L-01] Derivative contracts could be initialized with 0 address owner

There is no checking of 0 address, so this code from the Test showing deployment of derivative contracts works:

 describe("Derivatives", async () => {
  let derivatives = [] as any;
  beforeEach(async () => {
  await resetToBlock(initialHardhatBlock);
  derivatives = [];
  const factory0 = await ethers.getContractFactory("Reth");
  const factory1 = await ethers.getContractFactory("SfrxEth");
  const factory2 = await ethers.getContractFactory("WstEth");

  const derivative0 = await upgrades.deployProxy(factory0, [
    "0x0000000000000000000000000000000000000000",
  ]);
  await derivative0.deployed();

This could lead to redeployment and reinitialization.

[L-02] Accepting ownership makes the process of transferring ownership safer

Recommend considering implementing a two-step process where the owner or admin nominates an account and the nominated account needs to call an acceptOwnership() function for the transfer of ownership to fully succeed. This ensures the nominated EOA account is a valid and active account.

[N-01 ] INCONSISTENT SOLIDITY VERSIONS

Avoid floating pragmas for non-library contracts.

While floating pragmas make sense for libraries to allow them to be included with multiple different versions of applications, it may be a security risk for application implementations.

A known vulnerable compiler version may accidentally be selected or security tools might fall back to an older compiler version ending up checking a different EVM compilation that is ultimately deployed on the blockchain.

It is recommended to pin to a concrete compiler version.

All files: pragma solidity ^0.8.13;

[N-02] NON-USAGE OF SPECIFIC IMPORTS

All files.

It’s possible to name the imports to improve code readability