Skip to content
This repository was archived by the owner on May 23, 2023. It is now read-only.

Commit aff9e90

Browse files
scnalescnale
authored
Negative test for a reentrant attack on the core relayer forward mechanism (#83)
* Modifies the relayer simulation to be easier to use in negative tests. * Adds negative test for a reentrancy attack on the forward mechanism. * `forge fmt` run.
1 parent 98b54ff commit aff9e90

File tree

2 files changed

+193
-10
lines changed

2 files changed

+193
-10
lines changed

‎ethereum/contracts/mock/AttackForwardIntegration.sol

+66
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
// SPDX-License-Identifier: UNLICENSED
2+
pragma solidity ^0.8.17;
3+
4+
import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
5+
6+
import "../interfaces/IWormhole.sol";
7+
import "../interfaces/IWormholeReceiver.sol";
8+
import "../interfaces/ICoreRelayer.sol";
9+
10+
/**
11+
* This contract is a malicious "integration" that attempts to attack the forward mechanism.
12+
*/
13+
contract AttackForwardIntegration is IWormholeReceiver {
14+
mapping(bytes32 => bool) consumedMessages;
15+
address attackerReward;
16+
IWormhole wormhole;
17+
ICoreRelayer core_relayer;
18+
uint32 nonce = 1;
19+
uint16 targetChainId;
20+
21+
// Capture 30k gas for fees
22+
// This just needs to be enough to pay for the call to the destination address.
23+
uint32 SAFE_DELIVERY_GAS_CAPTURE = 30000;
24+
25+
constructor(IWormhole initWormhole, ICoreRelayer initCoreRelayer, uint16 chainId, address initAttackerReward) {
26+
attackerReward = initAttackerReward;
27+
wormhole = initWormhole;
28+
core_relayer = initCoreRelayer;
29+
targetChainId = chainId;
30+
}
31+
32+
// This is the function which receives all messages from the remote contracts.
33+
function receiveWormholeMessages(bytes[] memory vaas, bytes[] memory additionalData) public payable override {
34+
// Do nothing. The attacker doesn't care about this message; he sends it himself.
35+
}
36+
37+
receive() external payable {
38+
// Request forward from the relayer network
39+
// The core relayer could in principle accept the request due to this being the target of the message at the same time as being the refund address.
40+
// Note that, if succesful, this forward request would be processed after the time for processing forwards is past.
41+
// Thus, the request would "linger" in the forward request cache and be attended to in the next delivery.
42+
requestForward(targetChainId, toWormholeFormat(attackerReward));
43+
}
44+
45+
function requestForward(uint16 targetChain, bytes32 attackerRewardAddress) internal {
46+
uint256 computeBudget = core_relayer.quoteGasDeliveryFee(
47+
targetChain, SAFE_DELIVERY_GAS_CAPTURE, core_relayer.getDefaultRelayProvider()
48+
);
49+
50+
ICoreRelayer.DeliveryRequest memory request = ICoreRelayer.DeliveryRequest({
51+
targetChain: targetChain,
52+
targetAddress: attackerRewardAddress,
53+
// All remaining funds will be returned to the attacker
54+
refundAddress: attackerRewardAddress,
55+
computeBudget: computeBudget,
56+
applicationBudget: 0,
57+
relayParameters: core_relayer.getDefaultRelayParams()
58+
});
59+
60+
core_relayer.requestForward{value: computeBudget}(request, nonce, core_relayer.getDefaultRelayProvider());
61+
}
62+
63+
function toWormholeFormat(address addr) public pure returns (bytes32 whFormat) {
64+
return bytes32(uint256(uint160(addr)));
65+
}
66+
}

‎ethereum/forge-test/CoreRelayer.t.sol

+127-10
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import {Wormhole} from "../wormhole/ethereum/contracts/Wormhole.sol";
2222
import {IWormhole} from "../contracts/interfaces/IWormhole.sol";
2323
import {WormholeSimulator} from "./WormholeSimulator.sol";
2424
import {IWormholeReceiver} from "../contracts/interfaces/IWormholeReceiver.sol";
25+
import {AttackForwardIntegration} from "../contracts/mock/AttackForwardIntegration.sol";
2526
import {MockRelayerIntegration} from "../contracts/mock/MockRelayerIntegration.sol";
2627
import "../contracts/libraries/external/BytesLib.sol";
2728

@@ -495,6 +496,106 @@ contract TestCoreRelayer is Test {
495496
assertTrue(keccak256(setup.source.integration.getMessage()) == keccak256(bytes("received!")));
496497
}
497498

499+
function testAttackForwardRequestCache(GasParameters memory gasParams, FeeParameters memory feeParams) public {
500+
// General idea:
501+
// 1. Attacker sets up a malicious integration contract in the target chain.
502+
// 2. Attacker requests a message send to `target` chain.
503+
// The message destination and the refund address are both the malicious integration contract in the target chain.
504+
// 3. The delivery of the message triggers a refund to the malicious integration contract.
505+
// 4. During the refund, the integration contract activates the forwarding mechanism.
506+
// This is allowed due to the integration contract also being the target of the delivery.
507+
// 5. The forward request is left as is in the `CoreRelayer` state.
508+
// 6. The next message (i.e. the victim's message) delivery on `target` chain, from any relayer, using any `RelayProvider` and any integration contract,
509+
// will see the forward request placed by the malicious integration contract and act on it.
510+
// Caveat: the delivery of the victim's message must not invoke the forwarding mechanism for the attack test to be meaningful.
511+
//
512+
// In essence, this tries to attack the shared forwarding request cache present in the contract state.
513+
// This attack doesn't work thanks to the check inside the `requestForward` function that only allows requesting a forward when there is a delivery being processed.
514+
515+
StandardSetupTwoChains memory setup = standardAssumeAndSetupTwoChains(gasParams, feeParams, 1000000);
516+
517+
// Collected funds from the attack are meant to be sent here.
518+
address attackerSourceAddress =
519+
address(uint160(uint256(keccak256(abi.encodePacked(bytes("attackerAddress"), setup.sourceChainId)))));
520+
assertTrue(attackerSourceAddress.balance == 0);
521+
522+
// Borrowed assumes from testForward. They should help since this test is similar.
523+
vm.assume(
524+
uint256(1) * gasParams.targetGasPrice * feeParams.targetNativePrice
525+
> uint256(1) * gasParams.sourceGasPrice * feeParams.sourceNativePrice
526+
);
527+
528+
vm.assume(
529+
setup.source.coreRelayer.quoteGasDeliveryFee(
530+
setup.targetChainId, gasParams.targetGasLimit, setup.source.relayProvider
531+
) < uint256(2) ** 222
532+
);
533+
vm.assume(
534+
setup.target.coreRelayer.quoteGasDeliveryFee(setup.sourceChainId, 500000, setup.target.relayProvider)
535+
< uint256(2) ** 222 / feeParams.targetNativePrice
536+
);
537+
538+
// Estimate the cost based on the initialized values
539+
uint256 computeBudget = setup.source.coreRelayer.quoteGasDeliveryFee(
540+
setup.targetChainId, gasParams.targetGasLimit, setup.source.relayProvider
541+
);
542+
543+
{
544+
AttackForwardIntegration attackerContract =
545+
new AttackForwardIntegration(setup.target.wormhole, setup.target.coreRelayer, setup.targetChainId, attackerSourceAddress);
546+
bytes memory attackMsg = "attack";
547+
548+
vm.recordLogs();
549+
550+
// The attacker requests the message to be sent to the malicious contract.
551+
// It is critical that the refund and destination (aka integrator) addresses are the same.
552+
setup.source.integration.sendMessage{value: computeBudget + 2 * setup.source.wormhole.messageFee()}(
553+
attackMsg, setup.targetChainId, address(attackerContract), address(attackerContract)
554+
);
555+
556+
// The relayer triggers the call to the malicious contract.
557+
genericRelayer(setup.sourceChainId, 2);
558+
559+
// The message delivery should fail
560+
assertTrue(keccak256(setup.target.integration.getMessage()) != keccak256(attackMsg));
561+
}
562+
563+
{
564+
// Now one victim sends their message. It doesn't need to be from the same source chain.
565+
// What's necessary is that a message is delivered to the chain targeted by the attacker.
566+
bytes memory victimMsg = "relay my message";
567+
568+
uint256 victimBalancePreDelivery = setup.target.refundAddress.balance;
569+
570+
// We will reutilize the compute budget estimated for the attacker to simplify the code here.
571+
// The victim requests their message to be sent.
572+
setup.source.integration.sendMessage{value: computeBudget + 2 * setup.source.wormhole.messageFee()}(
573+
victimMsg, setup.targetChainId, address(setup.target.integration), address(setup.target.refundAddress)
574+
);
575+
576+
// The relayer delivers the victim's message.
577+
// During the delivery process, the forward request injected by the malicious contract is acknowledged.
578+
// The victim's refund address is not called due to this.
579+
genericRelayer(setup.sourceChainId, 2);
580+
581+
// Ensures the message was received.
582+
assertTrue(keccak256(setup.target.integration.getMessage()) == keccak256(victimMsg));
583+
// Here we assert that the victim's refund is safe.
584+
assertTrue(victimBalancePreDelivery < setup.target.refundAddress.balance);
585+
}
586+
587+
Vm.Log[] memory entries = relayerWormholeSimulator.fetchWormholeMessageFromLog(vm.getRecordedLogs());
588+
if (entries.length > 0) {
589+
// There was a wormhole message produced.
590+
// If the attack is successful this is a forward.
591+
// We'll invoke the relay simulation here and later assert that the attack wasn't successful.
592+
// Relay from target chain to source chain.
593+
genericRelayerProcessLogs(setup.targetChainId, entries);
594+
}
595+
// Assert that the attack wasn't successful.
596+
assertTrue(attackerSourceAddress.balance == 0);
597+
}
598+
498599
function testRedelivery(GasParameters memory gasParams, FeeParameters memory feeParams, bytes memory message)
499600
public
500601
{
@@ -1219,18 +1320,34 @@ contract TestCoreRelayer is Test {
12191320
mapping(bytes32 => ICoreRelayer.TargetDeliveryParametersSingle) pastDeliveries;
12201321

12211322
function genericRelayer(uint16 chainId, uint8 num) internal {
1222-
bytes[] memory encodedVMs = new bytes[](num);
1223-
{
1224-
// Filters all events to just the wormhole messages.
1225-
Vm.Log[] memory entries = relayerWormholeSimulator.fetchWormholeMessageFromLog(vm.getRecordedLogs());
1226-
assertTrue(entries.length >= num);
1227-
for (uint256 i = 0; i < num; i++) {
1228-
encodedVMs[i] = relayerWormholeSimulator.fetchSignedMessageFromLogs(
1229-
entries[i], chainId, address(uint160(uint256(bytes32(entries[i].topics[1]))))
1230-
);
1231-
}
1323+
Vm.Log[] memory entries = truncateRecordedLogs(chainId, num);
1324+
genericRelayerProcessLogs(chainId, entries);
1325+
}
1326+
1327+
/**
1328+
* Discards wormhole events beyond `num` events.
1329+
* Expects at least `num` wormhole events.
1330+
*/
1331+
function truncateRecordedLogs(uint16 chainId, uint8 num) internal returns (Vm.Log[] memory) {
1332+
// Filters all events to just the wormhole messages.
1333+
Vm.Log[] memory entries = relayerWormholeSimulator.fetchWormholeMessageFromLog(vm.getRecordedLogs());
1334+
// We expect at least `num` events.
1335+
assertTrue(entries.length >= num);
1336+
1337+
Vm.Log[] memory firstEntries = new Vm.Log[](num);
1338+
for (uint256 i = 0; i < num; i++) {
1339+
firstEntries[i] = entries[i];
12321340
}
1341+
return firstEntries;
1342+
}
12331343

1344+
function genericRelayerProcessLogs(uint16 chainId, Vm.Log[] memory entries) internal {
1345+
bytes[] memory encodedVMs = new bytes[](entries.length);
1346+
for (uint256 i = 0; i < encodedVMs.length; i++) {
1347+
encodedVMs[i] = relayerWormholeSimulator.fetchSignedMessageFromLogs(
1348+
entries[i], chainId, address(uint160(uint256(bytes32(entries[i].topics[1]))))
1349+
);
1350+
}
12341351
IWormhole.VM[] memory parsed = new IWormhole.VM[](encodedVMs.length);
12351352
for (uint16 i = 0; i < encodedVMs.length; i++) {
12361353
parsed[i] = relayerWormhole.parseVM(encodedVMs[i]);

0 commit comments

Comments
 (0)