search

code-423n4 / 2022-02-tribe-turbo-findings

1 stars 0 forks source link

ERC4626 mints token amount, not number of shares #20

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/Rari-Capital/solmate/blob/main/src/mixins/ERC4626.sol#L67

Vulnerability details

Impact

If the number of assets is different from the number of shares, the user will get more or less shares than they expect.

Users don't have to be sophisticated at all, just using the contract as intended can cause users to get more or less of the shares of a vault.

Proof of Concept

Here's a proof of concept:

  1. Alice deposits 1e18 tokens into ERC4626 vault.
  2. Bob deposits 1e18 tokens into ERC4626 vault.
  3. The amount of tokens in the vault doubles (maybe due to yield), so there are 4e18 tokens in the vault
  4. Alice calls mint with 1e18 shares.

We would expect the following: a. Alice now has 2/3rds of the shares (2e18) b. Alice has to transfer 2e18 tokens

Alice correctly has to transfer 2e18 tokens. But she receives 2e18 shares instead of 1e18 shares because of the line of code here. https://github.com/Rari-Capital/solmate/blob/main/src/mixins/ERC4626.sol#L67

Recommended Mitigation Steps

Change amount to shares on that line. Also should check other implementations to ensure this isn't exploitable in any production contracts.

Joeysantoro commented 2 years ago

https://github.com/code-423n4/2022-02-tribe-turbo-findings/issues/18

GalloDaSballo commented 2 years ago

The warden has identified what is most likely a small oversight, which would have drastic consequences in the internal accounting of the Vault. Because of impact, I agree with high severity.

Will make this finding primary because it shows some details.

The sponsor has mitigated

GalloDaSballo commented 2 years ago

Duplicate of #27