Skip to content

Latest commit

 

History

History
59 lines (33 loc) · 2.95 KB

045.md

File metadata and controls

59 lines (33 loc) · 2.95 KB

Damp Cornflower Albatross

Medium

Missing Allowance Reset Before safeIncreaseAllowance in the BalancerRouter

Summary and Impact

The BalancerRouter contract contains a vulnerability where the safeIncreaseAllowance function is used to increase token allowances without first resetting the allowance to zero. This can lead to unintentionally high allowances for the Balancer Vault, Pool, and PreDeposit contracts.

This is a vulnerability because the standard ERC20 approve function has a known race condition that can be exploited. safeIncreaseAllowance was designed to mitigate this by only allowing increases to the allowance. However, if used repeatedly without resetting to zero, it leads to an ever-increasing allowance.

If left unaddressed, this vulnerability could result in users inadvertently granting excessively large allowances to the Balancer Vault, Pool, and PreDeposit contracts.

Vulnerability Details

The BalancerRouter contract facilitates interactions with Balancer pools and the Plaza protocol. It uses safeIncreaseAllowance in several functions:

  • joinBalancerAndPredeposit()
  • joinBalancerAndPlaza()
  • joinBalancerPool()
  • exitPlazaAndBalancer()
  • exitBalancerPool()

In each of these functions, safeIncreaseAllowance is called without first resetting the allowance to zero using approve(spender, 0).

Code Snippet:

Here's one example from joinBalancerAndPredeposit():

https://github.com/sherlock-audit/2024-12-plaza-finance/blob/main/plaza-evm/src/BalancerRouter.sol#L23-L40

Violation of Invariants and Core Principles:

This vulnerability violates the principle of least privilege. The Balancer Vault, Pool, and PreDeposit contracts should only have the minimum necessary allowance to perform their functions. By unintentionally increasing the allowance, the BalancerRouter grants these contracts more power than they need. It also violates the implicit invariant that the allowance granted to a contract should reflect the user's explicit intention, not be silently increased due to the internal mechanics of the protocol. The documentation makes no mention of this behavior either, making this unexpected for the user.

Tools Used

  • Manual Review

Recommendations

The BalancerRouter contract should be modified to reset the allowance to zero before calling safeIncreaseAllowance. This should be done in all affected functions:

  • joinBalancerAndPredeposit()
  • joinBalancerAndPlaza()
  • joinBalancerPool()
  • exitPlazaAndBalancer()
  • exitBalancerPool()

By adding balancerPoolToken.safeApprove(_predeposit, 0); before the safeIncreaseAllowance call, the allowance is reset to zero, ensuring that the subsequent increase only grants the intended allowance. This change should be implemented consistently across all functions in the BalancerRouter that use safeIncreaseAllowance. This will prevent unintended allowance increases and mitigate the identified vulnerability.