Funds in the pool could be stolen by exploiting `flashSwap` in `HybridPool`

[code423n4]

Handle

broccoli

Vulnerability details

Impact

An attacker can call the bento.harvest function during the callback function of a flash swap of the HybridPool to reduce the number of input tokens that he has to pay to the pool, as long as there is any unrealized profit in the strategy contract of the underlying asset.

Proof of Concept

1. The HybridPool accounts for the reserve and balance of the pool using the bento.toAmount function, which represents the actual amount of assets that the pool owns instead of the relative share. The value of toAmount could increase or decrease if the bento.harvest function is called (by anyone), depending on whether the strategy contract earns or loses money.
2. Supposing that the DAI strategy contract of Bento has a profit not accounted for yet. To account for the profit, anyone could call harvest on Bento with the corresponding parameters, which, as a result, increases the elastic of the DAI token.
3. Now, an attacker wants to utilize the unrealized profit to steal funds from a DAI-WETH hybrid pool. He calls flashSwap to initiate a flash swap from WETH to DAI. First, the pool transfers the corresponding amount of DAI to him, calls the tridentSwapCallback function on the attacker's contract, and expects that enough DAI is received at the end.
4. During the tridentSwapCallback function, the attacker calls bento.harvest to realize the profit of DAI. As a result, the pool's bento.toAmount increases, and the amount of DAI that the attacker has to pay to the pool is decreased. The attacker could get the same amount of ETH but paying less DAI by exploiting this bug.

Referenced code: HybridPool.sol#L218-L220 HybridPool.sol#L249-L250 HybridPool.sol#L272-L285 BentoBoxV1Flat.sol#L1105 BentoBoxV1Flat.sol#L786-L792 BentoBoxV1Flat.sol#L264-L277

Recommended Mitigation Steps

Consider not using bento.toAmount to track the reservers and balances, but use balanceOf instead (as done in the other two pools).

[maxsam4]

Stableswap needs to use toAmount balances rather shares to work. This issue allows skimming yield profits from the pool. There's no user funds at risk but still an issue.

We plan on resolving this by using a fixed toElastic ratio during the whole swap.