standard.md

This document aims to clarify the meaning of each bug label and the process we use to classify bugs in our study. We have also included some examples of ambiguous cases at the end of the document for reference.

It is important to note that classifying functional bugs can be a subjective process, and we welcome any suggestions for improving our criteria for bug classification

Bug Labels

We classify the surveyed bugs into three main categories based on their nature:

• Out-of-scope bugs (denoted by O)
• Bugs with simple and general testing oracles (denoted by L)
• Bugs that require high-level semantical oracles (denoted by S)

Out-of-scope Bugs

These are bugs that fall outside the scope of our study and are thus not analyzed further.

• O1: These vulnerabilities can only be exploited by privileged users (e.g., rug pull), or when the privileged users make mistakes (e.g., applying incorrect configuration during deployment).
• O2: We cannot access the source code of the project.
• O3: These vulnerabilities can only be exploited with further actions by victim users (e.g., EIP-4626 inflation attacks)
• O4: Bugs that occur in off-chain components.
• O5: Typo or trivial bugs that render the contract non-deployable or non-functional. We believe these types of bugs are unlikely to occur in contracts that are ready for audit or have been deployed.
• O6: Bugs that are not considered as such by the project. This can be due to disagreements between the auditors and the project (common in early contests), no explicit code affected by the bug, or intentional behavior that aligns with the business model (where the business model may be flawed).
• O7: Doubtful findings, which we believe may be invalid, duplicated, or non-critical (common in early contests).

Bugs with Simple and General Testing Oracles

These are bugs that can be detected using simple and general oracles and do not require an in-depth understanding of the code semantics.

• L1: Reentrancy.
• L2: Rounding issues or precision loss.
• L3: Bugs that are caused by using uninitialized variables.
• L4: Bugs that are caused by exceeding the gas limitation.
• L5: Storage collision and confusion between proxy and implementation.
• L6: Arbitrary external function call.
• L7: Integer overflow and underflow.
• L8: Revert issues caused by low-level calls or external libraries.
• L9: Bugs that are caused by writing to memory that does not apply to the storage.
• LA: Cryptographic issues.
• LB: Using tx.origin.

Bugs that Require High-level Semantical Oracles

These are bugs that require high-level semantical oracles to detect, as they arise from inconsistencies between the code implementation and the business model.

• S1: Price oracle manipulation.
  • S1-1: AMM price oracle manipulation.
  • S1-2: Sandwich attack.
  • S1-3: Non-AMM price oracle manipulation.
• S2: ID-related violations.
  • S2-1: ID can be arbitrarily set by users or lack of ID validation. ID can also be a project-specified variable (e.g., hash) or an address.
  • S2-2: Shared resource (e.g., token) without proper locks.
  • S2-3: ID uniqueness violation (i.e., an ID should be unique but it is not).
• S3: Erroneous state updates.
  • S3-1: Missing state update.
  • S3-2: Incorrect state updates, e.g., a state update that should not be there.
• S4: Business-flow atomicity violations.
  • S4-1: Lack of proper locks for a business flow consisting of multiple transactions.
• S5: Privilege escalation and access control issues.
  • S5-1: Users can update privileged state variables arbitrarily (caused by lack of ID-unrelated input sanitization).
  • S5-2: Users can invoke some functions at a time they should not be able to do so.
  • S5-3: Privileged functions can be called by anyone or at any time.
• S6: Erroneous accounting.
  • S6-1: Incorrect calculating order.
  • S6-2: Returning an unexpected value that deviates from the expected semantics specified for the contract.
  • S6-3: Calculations performed with incorrect numbers (e.g., x = a + b ==> x = a + c).
  • S6-4: Other accounting errors (e.g., x = a + b ==> x = a - b).
• SE: Broken business models due to unexpected operations
  • SE-1: Unexpected function invocation sequences (e.g., external calls to dependent contracts).
  • SE-2: Unexpected environment or contract conditions (e.g., ChainLink returning outdated data or significant slippage occurring).
  • SE-3: A given function is invoked multiple times unexpectedly.
  • SE-4: Unexpected function arguments.
• SC: Contract implementation-specific bugs. These bugs are difficult to categorize into the above categories.

Process

To classify a bug, we follow these steps:

