Reentrancy with ERC677/777 tokens gives rise to unlimited balance (no funds at risk - yet)
Report ID
#12997
Target
Report type
Smart Contract
Impacts
Direct theft of any user funds, whether at-rest or in-motion, other than unclaimed yield
Has PoC?
Yes
Bug Description
Account balance calculation in TokenFacet is vulnerable to reentrancy, which leads to potentially unlimited balance for ERC777 tokens - some of which are listed in CurveRegistry w/ $10M-$20M pool balances.
Impact
Currently none, this will change if Beanstalk adds any ERC777 tokens to its Deposit Whitelist i.e. pBTC, imBTC, AMP etc.
Risk Breakdown
Difficulty to Exploit: Medium Weakness: Reentrancy allows attacker to inflate their account internal balance, and potentially drain the core Beanstalk Protocol pool if any ERC777 tokens become eligible on the whitelist. CVSS2 Score:
Recommendation
And reentrancy guard on TokenFacet OR on the vulnerable functions - transferToken and receiveToken in the LibTransfer library. The latter is recommended, as LibTransfer is used across multiple Beanstalk Facet contracts.
References
Whitehat has previously discovered similar inflated balance bugs in Wormhole, Thorstarter etc.
Proof of concept
Let me know if you need the following coded. Here's the pseudo:
- Call function transferToken from TokenFacet per delegatecall in Beanstalk with an ERC777 token
- transferToken balance calculation in LibTransfer does the following:
- Measure initial contract balance
- Call safetransferfrom on token contract -> Attacker re-enters here using _calltokenstosend function of ERC777 tokens
- Measure final contract balance
- Balance credited is equal to final balance - initial contract balance
- During reentry, the attacker calls transferToken again, thereby securing a low initial contract balance
- Balance credited is then erroneously calculated due to incorrect initial balance
Taking an example of 100 units of an arbitrary ERC777 token (initially with 0 balance), only looped twice
- On first loop, initial balance = 0
- Attacker re-enters during Safetransferfrom loop to call transferTokens again with 100 units
- On second loop, initial balance = 0
- Attacker chooses to not re-enter during safetransferfrom and 100 units are transferred
- Final balance calculation for second loop: balanceOf(beanstalk) = 100
- Final balance for first loop = 200
- For the second loop, attacker is credited (correctly): 100 - 0 = 100
- For the first loop, attacker is erroneously credited: 200 - 0 (wrong!) = 200
Hence, for a 200 unit token transfer, attacker is credited 100 + 200 = 300.
This effect magnifies with each successive loop due to initialBalance being erroneously measured as '0' due to re-entry.
With this erroneously high balance, attacker can then carry out an attack on Beanstalk i.e. trade this token and its BDV for other assets, or withdraw this balance if there is a sizeable chunk of ERC777 tokens already in pool.
Again, this is currently NOT profitable as there are no ERC777 tokens on the deposit whitelist, so this report is more a warning if this were to change in future.
BIC Response
This submission is out of scope because it is related to code / functionality that is not deployed. The submission explicitly states that the impact is none as the Deposit Whitelist does not currently support ERC 777 tokens. Thus, there is no risk either.
Because of these reasons, this submission not eligible for a reward.
Halborn Response
If you are not supporting ERC777 tokens right now, then it’s not a risk.