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

Commit 4ae418d

Browse files
scnalescnale
committed
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 cef42bd commit 4ae418d

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
@@ -23,6 +23,7 @@ import {Wormhole} from "../wormhole/ethereum/contracts/Wormhole.sol";
2323
import {IWormhole} from "../contracts/interfaces/IWormhole.sol";
2424
import {WormholeSimulator} from "./WormholeSimulator.sol";
2525
import {IWormholeReceiver} from "../contracts/interfaces/IWormholeReceiver.sol";
26+
import {AttackForwardIntegration} from "../contracts/mock/AttackForwardIntegration.sol";
2627
import {MockRelayerIntegration} from "../contracts/mock/MockRelayerIntegration.sol";
2728
import "../contracts/libraries/external/BytesLib.sol";
2829

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

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

12291330
function genericRelayer(uint16 chainId, uint8 num) internal {
1230-
bytes[] memory encodedVMs = new bytes[](num);
1231-
{
1232-
// Filters all events to just the wormhole messages.
1233-
Vm.Log[] memory entries = relayerWormholeSimulator.fetchWormholeMessageFromLog(vm.getRecordedLogs());
1234-
assertTrue(entries.length >= num);
1235-
for (uint256 i = 0; i < num; i++) {
1236-
encodedVMs[i] = relayerWormholeSimulator.fetchSignedMessageFromLogs(
1237-
entries[i], chainId, address(uint160(uint256(bytes32(entries[i].topics[1]))))
1238-
);
1239-
}
1331+
Vm.Log[] memory entries = truncateRecordedLogs(chainId, num);
1332+
genericRelayerProcessLogs(chainId, entries);
1333+
}
1334+
1335+
/**
1336+
* Discards wormhole events beyond `num` events.
1337+
* Expects at least `num` wormhole events.
1338+
*/
1339+
function truncateRecordedLogs(uint16 chainId, uint8 num) internal returns (Vm.Log[] memory) {
1340+
// Filters all events to just the wormhole messages.
1341+
Vm.Log[] memory entries = relayerWormholeSimulator.fetchWormholeMessageFromLog(vm.getRecordedLogs());
1342+
// We expect at least `num` events.
1343+
assertTrue(entries.length >= num);
1344+
1345+
Vm.Log[] memory firstEntries = new Vm.Log[](num);
1346+
for (uint256 i = 0; i < num; i++) {
1347+
firstEntries[i] = entries[i];
12401348
}
1349+
return firstEntries;
1350+
}
12411351

1352+
function genericRelayerProcessLogs(uint16 chainId, Vm.Log[] memory entries) internal {
1353+
bytes[] memory encodedVMs = new bytes[](entries.length);
1354+
for (uint256 i = 0; i < encodedVMs.length; i++) {
1355+
encodedVMs[i] = relayerWormholeSimulator.fetchSignedMessageFromLogs(
1356+
entries[i], chainId, address(uint160(uint256(bytes32(entries[i].topics[1]))))
1357+
);
1358+
}
12421359
IWormhole.VM[] memory parsed = new IWormhole.VM[](encodedVMs.length);
12431360
for (uint16 i = 0; i < encodedVMs.length; i++) {
12441361
parsed[i] = relayerWormhole.parseVM(encodedVMs[i]);

0 commit comments

Comments
 (0)