Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Free loans issue #325

Open
c4-submissions opened this issue Nov 14, 2023 · 10 comments
Open

Free loans issue #325

c4-submissions opened this issue Nov 14, 2023 · 10 comments
Labels
bug Something isn't working disagree with severity Sponsor confirms validity, but disagrees with warden’s risk assessment (sponsor explain in comments) downgraded by judge Judge downgraded the risk level of this issue edited-by-warden grade-a insufficient quality report This report is not of sufficient quality Q-02 QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax

Comments

@c4-submissions
Copy link
Contributor

c4-submissions commented Nov 14, 2023

Lines of code

https://github.com/code-423n4/2023-10-badger/blob/f2f2e2cf9965a1020661d179af46cb49e993cb7e/packages/contracts/contracts/BorrowerOperations.sol#L439-L547
https://github.com/code-423n4/2023-10-badger/blob/f2f2e2cf9965a1020661d179af46cb49e993cb7e/packages/contracts/contracts/BorrowerOperations.sol#L553-L594

Vulnerability details

Summary

Lido usually rebases stEth once a day at 12 pm UTC. Badger makes profit by taking a percentage of the rewards distributed among stEth token holders. It is possible for a user to open a CDP right after the rebase and close this CDP just before the next rebase. This way, they can use eBTC tokens for their purposes during these 24 hours (for example, for arbitrage) without paying anything to the protocol. This action can be repeated for a long period, such as a year, allowing the user to both use eBTC and keep the entire reward from stEth tokens for themselves. The expenses are in the form of gas for transactions. Based on my calculations, with a large enough amount of stEth, the user will come out with a profit at the expense of the protocol.

Impact

Financial losses for the protocol, which are a function of the amount of stEth and the duration of the vulnerability exploitation. An easy-to-exploit issue, where the more stEth is used, the greater the profit for the attacker.

Proof of Concept

This POC simulates the exploit for a period of 1 year. You can see that at the end of the period the difference in the balance of shares is insignificant (Initial: 10000000000000000000000 After 1 year: 9999999999999999999847). The balances before and after are not equal because of rounding errors along the way. And can conclude that the protocol was not able to collect their fees.

    function testFreeLoan() public 
    {
        uint256 collAmount = 30 ether;
        uint256 netColl = collAmount - borrowerOperations.LIQUIDATOR_REWARD();
        address user = _utils.getNextUserAddress();
        address user2 = _utils.getNextUserAddress();

        // in order to close a CDP later in the code need at least one active CDP
        vm.startPrank(user2);
        vm.deal(user2, type(uint96).max);
        collateral.approve(address(borrowerOperations), type(uint256).max);
        collateral.deposit{value: 10000 ether}();

         // Calculate borrowed amount
        uint256 borrowedAmount = _utils.calculateBorrowAmount(
            collAmount,
            priceFeedMock.fetchPrice(),
            COLLATERAL_RATIO
        );
        borrowerOperations.openCdp(borrowedAmount, HINT, HINT, collAmount);
        vm.stopPrank();

        vm.startPrank(user);
        vm.deal(user, type(uint96).max);
        collateral.approve(address(borrowerOperations), type(uint256).max);
        collateral.deposit{value: 10000 ether}();

        console.log("Initial shares balance: %s", collateral.sharesOf(user));

        bytes32 cdpId;

        //365 days
        for(uint256 i; i < 365; i++)
        {
            // for simplicity always increase the index
            _applyIndexChange(1e16);

            //console.log("stEth balance before openCdp: %s", collateral.balanceOf(user));

            // Calculate borrowed amount
            borrowedAmount = _utils.calculateBorrowAmount(
                collAmount,
                priceFeedMock.fetchPrice(),
                COLLATERAL_RATIO
            );
            borrowerOperations.openCdp(borrowedAmount, HINT, HINT, collAmount);

            // Get new CDP id
            cdpId = sortedCdps.cdpOfOwnerByIndex(user, 0);

            //wait 1 day in which the malicious user could make whatever wants with the eBTC amount
            vm.warp(block.timestamp + 86400);

            //close CDP before the rebase
            borrowerOperations.closeCdp(cdpId);
        }

        vm.stopPrank();

        //there is small change caused by a rounding errors along the way
        console.log("Shares balance after 1 year: %s", collateral.sharesOf(user));
    }

    function _applyIndexChange(int indexChange) internal returns (uint256, uint256) {
        uint256 currentIndex = collateral.getPooledEthByShares(1e18);
        uint256 proposedIndex;

        if (indexChange >= 0) {
            proposedIndex = currentIndex + uint256(indexChange);
        } else if (indexChange < 0) {
            // if we will be above zero
            if (currentIndex >= uint256(indexChange)) {
                proposedIndex = currentIndex - uint256(indexChange);
            }
            // handle zero / underflow
            else {
                proposedIndex = 0;
            }
        }

        collateral.setEthPerShare(proposedIndex);
        return (currentIndex, collateral.getPooledEthByShares(1e18));
    }

