sherlock-audit/2024-04-xkeeper-judging

MrPotatoMagic - No guarantee if constant _NATIVE_TOKEN in OpenRelay is same as immutable NATIVE_TOKEN in AutomationVault

Closed this issue · 1 comments

MrPotatoMagic

medium

No guarantee if constant _NATIVE_TOKEN in OpenRelay is same as immutable NATIVE_TOKEN in AutomationVault

Summary

When a automation vault is deployed using the AutomationVaultFactory function deployAutomationVault(), it allows the owner to supply the native token address as a parameter as well, which is set during construction of the vault as an immutable variable.

Vulnerability Detail

The issue is that this native token set as an immutable variable in the AutomationVault can be different than the one used from Constants.sol in the OpenRelay.sol contract (see here).

Impact

Due to this, the following scenarios could occur:

  1. If both native tokens are same, ETH payment is correctly sent to the recipient.
  2. If both are different, the call reverts since ERC20 transfer function does not exist on address 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE

We are interested in case 2 above, since that would mean that none of the callers would be able to execute jobs for the owner. This would also mean waste of gas on the job execution that was done before we revert during payment issuing.

The wasting of gas on job execution could also be seen as a griefing attack intentionally setup by an owner to cause harm to bots/callers aiming to profit from open/public opportunities.

Code Snippet

Function exec() snippet:

  • _NATIVE_TOKEN passed on Line 39 to the automation vault is the constant from Constants.sol
File: OpenRelay.sol
39:     _feeData[0] = IAutomationVault.FeeData(_feeRecipient, _NATIVE_TOKEN, _payment);
40:     _automationVault.exec(msg.sender, new IAutomationVault.ExecData[](0), _feeData);

AutomationVault exec():

  • NATIVE_TOKEN on Line 444 is the immutable variable set in the constructor here during deployment here through the factory.
File: AutomationVault.sol
444:       if (_feeInfo.feeToken == NATIVE_TOKEN) {
445:         (_success,) = _feeInfo.feeRecipient.call{value: _feeInfo.fee}('');
446:         if (!_success) revert AutomationVault_NativeTokenTransferFailed();
447:       } else {
448:         IERC20(_feeInfo.feeToken).safeTransfer(_feeInfo.feeRecipient, _feeInfo.fee);
449:       }

Tool used

Manual Review

Recommendation

During deployment in the factory contract, make sure the native token is hardcoded to the value from Constants.sol to ensure all native tokens are correctly issued during payments and jobs are executed correctly.

Duplicate of #22

1 comment(s) were left on this issue during the judging contest.

Kose commented:

Although this is correct and hardcoded NATIVE_TOKEN for all vaults would be better the loss of fund require user input error. NATIVE_TOKEN should be inputted as 0xeeee.. by all users in every chain. Can be Low/Invalid.