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.
This commit is contained in:
parent
ca71dcbfee
commit
0f7c0a0c69
|
@ -0,0 +1,66 @@
|
|||
// SPDX-License-Identifier: UNLICENSED
|
||||
pragma solidity ^0.8.17;
|
||||
|
||||
import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
|
||||
|
||||
import "../interfaces/IWormhole.sol";
|
||||
import "../interfaces/IWormholeReceiver.sol";
|
||||
import "../interfaces/ICoreRelayer.sol";
|
||||
|
||||
/**
|
||||
* This contract is a malicious "integration" that attempts to attack the forward mechanism.
|
||||
*/
|
||||
contract AttackForwardIntegration is IWormholeReceiver {
|
||||
mapping(bytes32 => bool) consumedMessages;
|
||||
address attackerReward;
|
||||
IWormhole wormhole;
|
||||
ICoreRelayer core_relayer;
|
||||
uint32 nonce = 1;
|
||||
uint16 targetChainId;
|
||||
|
||||
// Capture 30k gas for fees
|
||||
// This just needs to be enough to pay for the call to the destination address.
|
||||
uint32 SAFE_DELIVERY_GAS_CAPTURE = 30000;
|
||||
|
||||
constructor(IWormhole initWormhole, ICoreRelayer initCoreRelayer, uint16 chainId, address initAttackerReward) {
|
||||
attackerReward = initAttackerReward;
|
||||
wormhole = initWormhole;
|
||||
core_relayer = initCoreRelayer;
|
||||
targetChainId = chainId;
|
||||
}
|
||||
|
||||
// This is the function which receives all messages from the remote contracts.
|
||||
function receiveWormholeMessages(bytes[] memory vaas, bytes[] memory additionalData) public payable override {
|
||||
// Do nothing. The attacker doesn't care about this message; he sends it himself.
|
||||
}
|
||||
|
||||
receive() external payable {
|
||||
// Request forward from the relayer network
|
||||
// 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.
|
||||
// Note that, if succesful, this forward request would be processed after the time for processing forwards is past.
|
||||
// Thus, the request would "linger" in the forward request cache and be attended to in the next delivery.
|
||||
requestForward(targetChainId, toWormholeFormat(attackerReward));
|
||||
}
|
||||
|
||||
function requestForward(uint16 targetChain, bytes32 attackerRewardAddress) internal {
|
||||
uint256 computeBudget = core_relayer.quoteGasDeliveryFee(
|
||||
targetChain, SAFE_DELIVERY_GAS_CAPTURE, core_relayer.getDefaultRelayProvider()
|
||||
);
|
||||
|
||||
ICoreRelayer.DeliveryRequest memory request = ICoreRelayer.DeliveryRequest({
|
||||
targetChain: targetChain,
|
||||
targetAddress: attackerRewardAddress,
|
||||
// All remaining funds will be returned to the attacker
|
||||
refundAddress: attackerRewardAddress,
|
||||
computeBudget: computeBudget,
|
||||
applicationBudget: 0,
|
||||
relayParameters: core_relayer.getDefaultRelayParams()
|
||||
});
|
||||
|
||||
core_relayer.requestForward{value: computeBudget}(request, nonce, core_relayer.getDefaultRelayProvider());
|
||||
}
|
||||
|
||||
function toWormholeFormat(address addr) public pure returns (bytes32 whFormat) {
|
||||
return bytes32(uint256(uint160(addr)));
|
||||
}
|
||||
}
|
|
@ -23,6 +23,7 @@ import {Wormhole} from "../wormhole/ethereum/contracts/Wormhole.sol";
|
|||
import {IWormhole} from "../contracts/interfaces/IWormhole.sol";
|
||||
import {WormholeSimulator} from "./WormholeSimulator.sol";
|
||||
import {IWormholeReceiver} from "../contracts/interfaces/IWormholeReceiver.sol";
|
||||
import {AttackForwardIntegration} from "../contracts/mock/AttackForwardIntegration.sol";
|
||||
import {MockRelayerIntegration} from "../contracts/mock/MockRelayerIntegration.sol";
|
||||
import "../contracts/libraries/external/BytesLib.sol";
|
||||
|
||||
|
@ -497,6 +498,106 @@ contract TestCoreRelayer is Test {
|
|||
assertTrue(keccak256(setup.source.integration.getFirstMessage()) == keccak256(bytes("received!")));
|
||||
}
|
||||
|
||||
function testAttackForwardRequestCache(GasParameters memory gasParams, FeeParameters memory feeParams) public {
|
||||
// General idea:
|
||||
// 1. Attacker sets up a malicious integration contract in the target chain.
|
||||
// 2. Attacker requests a message send to `target` chain.
|
||||
// The message destination and the refund address are both the malicious integration contract in the target chain.
|
||||
// 3. The delivery of the message triggers a refund to the malicious integration contract.
|
||||
// 4. During the refund, the integration contract activates the forwarding mechanism.
|
||||
// This is allowed due to the integration contract also being the target of the delivery.
|
||||
// 5. The forward request is left as is in the `CoreRelayer` state.
|
||||
// 6. The next message (i.e. the victim's message) delivery on `target` chain, from any relayer, using any `RelayProvider` and any integration contract,
|
||||
// will see the forward request placed by the malicious integration contract and act on it.
|
||||
// Caveat: the delivery of the victim's message must not invoke the forwarding mechanism for the attack test to be meaningful.
|
||||
//
|
||||
// In essence, this tries to attack the shared forwarding request cache present in the contract state.
|
||||
// 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.
|
||||
|
||||
StandardSetupTwoChains memory setup = standardAssumeAndSetupTwoChains(gasParams, feeParams, 1000000);
|
||||
|
||||
// Collected funds from the attack are meant to be sent here.
|
||||
address attackerSourceAddress =
|
||||
address(uint160(uint256(keccak256(abi.encodePacked(bytes("attackerAddress"), setup.sourceChainId)))));
|
||||
assertTrue(attackerSourceAddress.balance == 0);
|
||||
|
||||
// Borrowed assumes from testForward. They should help since this test is similar.
|
||||
vm.assume(
|
||||
uint256(1) * gasParams.targetGasPrice * feeParams.targetNativePrice
|
||||
> uint256(1) * gasParams.sourceGasPrice * feeParams.sourceNativePrice
|
||||
);
|
||||
|
||||
vm.assume(
|
||||
setup.source.coreRelayer.quoteGasDeliveryFee(
|
||||
setup.targetChainId, gasParams.targetGasLimit, setup.source.relayProvider
|
||||
) < uint256(2) ** 222
|
||||
);
|
||||
vm.assume(
|
||||
setup.target.coreRelayer.quoteGasDeliveryFee(setup.sourceChainId, 500000, setup.target.relayProvider)
|
||||
< uint256(2) ** 222 / feeParams.targetNativePrice
|
||||
);
|
||||
|
||||
// Estimate the cost based on the initialized values
|
||||
uint256 computeBudget = setup.source.coreRelayer.quoteGasDeliveryFee(
|
||||
setup.targetChainId, gasParams.targetGasLimit, setup.source.relayProvider
|
||||
);
|
||||
|
||||
{
|
||||
AttackForwardIntegration attackerContract =
|
||||
new AttackForwardIntegration(setup.target.wormhole, setup.target.coreRelayer, setup.targetChainId, attackerSourceAddress);
|
||||
bytes memory attackMsg = "attack";
|
||||
|
||||
vm.recordLogs();
|
||||
|
||||
// The attacker requests the message to be sent to the malicious contract.
|
||||
// It is critical that the refund and destination (aka integrator) addresses are the same.
|
||||
setup.source.integration.sendMessage{value: computeBudget + 2 * setup.source.wormhole.messageFee()}(
|
||||
attackMsg, setup.targetChainId, address(attackerContract), address(attackerContract)
|
||||
);
|
||||
|
||||
// The relayer triggers the call to the malicious contract.
|
||||
genericRelayer(setup.sourceChainId, 2);
|
||||
|
||||
// The message delivery should fail
|
||||
assertTrue(keccak256(setup.target.integration.getMessage()) != keccak256(attackMsg));
|
||||
}
|
||||
|
||||
{
|
||||
// Now one victim sends their message. It doesn't need to be from the same source chain.
|
||||
// What's necessary is that a message is delivered to the chain targeted by the attacker.
|
||||
bytes memory victimMsg = "relay my message";
|
||||
|
||||
uint256 victimBalancePreDelivery = setup.target.refundAddress.balance;
|
||||
|
||||
// We will reutilize the compute budget estimated for the attacker to simplify the code here.
|
||||
// The victim requests their message to be sent.
|
||||
setup.source.integration.sendMessage{value: computeBudget + 2 * setup.source.wormhole.messageFee()}(
|
||||
victimMsg, setup.targetChainId, address(setup.target.integration), address(setup.target.refundAddress)
|
||||
);
|
||||
|
||||
// The relayer delivers the victim's message.
|
||||
// During the delivery process, the forward request injected by the malicious contract is acknowledged.
|
||||
// The victim's refund address is not called due to this.
|
||||
genericRelayer(setup.sourceChainId, 2);
|
||||
|
||||
// Ensures the message was received.
|
||||
assertTrue(keccak256(setup.target.integration.getMessage()) == keccak256(victimMsg));
|
||||
// Here we assert that the victim's refund is safe.
|
||||
assertTrue(victimBalancePreDelivery < setup.target.refundAddress.balance);
|
||||
}
|
||||
|
||||
Vm.Log[] memory entries = relayerWormholeSimulator.fetchWormholeMessageFromLog(vm.getRecordedLogs());
|
||||
if (entries.length > 0) {
|
||||
// There was a wormhole message produced.
|
||||
// If the attack is successful this is a forward.
|
||||
// We'll invoke the relay simulation here and later assert that the attack wasn't successful.
|
||||
// Relay from target chain to source chain.
|
||||
genericRelayerProcessLogs(setup.targetChainId, entries);
|
||||
}
|
||||
// Assert that the attack wasn't successful.
|
||||
assertTrue(attackerSourceAddress.balance == 0);
|
||||
}
|
||||
|
||||
function testRedelivery(GasParameters memory gasParams, FeeParameters memory feeParams, bytes memory message)
|
||||
public
|
||||
{
|
||||
|
@ -1238,18 +1339,34 @@ contract TestCoreRelayer is Test {
|
|||
mapping(bytes32 => CoreRelayer.TargetDeliveryParametersSingle) pastDeliveries;
|
||||
|
||||
function genericRelayer(uint16 chainId, uint8 num) internal {
|
||||
bytes[] memory encodedVMs = new bytes[](num);
|
||||
{
|
||||
// Filters all events to just the wormhole messages.
|
||||
Vm.Log[] memory entries = relayerWormholeSimulator.fetchWormholeMessageFromLog(vm.getRecordedLogs());
|
||||
assertTrue(entries.length >= num);
|
||||
for (uint256 i = 0; i < num; i++) {
|
||||
encodedVMs[i] = relayerWormholeSimulator.fetchSignedMessageFromLogs(
|
||||
entries[i], chainId, address(uint160(uint256(bytes32(entries[i].topics[1]))))
|
||||
);
|
||||
}
|
||||
}
|
||||
Vm.Log[] memory entries = truncateRecordedLogs(chainId, num);
|
||||
genericRelayerProcessLogs(chainId, entries);
|
||||
}
|
||||
|
||||
/**
|
||||
* Discards wormhole events beyond `num` events.
|
||||
* Expects at least `num` wormhole events.
|
||||
*/
|
||||
function truncateRecordedLogs(uint16 chainId, uint8 num) internal returns (Vm.Log[] memory) {
|
||||
// Filters all events to just the wormhole messages.
|
||||
Vm.Log[] memory entries = relayerWormholeSimulator.fetchWormholeMessageFromLog(vm.getRecordedLogs());
|
||||
// We expect at least `num` events.
|
||||
assertTrue(entries.length >= num);
|
||||
|
||||
Vm.Log[] memory firstEntries = new Vm.Log[](num);
|
||||
for (uint256 i = 0; i < num; i++) {
|
||||
firstEntries[i] = entries[i];
|
||||
}
|
||||
return firstEntries;
|
||||
}
|
||||
|
||||
function genericRelayerProcessLogs(uint16 chainId, Vm.Log[] memory entries) internal {
|
||||
bytes[] memory encodedVMs = new bytes[](entries.length);
|
||||
for (uint256 i = 0; i < encodedVMs.length; i++) {
|
||||
encodedVMs[i] = relayerWormholeSimulator.fetchSignedMessageFromLogs(
|
||||
entries[i], chainId, address(uint160(uint256(bytes32(entries[i].topics[1]))))
|
||||
);
|
||||
}
|
||||
IWormhole.VM[] memory parsed = new IWormhole.VM[](encodedVMs.length);
|
||||
for (uint16 i = 0; i < encodedVMs.length; i++) {
|
||||
parsed[i] = relayerWormhole.parseVM(encodedVMs[i]);
|
||||
|
|
Loading…
Reference in New Issue