Asymmetry contest - eyexploit's results

Lines of code

https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/derivatives/WstEth.sol#L86-L88 https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/SafEth.sol#L71-L75 https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/SafEth.sol#L92-L95

Vulnerability details

Impact

WstEth contract is expected to receive derivative price in ETH when call to ethPerDerivative. But the function returns the price of WstETH in terms of stETH. And since, stETH does not have the same value as ETH the output price return is incorrect.

Consider the mentioned comments for WstETH derivative,

    function stake() external payable {
        ...
        // Getting underlying value in terms of ETH for each derivative
        for (uint i = 0; i < derivativeCount; i++)
            underlyingValue +=
                (derivatives[i].ethPerDerivative(derivatives[i].balance()) *
                    derivatives[i].balance()) /
                10 ** 18;  // @audit 01: returns stETH amount not the ETH actually

        ...
        if (totalSupply == 0)
            preDepositPrice = 10 ** 18; // initializes with a price of 1
        else preDepositPrice = (10 ** 18 * underlyingValue) / totalSupply;  // @audit 02: effects the safETH token price here 

        ...
        // mintAmount represents a percentage of the total assets in the system
        uint256 mintAmount = (totalStakeValueEth * 10 ** 18) / preDepositPrice; // @audit 03: totalStakeValueEth holds the stETH value not ETH, minting incorrect amount of safETH token.
        _mint(msg.sender, mintAmount);
        emit Staked(msg.sender, msg.value, mintAmount);
    }

Proof of Concept

The function getStETHByWstETH() only converts WstETH to stETH. Thus, price() returns the value of WstETH in terms of stETH, check comment.

    /**
        @notice - Get price of derivative in terms of ETH
     */
    function ethPerDerivative(uint256 _amount) public view returns (uint256) {
        return IWStETH(WST_ETH).getStETHByWstETH(10 ** 18); 
    }

Tools Used

Manual review

Recommended Mitigation Steps

Use stEth/ETH curve pool for price, the same way its doing SfrxETH.sol

Lines of code

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

Vulnerability details

Impact

While making a swap, AMM provide their user a deadline parameter which make sure the swap txn will only be executed under that deadline timestamp.

If no deadline is passed, the txn can be pending and will be executed in a long time after the user submit the transaction. At that time, the trade can be done in a poor price, which harms user's position.

Proof of Concept

For proof of concept, there is mentioned a similar issue here, with consequences of missing deadline check and like also how the similar txn can exploited maliciously through MEV.

Tools Used

Manual review

Recommended Mitigation Steps

It is recommended the protocol to add deadline.

Lines of code

https://github.com/code-423n4/2023-03-asymmetry/blob/44b5cd94ebedc187a08884a7f685e950e987261c/contracts/SafEth/SafEth.sol#L88

Vulnerability details

Impact

User stake ETH to the main contract SafEth.sol, the sent ETH amount is diversify between derivatives based on the weights[derivativeIndex]. Each weight for individual derivative constitute the totalWeight, if some irregualar weight or user sent ETH amount with upto large decimal. There will always be left some unused ETH into the contract.

Proof of Concept

Add this test to test/SafEth.test.ts

it("POC", async () => {
  // divided weight into 3 derivatives
  // 1e18, 1e18, 1e18  
  const preEthBalance = await ethers.provider.getBalance(safEthProxy.address);
  await safEthProxy.stake({value: ethers.utils.parseEther("1")});
  const postEthBalance = await ethers.provider.getBalance(safEthProxy.address);

  expect(Number(postEthBalance.sub(preEthBalance))).to.be.greaterThan(0); 
})

Tools Used

Manula review

Recommended Mitigation Steps

After making the deposit, transfer left dust back to the user .