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

BaseOFTV2.sendFrom and sendAndCall can be called by anyone to harm the protocol #693

Closed
c4-bot-7 opened this issue Jan 23, 2024 · 11 comments
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working disagree with severity Sponsor confirms validity, but disagrees with warden’s risk assessment (sponsor explain in comments) duplicate-525 edited-by-warden sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") sufficient quality report This report is of sufficient quality unsatisfactory does not satisfy C4 submission criteria; not eligible for awards

Comments

@c4-bot-7
Copy link
Contributor

c4-bot-7 commented Jan 23, 2024

Lines of code

https://github.com/LayerZero-Labs/solidity-examples/blob/ca7d4f1d482df5e17f8aaf1b34d0e4432020bc4e/contracts/token/oft/v2/BaseOFTV2.sol#L15-L45

Vulnerability details

Impact

The Decent token is a token that inherits from BaseOFTV2. There are defined two public functions - sendFrom and sendAndCall. They do not have access controls and can be called by anyone. This can be used by an attacker to block a path in the cross-chain communication or use the protocol without paying fees.

Proof of Concept

LayerZero is blocking by default. That's why NonBlockingLzApp.sol is used - to allow non-blocking behavior. The problem is that currently anyone can call the aforementioned functions with any parameters.

When used by the protocol, the router calls _getCallParams which calculates amounts of gas to be passed.

        uint256 GAS_FOR_RELAY = 100000;
        uint256 gasAmount = GAS_FOR_RELAY + _dstGasForCall;
        adapterParams = abi.encodePacked(PT_SEND_AND_CALL, gasAmount);

This calculation can be entirely skipped by directly calling sendAndCall. An attacker can pass a very small amount of gas.

On the receiving chain, LzApp.lzReceive will be executed which will call _blockingLzReceive.

[_blockingLzReceive] will try to execute the non-blocking functionality.

      // 63/64th of the gas will be forwarded to the call. Since the gas is not enough, success will be false
      (bool success, bytes memory reason) = address(this).excessivelySafeCall(
            gasleft(),
            150,
            abi.encodeWithSelector(this.nonblockingLzReceive.selector, _srcChainId, _srcAddress, _nonce, _payload)
        );

       // We store the failed message with the remaining 1/64th
        if (!success) {
            _storeFailedMessage(_srcChainId, _srcAddress, _nonce, _payload, reason);
        }

As we know, the EVM will forward 63/64th of the available gas to the internal call.
Since the passed gas is little, it will not be enough and success will return false.
This will trigger the if statement below and the message will be stored as failed, leading to blocked channel from one chain to another.

Another bad outcome for the protocol is the ability for users to bypass the fee mechanism and use Decent for free.

Tools Used

Manual Review

Recommended Mitigation Steps

Make sure that sendFrom and sendAndCall are callable only by the router.

Assessed type

Access Control

@c4-bot-7 c4-bot-7 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Jan 23, 2024
c4-bot-7 added a commit that referenced this issue Jan 23, 2024
@c4-bot-6 c4-bot-6 changed the title NativeOFTV2.sendFrom and sendAndCall can be called by anyone to harm the protocol BaseOFTV2.sendFrom and sendAndCall can be called by anyone to harm the protocol Jan 23, 2024
@c4-pre-sort
Copy link

raymondfam marked the issue as sufficient quality report

@c4-pre-sort c4-pre-sort added the sufficient quality report This report is of sufficient quality label Jan 25, 2024
@c4-pre-sort
Copy link

raymondfam marked the issue as duplicate of #212

@raymondfam
Copy link

raymondfam commented Jan 25, 2024

Inadequate integration substantiated by solidity-example file.

@c4-pre-sort c4-pre-sort reopened this Jan 25, 2024
@c4-pre-sort
Copy link

raymondfam marked the issue as not a duplicate

@c4-pre-sort
Copy link

raymondfam marked the issue as primary issue

@c4-sponsor
Copy link

wkantaros marked the issue as disagree with severity

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

wkantaros (sponsor) confirmed

@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 Jan 30, 2024
@c4-judge c4-judge closed this as completed Feb 1, 2024
@c4-judge c4-judge added duplicate-697 and removed primary issue Highest quality submission among a set of duplicates labels Feb 1, 2024
@c4-judge
Copy link

c4-judge commented Feb 1, 2024

alex-ppg marked issue #697 as primary and marked this issue as a duplicate of 697

@alex-ppg
Copy link

alex-ppg commented Feb 1, 2024

The Warden has failed to properly substantiate the vulnerability in contrast to its primary duplicate.

In detail, the Warden highlights an incorrect segment of code (it behaves as expected), states that the failed message will be stored which is incorrect (the vulnerability is that it might not be stored), and generally fails to identify the actual vulnerability and impact.

@c4-judge
Copy link

c4-judge commented Feb 1, 2024

alex-ppg marked the issue as unsatisfactory:
Insufficient quality

@c4-judge c4-judge added the unsatisfactory does not satisfy C4 submission criteria; not eligible for awards label Feb 1, 2024
@c4-judge
Copy link

c4-judge commented Feb 9, 2024

alex-ppg marked the issue as duplicate of #525

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 disagree with severity Sponsor confirms validity, but disagrees with warden’s risk assessment (sponsor explain in comments) duplicate-525 edited-by-warden sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") sufficient quality report This report is of sufficient quality unsatisfactory does not satisfy C4 submission criteria; not eligible for awards
Projects
None yet
Development

No branches or pull requests

7 participants