[swaps] Check Balancer security audit

[maltekliemann]

balancer.fi published a security audit for Balancer v1: https://github.com/balancer-labs/balancer-core/blob/master/Trail%20of%20Bits%20Full%20Audit.pdf

We should review which of these findings apply to our implementation of Balancer pools.

• 1. Single-asset liquidity functions allow stealing of assets
• 2. Lack of events in setBLabs is error-prone
• 3. Parameters' order of single-asset functions is confusing
• 4. Assets will be lost in case of token migrations
• 5. Users can silently burn tokens with transfers to 0x0
• 6. Privileged addresses can be transferred without confirmation even to invalid values
• 7. Users can join and exit pools even where there are no tokens
• 8. Single-asset exit functions allow withdrawing of a negligible amount of assets for free
• 9. Assets with low decimals or low liquidity lead to withdraw a negligible amount of assets for free
• 10. Rounding issues in joinPool/exitPool allow for a negligible amount of free pool tokens
• 11. Attackers with large funds can steal the pool's assets
• 12. The normalized sum of the weight is not always equal to 1
• 13. Pools with a large total supply cause SWAP functions to always revert
• 14. Token balance limits are declared but not enforced
• 15. The swap-in and swap-out ratios are not correctly enforced

[maltekliemann]

2, 4, 5, 6 , 7 seem to be EVM-only problems or based on Balancer's concept of a non-finalized pool.

[maltekliemann]

3 is resolved in Balancer and Zeitgeist

[maltekliemann]

1 is also based on Balancer's "controller" address and non-finalized pools. Note that this vulnerability is not related to #496.

[maltekliemann]

Regarding 8, the report later states (p. 44):

This seems to have been mitigated by a check that prevents zero division errors from reaching zero.

Not sure what "reaching zero" refers to, or what zero division errors this is refering to, but we have a check that ensures that calc_pool_in_given_single_out doesn't return zero.

Furthermore, the report states on the same page:

While it is still possible to take advantage of rounding errors [...], it incurs and extremely high cost to the attacker.

This checks out. Rounding errors can also be "exploited" to withdraw more liquidity from the pool than you own, but even in pools with extremely large liquidity, these "profit" is well below the required amount of transaction fees.

Finally, the report suggests the following long-term solution on p. 27:

Favor exact amount-in functions over exact amount-out.

I agree. I don't see the use case for exact amount-out. On the other hand, this issue displays a conceptual security issue associated with exact amount-out extrinsics.

[maltekliemann]

Regarding 10, see the comments above. To elaborate on the scope of this issue, in a pool with 10,000,000,000,000,000 shares (that's the equivalent of 1 million ZTG in outcome tokens plus another 1 million ZTG; this pool would contain 2% of the genesis supply of ZTG, which seems unrealistic), it is possible to remove an extra 1,000,000 of liquidity, but this would cost about 1,000,000,000 in transaction fees.

[maltekliemann]

9 deals with small token balances, and kind of highlights why it's important to 1) require a sufficiently large amount of minimum liquidity and 2) ensure that no balance in the pool gets drained close to zero (using a in/out limit of, say, 30%).

As in the case of 8 and 10, this is a matter of scale. Given our existential balance of 1 cent, this seems unproblematic to me.

[maltekliemann]

Regarding 12, I've raised this issue: zeitgeistpm/documentation#37

[maltekliemann]

15 seems to be lacking detail. I can't say I see the issue here.

[maltekliemann]

14 doesn't seem to pertain to our implementation. Should our pools have a maximum balance?

[maltekliemann]

13 is fixed in Balancer and seems to have been cause by letting the controller specify the amount of pool tokens to be minted when finalizing the pool. None of this plays a role in our code.

[maltekliemann]

11 - doesn't seem possible to do this attack thanks to in/out ratios.