Finally i want to show that this attack can be profitable for the attacker. From the gas report can be seen that the average gas cost for openCDP is 294753 and for closeCDP it is 43736, total 338489. It costs < $30 at that moment.

30 * 365 = $10950 cost for the attack for 1 year

The APR for stEth in Lido is 4%. So the attacker need > 273750 in \stEth to make a profit. For example if an attacker has $1 000 000 in stEth the profit for 1 year would be $1000000 * 4% - 10950.

Tools Used

Manual review

Recommended Mitigation Steps

A couple of ideas:

  • add a fixed fee for loans that last less than 1 day
  • forbid loans that last less than 1 day
  • i also noticed that there is no low limit for the flash loans fee. If you make this fee very low or 0 someday an attacker can use the flash loans to close CDP.

Assessed type

Other

@c4-submissions c4-submissions added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Nov 14, 2023
c4-submissions added a commit that referenced this issue Nov 14, 2023
@bytes032
Copy link

If the "attacker" has 273750 ETH, this is not an attack, but rather using the protocol and receiving rewards as expected.

@c4-pre-sort c4-pre-sort added the insufficient quality report This report is not of sufficient quality label Nov 16, 2023
@c4-pre-sort
Copy link

bytes032 marked the issue as insufficient quality report

@GalloDaSballo
Copy link

This finding should be awarded as QA, we are aware that you can skip the rebase fee

However, the finding only works for people that deposited collateral without any significant usage of their tokens

For example, a Delta Neutral LP will have to pay 30 BPS on withdrawal to re-balance it's single sided withdrawal, meaning that skipping the rebase is actually a loss in every case except for people that are giving the protocol free yield

@c4-sponsor c4-sponsor added the disagree with severity Sponsor confirms validity, but disagrees with warden’s risk assessment (sponsor explain in comments) label Nov 20, 2023
@c4-sponsor
Copy link

GalloDaSballo marked the issue as disagree with severity

@jhsagd76
Copy link

I think there is a typo, it should be $273750 stETH. However I agree with the sponsor. The profits of such an attack would most likely be smaller than the losses, making it impossible to carry out. It should be QA

@c4-judge c4-judge added downgraded by judge Judge downgraded the risk level of this issue QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax and removed 3 (High Risk) Assets can be stolen/lost/compromised directly labels Nov 25, 2023
@c4-judge
Copy link

jhsagd76 changed the severity to QA (Quality Assurance)

@c4-judge
Copy link

jhsagd76 marked the issue as grade-a

@gstoyanovbg
Copy link

@GalloDaSballo @jhsagd76 I would probably agree with you on the severity of the finding if we exclude the financial significance of BTC, and consequently, of eBTC as a token tracking the price of BTC. I won't go into detail about its characteristics; we are all familiar with them. I'll just note that Bitcoin is the least risky cryptocurrency, and due to its mechanics, the price measured in $ is expected to rise in the long term. A similar asset, which is a good store of value in traditional finance, is gold. These properties of Bitcoin make it the most common choice for reserves in the crypto world. Keeping this in mind, in the following lines, I will describe a specific scenario demonstrating how a user can take advantage of the vulnerability described in this report.

