balancer / balancer-v3-monorepo

GNU General Public License v3.0
28 stars 6 forks source link

Security constraints #92

Open jubeira opened 10 months ago

jubeira commented 10 months ago

Transient API and composable pools may intensify certain types of attacks, but they are not the inherent problem. To ensure safety, we should:

Note: in this context it is not enough to e.g. mint a small amount of BPT to ensure the balance never goes below a certain amount. You can always trade on debt with transient accounting, even without holding the tokens in the first place, given that everything's settled at the end.

jubeira commented 9 months ago

Treat BPT 'differently'? Don't allow e.g. inflating the supply in the middle of a tx.

Note: in this context it is not enough to e.g. mint a small amount of BPT to ensure the balance never goes below a certain amount. You can always trade on debt with transient accounting, even without holding the tokens in the first place, given that everything's settled at the end.

BPT is not transient since #105. This means that you cannot drain the pool without owning the BPT, and sending some tokens to address(0) during initialization could help in that regard. Limiting trade amounts would still be a good idea.

You can still inflate the supply on debt, which is something worth analyzing.

jubeira commented 8 months ago

BPT is not transient since https://github.com/balancer/balancer-v3-monorepo/pull/105.

We'll need to review this for batched operations (#154). In a batch swap with nested pools in the middle, you might need to burn BPT that is transient in the previous step.

E.g. let's assume some form of bb-a-usd paired with GHO, and the user wants to trade GHO for DAI. The route will be GHO --> bb-a-usd --> DAI, which means:

After the first step, the caller will have a credit in bb-a-usd, and the operation will fail in the current implementation when trying to burn that credit.

It would be OK to burn from the credit as long as the amount is capped to the liquidity that the caller can get from the first pool. The worst case scenario would be equivalent to getting a flashloan and swapping everything in the GHO / bb-a-usd pool. In other words, burning from credit with caps doesn't add an extra attack vector that cannot be done with an external flashloan.