code-423n4 / 2022-11-redactedcartel-findings

3 stars 2 forks source link

Attackers can manipulate ERC4626 price per share to take an unfair share of future users #339

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-11-redactedcartel/blob/03b71a8d395c02324cb9fdaf92401357da5b19d1/src/vaults/PirexERC4626.sol#L60 https://github.com/code-423n4/2022-11-redactedcartel/blob/03b71a8d395c02324cb9fdaf92401357da5b19d1/src/vaults/PirexERC4626.sol#L80

Vulnerability details

Impact

The attacker can get funds from future users, and the future users will lose their funds.

Proof of Concept

A malicious early user can deposit() with 1 wei of asset token and get 1 wei of shares. Then he/she can send 10000e18 - 1 of asset tokens and inflate the price per share from 1 to an extreme value of 1e22

(1 + 10000e18 - 1) / 1 = 1e22

A future user who deposits 19999e18 will only receive 1 wei of shares token.

19999e18 * 1 / 10000e18 = 1

he/she would lose 9999e18 if they redeem() right after the deposit().

https://github.com/code-423n4/2022-11-redactedcartel/blob/03b71a8d395c02324cb9fdaf92401357da5b19d1/src/vaults/PirexERC4626.sol#L60

https://github.com/code-423n4/2022-11-redactedcartel/blob/03b71a8d395c02324cb9fdaf92401357da5b19d1/src/vaults/PirexERC4626.sol#L80

Tools Used

Manual Review

Recommended Mitigation Steps

Require minimum amount of share in deposit function and mint function.

c4-judge commented 1 year ago

Picodes marked the issue as duplicate of #407

c4-judge commented 1 year ago

Picodes marked the issue as satisfactory

c4-judge commented 1 year ago

Picodes changed the severity to 3 (High Risk)

C4-Staff commented 1 year ago

JeeberC4 marked the issue as duplicate of #275