code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-05-maia/blob/78e49c651fd119b85bb79296620d5cb39efa7cdd/src/hermes/bHermes.sol#L115-L117

Vulnerability details

Impact

An early depositor can break the future minting of shares by minting a very small number of shares, but donating a large amount of the underlying token afterwards such that each wei of the shares token is worth a large amount of the underlying.

If one wei of the token is worth, say, 1 Eth, if a second user calls deposit() or mint() with a value less than this amount, the number of shares they get back round down to zero due to loss of precision and the call will revert, essentially bricking the contract for users with small amounts. If a user instead deposits an amount larger than 1 Eth, the amounts over multiples of one whole Eth is given to all shareholders due to loss of precision. An attacker can do this from the start of the contract to regularly skim small amounts, or can wait and see if the first deposit is a large one, and front-run it with the right amount to steal up to half of the large deposited amount.

The root cause of the issue is that the solmate/solady implementations of ERC-4626 rely on the function totalAssets() in order to determine how many shares to mint in both the preview() and convertTo() functions, and the implementation of totalAssets() here relies on the contract's balance of underlying tokens, and this balance can be manipulated outside of the minting/depositing functions.

Proof of Concept

File: src/hermes/bHermes.sol

115      function totalAssets() public view virtual override returns (uint256) {
116          return address(asset).balanceOf(address(this));
117:     }
https://github.com/code-423n4/2023-05-maia/blob/78e49c651fd119b85bb79296620d5cb39efa7cdd/src/hermes/bHermes.sol#L115-L117

Tools Used

IllIllI-bot

Recommended Mitigation Steps

One way to prevent this sort of attack is to, upon initialization, mint an initial amount of shares to an address that is not able to withdraw them, so the amount of underlying needed to cause a loss of precision makes the attack infeasible. The OpenZepelin implementation starting with version 4.9 has this protection built in and enabled by default.