Skip to content
This repository has been archived by the owner on Nov 24, 2024. It is now read-only.

zzykxx - depositSteth() and depositeEth() will revert #119

Closed
sherlock-admin4 opened this issue May 24, 2024 · 2 comments
Closed

zzykxx - depositSteth() and depositeEth() will revert #119

sherlock-admin4 opened this issue May 24, 2024 · 2 comments
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label High A valid High severity issue Reward A payout will be made for this issue Will Fix The sponsor confirmed this issue will be fixed

Comments

@sherlock-admin4
Copy link
Contributor

sherlock-admin4 commented May 24, 2024

zzykxx

medium

depositSteth() and depositeEth() will revert

Summary

Vulnerability Detail

The protocol always assumes that the amount of tokens received is equal to the amount of tokens transferred.
This is not the case for rebasing tokens, such as stETH and eETH, because internally they transfer shares which generally results in the received amount of tokens being lower than the requested one by a couple of wei because of roundings: transferring 1e18 eETH tokens from A to B, will result in B receiving 0.99e18 eETH tokens.

Sophon deals with rebasing token transfers in 3 functions:

As an example the depositeEth() takes as input an amount of eETH to deposit, then:

  1. Transfers amount of eETH from the caller to the contract itself
  2. Wraps amount of eETH to weETH, which will attempt to transfer amount from the contract to the Etherfi protocol.

Step 2 will fail, because the contract doesn't have enough eETH. The issue lies in attempting to wrap amount of eETH in step 2 instead of wrapping the actual amount of tokens received.

This also applies to the other two functions listed above, depositSteth() and deposit() (if a stETH and/or eETH pool exists).

Impact

The functions depositSteth() and depositeEth() will revert.

Code Snippet

Tool used

Manual Review

Recommendation

In depositSteth(), depositeEth() and deposit() deposit the actual amount of tokens received instead of the amount specified as input.

Duplicate of #63

@github-actions github-actions bot added High A valid High severity issue Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label labels May 28, 2024
@sherlock-admin3
Copy link
Contributor

1 comment(s) were left on this issue during the judging contest.

0xmystery commented:

valid because this parallels FOT as documented by Lido, just not sure if this would apply to eETH too.

@sherlock-admin3 sherlock-admin3 added the Will Fix The sponsor confirmed this issue will be fixed label May 29, 2024
@sherlock-admin3 sherlock-admin3 changed the title Happy Aegean Crab - depositSteth() and depositeEth() will revert zzykxx - depositSteth() and depositeEth() will revert Jun 1, 2024
@sherlock-admin3 sherlock-admin3 added the Reward A payout will be made for this issue label Jun 1, 2024
@sherlock-admin2
Copy link
Contributor

The protocol team fixed this issue in the following PRs/commits:
sophon-org/farming-contracts@73adb67

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label High A valid High severity issue Reward A payout will be made for this issue Will Fix The sponsor confirmed this issue will be fixed
Projects
None yet
Development

No branches or pull requests

3 participants