1. First, we validate whether the bug is within the scope of our study. Our focus is on exploitable bugs in smart contracts, so many bugs may be excluded. If the bug falls into any of the O categories, it is considered out of scope.
2. We then validate whether the bug can be found by tools with simple and generic testing oracles. To do so, we investigate how the bug is exploited. If any oracle mentioned by the L categories can detect the exploit, we classify it as an L bug. It is important to note that this is an over-approximation of current vulnerability detection techniques. As long as there is an oracle that can detect the exploit, we assume the detection tool can detect it (regardless of the tool's effectiveness). For example, we assume there is no path exploration issue for symbolic execution and any constraint can be solved in time.
3. Any bugs that remain after the previous steps are labeled as S bugs.
   1. We first investigate the root cause of the bug. If the root cause can be classified as S1 to S6, we label it accordingly.
   2. For the remaining bugs, we investigate how they can be exploited. If the way of exploit matches any of the SE types, we label it accordingly.
   3. Any remaining bugs are labeled as SC.

Ambiguous Cases

Case 1: Referrer can drain ReferralFeePoolV0

Bug Description

function claimRewardAsMochi in ReferralFeePoolV0.sol did not reduce user reward balance, allowing referrer to claim the same reward repeatedly and thus draining the fee pool.

Did not reduce user reward balance at L28-47 in ReferralFeePoolV0.sol

To mitigate the issue, add the following lines

rewards -= reward[msg.sender];

reward[msg.sender] = 0;

Explanation

We have classified this bug as S3-1 since the root cause is related to missing state update regarding rewards and reward[msg.sender]. While it may appear similar to SE-3 since the attacker needs to invoke the claimRewardAsMochi function repeatedly to exploit the bug, we followed our classification process and identified the root cause as S3-1.

Case 2: Miners Can Re-Roll the VRF Output to Game the Protocol

Bug Description

Miners are able to rewrite a chain's history if they dislike the VRF output used by the protocol. Consider the following example:

• A miner or well-funded user is participating in the PoolTogether protocol.
• A VRF request is made and fulfilled in the same block.
• The protocol participant does not benefit from the VRF output and therefore wants to increase their chances of winning by including the output in another block, producing an entirely new VRF output. This is done by re-orging the chain, i.e. following a new canonical chain where the VRF output has not been included in a block.
• This attack can be continued as long as the attacker controls 51% of the network. The miner itself could control a much smaller proportion of the network and still be able to mine a few blocks in succession, although this is of low probability but entirely possible.
• A well-funded user could also pay miners to re-org the chain on their behalf in the form of MEV to achieve the same benefit. The PoolTogether team is aware of this issue but is yet to mitigate this attack vector fully.

Explanation

We have classified this bug as O7, as we believe it is not specific to the subject project, but rather a general question of whether the security model of Chainlink's VRF can be trusted.

Case 3: Logic error in burnFlashGovernanceAsset can cause locked assets to be stolen

Bug Description

A logic error in the burnFlashGovernanceAsset function that resets a user's pendingFlashDecision allows that user to steal other user's assets locked in future flash governance decisions. As a result, attackers can get their funds back even if they execute a malicious flash decision and the community burns their assets.

1. An attacker Alice executes a malicious flash governance decision, and her assets are locked in the FlashGovernanceArbiter contract.
2. The community disagrees with Alice's flash governance decision and calls burnFlashGovernanceAsset to burn her locked assets. However, the burnFlashGovernanceAsset function resets Alice's pendingFlashDecision to the default config (see line 134).
3. A benign user, Bob executes another flash governance decision, and his assets are locked in the contract.
4. Now, Alice calls withdrawGovernanceAsset to withdraw Bob's locked asset, effectively the same as stealing Bob's assets. Since Alice's pendingFlashDecision is reset to the default, the unlockTime < block.timestamp condition is fulfilled, and the withdrawal succeeds.

Referenced code:

• DAO/FlashGovernanceArbiter.sol#L134
• DAO/FlashGovernanceArbiter.sol#L146

To mitigate this issue, it is suggested to change line 134 to delete pendingFlashDecision[targetContract][user] instead of setting the pendingFlashDecision to the default.

Explanation

The bug can be classified as either S3-2 or S6-4, but the root cause is related to the incorrect update of pendingFlashDecision, which is not an accounting issue. Therefore, we have classified this bug as S3-2.

Case 4: Controller does not raise an error when there's insufficient liquidity

Bug Description

When a user tries to withdraw the token from the vault, the vault would withdraw the token from the controller if there's insufficient liquidity in the vault. However, the controller does not raise an error when there's insufficient liquidity in the controller/ strategies. The user would lose his shares while getting nothing.

An MEV searcher could apply this attack on any withdrawal. When an attacker finds an unconfirmed tx that tries to withdraw 1M DAI, he can do such sandwich attack.

1. Deposits USDC into the vault.
2. Withdraw all DAI left in the vault/controller/strategy.
3. Place the victim tx here. The victim would get zero DAI while burning 1 M share. This would pump the share price.
4. Withdraw all liquidity.

All users would be vulnerable to MEV attackers. I consider this is a high-risk issue.

To mitigate this issue, the following steps are recommended:

• First, users pay the slippage when they try to withdraw. I do not find this fair. Users have to pay extra gas to withdraw liquidity from strategy, convert the token, and still paying the slippage. I recommend writing a view function for the frontend to display how much slippage the user has to pay. (Controler.sol#L448-L479)
• Second, the controller does not revert the transaction there's insufficient liquidity. Recommend to revert the transaction when _amount is not equal to zero after the loop finish. (Controller.sol#L577-L622)

Explanation

While this attack is launched through a sandwich attack, the root cause is not related to price changes, and as such, we do not classify it as S1. Instead, we classify it as SC since there is no explicit category for this type of bug.

Case 5: Access restrictions on NotionalV1ToNotionalV2.notionalCallback can be bypassed

Bug Description

The NotionalV1ToNotionalV2.notionalCallback is supposed to only be called from the verified contract that calls this callback but the access restrictions can be circumvented by simply providing sender = this as sender is a parameter of the function that can be chosen by the attacker.

function notionalCallback(
    address sender,
    address account,
    bytes calldata callbackData
) external returns (uint256) {
    require(sender == address(this), "Unauthorized callback");

Explanation

It can be challenging to determine whether a bug should be labeled as S2-1 or S5, as both categories may involve access control issues (especially when the ID is an address). In this case, the root cause is that the attacker can provide an arbitrary ID (i.e., address) to bypass the access control, which is more related to the S2-1 category. Although the bug could also be classified as S5-3, we believe that it is possible to develop an abstract bug model for ID-related issues. Therefore, we use S2-1 as the label for this bug.

Case 6: Anyone can affect deposits of any user and turn the owner of the token

Bug Description

On RCTreasury, we have the method collectRentUser. This method is public, so anyone can call it using whatever user and whatever timestamp.

So, calling this method using user = XXXXX and _timeToCollectTo = type(uint256).max), would make isForeclosed[user] = true.

function collectRentUser(address _user, uint256 _timeToCollectTo)
    public
    override
    returns (
        uint256 newTimeLastCollectedOnForeclosure
    )
{
    require(!globalPause, "Global pause is enabled");
    assert(_timeToCollectTo != 0);
    if (user[_user].lastRentCalc < _timeToCollectTo) {
        uint256 rentOwedByUser = rentOwedUser(_user, _timeToCollectTo);

        if (rentOwedByUser > 0 && rentOwedByUser > user[_user].deposit) {
            // The User has run out of deposit already.
            uint256 previousCollectionTime = user[_user].lastRentCalc;

            /*
        timeTheirDepsitLasted = timeSinceLastUpdate * (usersDeposit/rentOwed)
                              = (now - previousCollectionTime) * (usersDeposit/rentOwed)
        */
            uint256 timeUsersDepositLasts =
                ((_timeToCollectTo - previousCollectionTime) *
                    uint256(user[_user].deposit)) / rentOwedByUser;
            /*
        Users last collection time = previousCollectionTime + timeTheirDepsitLasted
        */
            rentOwedByUser = uint256(user[_user].deposit);
            newTimeLastCollectedOnForeclosure =
                previousCollectionTime +
                timeUsersDepositLasts;
            _increaseMarketBalance(rentOwedByUser, _user);
            user[_user].lastRentCalc = SafeCast.toUint64(
                newTimeLastCollectedOnForeclosure
            );
            assert(user[_user].deposit == 0);
            isForeclosed[_user] = true;
            emit LogUserForeclosed(_user, true);
        } else {
            // User has enough deposit to pay rent.
            _increaseMarketBalance(rentOwedByUser, _user);
            user[_user].lastRentCalc = SafeCast.toUint64(_timeToCollectTo);
        }
        emit LogAdjustDeposit(_user, rentOwedByUser, false);
    }
}

Now, we can do the same for all the users bidding for a specific token.

Finally, I can become the owner of the token by just calling newRental and using a small price. newRental will iterate over all the previous bids and will remove them because they are foreclosed.

Explanation

It's often difficult to differentiate between S2-1 and S5 bugs, and this case is no exception. Although the exploit in this case involves leveraging a feature that allows users to provide arbitrary user values, the root cause of the issue is that the code does not check if timeToCollectTo <= block.timestamp, which enables the attacker to manipulate the privileged isForeclosed[user] state variable in an arbitrary manner. Therefore, we have classified this bug as S5-1.

Case 7: lastUpdatedDay not initialized

Bug Description

The variable lastUpdatedDay in IncentiveDistribution.sol is not (properly) initialized.

This means the function updateDayTotals will end up in a very large loop which will lead to an out of gas error.

Even if the loop would end, the variable currentDailyDistribution would be updated very often. Thus updateDayTotals cannot be performed

Explanation

Although it is certain that this bug belongs to the L category, there is ambiguity regarding its specific type. The root cause is the improper initialization of the lastUpdatedDay variable in IncentiveDistribution.sol. However, the crucial aspect for L bugs is which oracle can detect them, according to our bug classification process. The gas oracle is the most effective in detecting this bug since the updateDayTotals function would lead to an out of gas error. Therefore, we classify this bug as L4.