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

CodeDesignz - [M-1] L1CrossDomainMessenger::_sendNativeTokenMessage has no check for _amount value 0, resulting in the relayMessage to trigger even for the 0 value #68

Open
sherlock-admin4 opened this issue Sep 25, 2024 · 0 comments

Comments

@sherlock-admin4
Copy link
Contributor

sherlock-admin4 commented Sep 25, 2024

CodeDesignz

Medium

[M-1] L1CrossDomainMessenger::_sendNativeTokenMessage has no check for _amount value 0, resulting in the relayMessage to trigger even for the 0 value

Summary

L1CrossDomainMessenger::_sendNativeTokenMessage has no check for _amount value 0, resulting in the relayMessage to trigger even for the 0 value. This means than the function call will not fail even when the amount is zero in the call.

Root Cause

There is a missing check for when _amount is equal to 0 in L1CrossDomainMessenger::_sendNativeTokenMessage function.
https://github.com/sherlock-audit/2024-08-tokamak-network/blob/main/tokamak-thanos/packages/tokamak/contracts-bedrock/src/L1/L1CrossDomainMessenger.sol#L191

Internal pre-conditions

  1. L1StandardBridge::_initiateBridgeNativeToken is initated with 0 value for the native token amount.
  2. sendNativeTokenMessage function is invoked on L1CrossDomainMessenger which in turn calls _sendNativeTokenMessage function.
  3. _sendNativeTokenMessage triggers _sendMessage which emits the SentMessage and SentMessageExtension1 events. Thoughout this chain, we are not checking if the amount is equal to 0. This results in the event emitted for the zero value as well.

External pre-conditions

No response

Attack Path

No response

Impact

The users will be able to trigger native token calls with zero token amount as well.

PoC

No response

Mitigation

Add a conditional to handle the case when _amount is equal to 0. For example, if a revert is to be expected, than and else clause can be added to to the L1CrossDomainMessenger::_sendNativeTokenMessage function.

{
        // Collect native token
        if (_amount > 0) {
            address _nativeTokenAddress = nativeTokenAddress();
            IERC20(_nativeTokenAddress).safeTransferFrom(_sender, address(this), _amount);
            IERC20(_nativeTokenAddress).approve(address(portal), _amount);
        }
+       else {
+           revert("Native Token amount should be greater than 0")
+       }
        // Triggers a message to the other messenger. Note that the amount of gas provided to the
        // message is the amount of gas requested by the user PLUS the base gas value. We want to
        // guarantee the property that the call to the target contract will always have at least
        // the minimum gas limit specified by the user.
        _sendMessage(
            address(otherMessenger),
            baseGas(_message, _minGasLimit),
            _amount,
            abi.encodeWithSelector(
                this.relayMessage.selector, messageNonce(), _sender, _target, _amount, _minGasLimit, _message
            )
        );
@sherlock-admin4 sherlock-admin4 changed the title Obedient Fuzzy Horse - [M-1] L1CrossDomainMessenger::_sendNativeTokenMessage has no check for _amount value 0, resulting in the relayMessage to trigger even for the 0 value CodeDesignz - [M-1] L1CrossDomainMessenger::_sendNativeTokenMessage has no check for _amount value 0, resulting in the relayMessage to trigger even for the 0 value Oct 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant