Protocol can be repeatedly gas griefed in AutoRange external call

[c4-bot-7]

Lines of code

https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/transformers/AutoRange.sol#L111-L272

Vulnerability details

Summary

Revert controlled AutoRange bot can be gas griefed and execute() reverted by malicious onERC721Received implementation

Vulnerability Details

The initiator of a transaction pays the transaction gas; in the case of AutoRange::execute() and AutoRange::executeWithVault(), this will be a Revert controlled bot which is set up as an operator.
Newly minted NFTs are sent to users via NPM::safeTransferFrom() which uses the onERC721Received callback.
An attacker can implement a malicious implementation of this callback; waste all the transaction gas and revreting the function to grief the protocol.
It is expected that the gas spent by bots initiating transactions will be covered by protocol fees; however no protocol fees will be generated from the attacker's position as AutoRange::execute() will not complete; so the protocol will experience a loss.

Furthermore, once attacker has set the token's config from positionConfigs, the protocol has no way to stop the griefing occuring each time the bot detects that the tokenId meets the conditions for a Range Change.
Token Config is only removed from positionConfigs at the end of execute() which the gas grief will prevent from being reached making it a recurring attack.
The only recourse to the protocol is shutting down the contract completely by removing the bot address as an operator and DOSing the contract.

All this makes the likelihood of this attack quite high as it is a very inexpensive attack; user does not even need an open position and loan in the vault.
A determined attacker

POC

Attacker would need to create a malicious contract to which they send their NPM NFT.
Via this contract they can then add token config for this NFT to the AutoRange contract via AutoRange::configToken().
The malicious contract would need to have a malicious implementation such as the one below which uses as much gas as possible before reverting.

    function onERC721Received(
        address operator,
        address from,
        uint256 tokenId,
        bytes calldata data
    ) external override returns (bytes4) {

    uint256 initialGas = gasleft();
    uint256 counter = 0;

    // Loop until only small amount gas is left for the revert
    uint256 remainingGasThreshold = 5000;

    while(gasleft() > remainingGasThreshold) {

        counter += 1;
    }

    // Explicitly revert transcation
    revert("Consumed the allotted gas");
        
    }

Impact

Protocol fees can be completely drained; particularly if a determined attacker sets token configs for multiple NFTs in AutoRange all linked to the same malicious contract.
Lack of fees can DOS multiple functions like the bot initiated AutoRange functions and affect the protocol's profitability by draining fees.

Tools Used

Manual Review
Foundry Testing

Recommendations

Enact a pull mechanism by transferring the newly minted NFT to a protocol owned contract such as the AutoRange contract itself from where the user initiates the transaction to transfer the NFT to themselves.

Assessed type

Other

[c4-pre-sort]

0xEVom marked the issue as sufficient quality report

[c4-pre-sort]

0xEVom marked the issue as primary issue

[0xEVom]

Grouping all gas griefing submissions under this but note that there are multiple versions of it:

• on onERC721Received()
• on receive() fallback from low-level call
• by consuming all available gas
  • and reverting
  • without reverting
• via return bomb as shown in Gas griefing/theft is possible on unsafe external call #29

[c4-sponsor]

kalinbas (sponsor) acknowledged

[kalinbas]

All these cases are possible but we are monitoring these off-chain bots and also implement gas-limiting, and taking action where needed.

[jhsagd76]

valid gas grief, but not a persistent dos, so M

[c4-judge]

jhsagd76 changed the severity to 2 (Med Risk)

[c4-judge]

jhsagd76 marked the issue as satisfactory

[c4-judge]

jhsagd76 marked the issue as selected for report

[Qormatic]

Hi @jhsagd76,

When user adds their position's config details via configToken(); the positionConfigs mapping is updated accordingly.
From what i can see, there is no way to remove that config apart from at the end of execute().
Everytime the parameters defined in positionConfigs are met, the bot will call execute(), get griefed and user's token config will remain in state.
A malicious user can set up multiple positionConfigs to grief with many different parameter trigger points and the only recourse would be the shutting down of the AutoRange contract.
Once a new contract is set up of course the exact same thing can be done again so I think it's a strong case for a full DOS of this part of the protocol's functionality and loss of funds for the protocol which would be more than dust.

[kalinbas]

The logic which positions to execute is the responsability of the bot. If there are worthless tokens detected or the tx simulation is not what expected the bot doesn't execute the operation. So this is a risk which is controlled off-chain.

[Qormatic]

How would that work? It doesn't seem possible to foresee getting griefed at least the first time and after that would the tokenId be blacklisted to prevent further griefing which necessitates the shutting down of the contract?

Also, the mitigation of bots monitoring the contract is not documented under the list of known issues of the Contest's README so i think it's fair to flag this issue and leave up to the judge to decide if mitigation can be applied retrospectively after the contest.

[jhsagd76]

I cannot understand what the warden is referring to with the "shutting down of the AutoRange contract". There is no reason for the operator to waste gas on a transaction that continuously fails. Gas griefing is valid, and it will indeed continue to deplete the resources of the off-chain operator, but this does not constitute a substantial DoS. For predictions on any future actions/deployments, please refer to code-423n4/org#72 .