Let's imagine that we are in the position of a crypto asset manager who manages funds for other people looking to have exposure to the crypto sector. Our strategy includes trading various tokens/currencies with the aim of higher profits. Volatility in these areas is significant, and even the best analysis cannot guarantee that we will not incur losses. To protect the investors' funds and reduce the risk for the entire portfolio, we decide to maintain reserve assets with lower risk that can cover potential losses in the long run. The obvious choice is BTC. Over the next 2 years, a bull market is expected, so even if we accumulate losses measured in $, they will be covered by the increasing price of BTC. We decide to use eBTC. In a balanced portfolio, there should be assets that generate a constant income, even in times of a bear market (dividend stocks in the traditional financial world). We choose stETH, a proven protocol with good APR. We trade the remaining part of the portfolio.

Due to some anomaly, the price of token X significantly drops compared to BTC (and eBTC). Positions in protocols lending eBTC against X become eligible for liquidation. Fundamental analysis shows us that this is an anomaly, and the old ratio will be restored in the future. We decide to take advantage of the situation. However, we need eBTC. It is unacceptable to use the reserves of eBTC; they guarantee the investors' funds and it is unacceptable to risk them. It is not a good time to buy, or we don't have enough free assets. We can use our stETH to take a loan from Badger. However, we are not allowed to risk the annual interest from Lido; it is included in our risk management plan. It turns out that there is a way to preserve it and take a loan. We take a loan with our stETH and liquidate the positions. We get a larger amount of X (for example, +3%). We just have to wait for the old prices to recover to lock in profits. Until then, every day, we have to roll the position in Badger to avoid paying fees to Badger. To do this, we will use part of the reserves with eBTC. This is an almost risk-free operation; eBTC from the reserve will stay in Badger for only a few blocks until the stETH rebase occurs. In case of doubts about the health of the protocol, the operation will not be initiated. A special case of what has been described is the liquidation of positions in Badger. A variation of what has been described so far is arbitrage in the X/eBTC pool. The possibilities are many.

In conclusion, I want to emphasize that the described risk management scenario is popular, and the vulnerability described in Badger provides an opportunity for every crypto asset manager to gain additional profit from their reserves, with almost no risk for them.

P.S It is correct that there is a typo in the original report, $273750 stETH not 273750 stETH ))

@jhsagd76
Copy link

jhsagd76 commented Dec 6, 2023

Firstly, I believe that both the sponsor and I have a clear understanding of technology and exploit scenarios with Warden. However, I also believe that given the numerous assumptions of the economic model, this issue is not significant enough to be marked as MED.

Considering that eBTC differs significantly from traditional collateralized assets, such as LUSD and YUSD, in fee design, as it does not have a fee for opening postion. I believe it is necessary to carefully consider mitigation for this issue, while ensuring not affect capital efficiency.

@gstoyanovbg
Copy link

gstoyanovbg commented Dec 6, 2023

@jhsagd76 I don't understand which assumptions you mean with 'numerous assumptions of the economic model':

  1. Price drop of one token compared to another - it happens all the time. I don't think it's necessary to provide historical examples.
  2. A portfolio structured in the described way is entirely normal for traders who want to reduce their risk. We are not talking about a portfolio that no one would use. Moreover, a user who is aware of this issue in Badger can simply structure their portfolio this way for that reason.
  3. The existence of pools that use eBTC, or protocols that lend eBTC, is the logical consequence of the adoption of eBTC.

I don't see any assumptions that are unlikely to happen. On the contrary, each of the conditions is very likely to be fulfilled. Also, this is just an example aiming to refute the claims from the comments above, and the goal was not to focus precisely on the probability of each component of the scenario.

From the actions described, the malicious user gains funds, and the protocol loses funds from uncollected fees. So the factor of fund loss is present. It is easy to execute, although not at every moment, due to the described economic assumptions. But it is certain that the necessary conditions will be met at certain times, and considering the volatility of the market, these moments will not be rare. And when they occur, serious profits can be made. There are no other limitations preventing execution.

Ultimately, if the difference between QA and >QA is how likely the necessary conditions for the malicious user to lock in profits are, I can try to provide historical examples. I don't know if they would carry any weight. However, if a decision has already been made based on subjective factors and cannot be changed, that is within the judge's authority. I will accept the decision even if I don't understand it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working disagree with severity Sponsor confirms validity, but disagrees with warden’s risk assessment (sponsor explain in comments) downgraded by judge Judge downgraded the risk level of this issue edited-by-warden grade-a insufficient quality report This report is not of sufficient quality Q-02 QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax
Projects
None yet
Development

No branches or pull requests

10 participants