Skip to content

Commit

Permalink
Merge pull request #102 from 1inch/audits/Add-test-for-reentrancy
Browse files Browse the repository at this point in the history
Add a test for reentrancy failure by the resolver
  • Loading branch information
byshape authored Aug 6, 2024
2 parents 99d6d00 + 6c6af19 commit ef6956a
Show file tree
Hide file tree
Showing 3 changed files with 170 additions and 16 deletions.
33 changes: 17 additions & 16 deletions .gas-snapshot
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
EscrowFactoryTest:test_MultipleFillsInvalidKey() (gas: 474076)
EscrowFactoryTest:test_MultipleFillsInvalidSecretsAmount() (gas: 473718)
EscrowFactoryTest:test_NoDeploymentForNotResolver() (gas: 190767)
EscrowFactoryTest:test_NoDeploymentForNotResolver() (gas: 190780)
EscrowFactoryTest:test_NoInsufficientBalanceDeploymentForMaker() (gas: 128460)
EscrowFactoryTest:test_NoInsufficientBalanceDeploymentForTaker() (gas: 27118)
EscrowFactoryTest:test_NoInsufficientBalanceNativeDeploymentForMaker() (gas: 121696)
Expand All @@ -10,16 +10,16 @@ EscrowTest:test_CancelDst() (gas: 116126)
EscrowTest:test_CancelDstDifferentTarget() (gas: 143384)
EscrowTest:test_CancelDstWithNativeToken() (gas: 93710)
EscrowTest:test_CancelPublicSrc() (gas: 165464)
EscrowTest:test_CancelResolverSrc() (gas: 168624)
EscrowTest:test_CancelResolverSrc() (gas: 168607)
EscrowTest:test_CancelResolverSrcReceiver() (gas: 179361)
EscrowTest:test_NoAnyoneCancelDuringResolverCancelSrc() (gas: 163856)
EscrowTest:test_NoCallsWithInvalidImmutables() (gas: 286518)
EscrowTest:test_NoCancelByAnyoneDst() (gas: 121726)
EscrowTest:test_NoCancelDuringWithdrawalDst() (gas: 121486)
EscrowTest:test_NoCancelDuringWithdrawalSrc() (gas: 163959)
EscrowTest:test_NoCancelDuringWithdrawalSrc() (gas: 164001)
EscrowTest:test_NoFailedNativeTokenTransferCancelSrc() (gas: 179339)
EscrowTest:test_NoFailedNativeTokenTransferWithdrawalDst() (gas: 154544)
EscrowTest:test_NoFailedNativeTokenTransferWithdrawalSrc() (gas: 312369)
EscrowTest:test_NoFailedNativeTokenTransferWithdrawalSrc() (gas: 312329)
EscrowTest:test_NoPublicCancelDuringPrivateCancellationSrc() (gas: 163166)
EscrowTest:test_NoPublicWithdrawOutsideOfAllowedPeriodDst() (gas: 126910)
EscrowTest:test_NoPublicWithdrawalOutsideOfAllowedPeriodSrc() (gas: 169085)
Expand All @@ -31,10 +31,10 @@ EscrowTest:test_NoRevertFailedNativeTokenTransferWithdrawalDstNative() (gas: 911
EscrowTest:test_NoWithdrawalByAnyoneSrc() (gas: 160873)
EscrowTest:test_NoWithdrawalByNonResolverDst() (gas: 121439)
EscrowTest:test_NoWithdrawalOutsideOfAllowedPeriodDst() (gas: 126326)
EscrowTest:test_NoWithdrawalOutsideOfAllowedPeriodSrc() (gas: 169627)
EscrowTest:test_NoWithdrawalOutsideOfAllowedPeriodSrc() (gas: 169636)
EscrowTest:test_NoWithdrawalWithWrongSecretDst() (gas: 122776)
EscrowTest:test_NoWithdrawalWithWrongSecretSrc() (gas: 164478)
EscrowTest:test_PublicWithdrawSrc() (gas: 181761)
EscrowTest:test_PublicWithdrawSrc() (gas: 181745)
EscrowTest:test_RescueFundsDst() (gas: 158304)
EscrowTest:test_RescueFundsDstNative() (gas: 186735)
EscrowTest:test_RescueFundsSrc() (gas: 195445)
Expand All @@ -45,13 +45,14 @@ EscrowTest:test_WithdrawByResolverDstNative() (gas: 97831)
EscrowTest:test_WithdrawByResolverPublicDst() (gas: 141757)
EscrowTest:test_WithdrawSrc() (gas: 186505)
EscrowTest:test_WithdrawSrcTo() (gas: 191335)
IntegrationEscrowFactoryTest:test_DeployCloneForMakerNonWhitelistedResolverInt() (gas: 473480)
IntegrationEscrowFactoryTest:test_NoInsufficientBalanceDeploymentForMakerInt() (gas: 341110)
IntegrationEscrowFactoryTest:test_DeployCloneForMakerNonWhitelistedResolverInt() (gas: 473542)
IntegrationEscrowFactoryTest:test_NoInsufficientBalanceDeploymentForMakerInt() (gas: 341264)
IntegrationEscrowFactoryTest:test_NoResolverReentrancy() (gas: 2065187)
IntegrationResolverMockTest:test_MockCancelDst() (gas: 157184)
IntegrationResolverMockTest:test_MockCancelSrc() (gas: 353972)
IntegrationResolverMockTest:test_MockCancelSrc() (gas: 353957)
IntegrationResolverMockTest:test_MockDeployDst() (gas: 151470)
IntegrationResolverMockTest:test_MockDeploySrc() (gas: 364921)
IntegrationResolverMockTest:test_MockPublicCancelSrc() (gas: 392502)
IntegrationResolverMockTest:test_MockPublicCancelSrc() (gas: 392459)
IntegrationResolverMockTest:test_MockPublicWithdrawDst() (gas: 164841)
IntegrationResolverMockTest:test_MockRescueFundsDst() (gas: 161103)
IntegrationResolverMockTest:test_MockRescueFundsSrc() (gas: 382583)
Expand All @@ -60,16 +61,16 @@ IntegrationResolverMockTest:test_MockWithdrawToSrc() (gas: 354839)
MerkleStorageInvalidatorIntTest:test_MultipleFillsFillAllExtra() (gas: 923576)
MerkleStorageInvalidatorIntTest:test_MultipleFillsFillAllFromLast() (gas: 922329)
MerkleStorageInvalidatorIntTest:test_MultipleFillsFillAllTwoFills() (gas: 922136)
MerkleStorageInvalidatorIntTest:test_MultipleFillsFillFirst() (gas: 707614)
MerkleStorageInvalidatorIntTest:test_MultipleFillsFillFirst() (gas: 707638)
MerkleStorageInvalidatorIntTest:test_MultipleFillsFillFirstTwoFills() (gas: 932969)
MerkleStorageInvalidatorIntTest:test_MultipleFillsFillLast() (gas: 707218)
MerkleStorageInvalidatorIntTest:test_MultipleFillsFillLast() (gas: 707297)
MerkleStorageInvalidatorIntTest:test_MultipleFillsNoDeploymentWithoutValidation() (gas: 301343)
MerkleStorageInvalidatorIntTest:test_MultipleFillsNoReuseOfSecrets() (gas: 1068017)
MerkleStorageInvalidatorIntTest:test_MultipleFillsNoReuseOfSecrets() (gas: 1069228)
MerkleStorageInvalidatorIntTest:test_MultipleFillsNoSecondDeploymentWithTheSameIndex() (gas: 786011)
MerkleStorageInvalidatorIntTest:test_MultipleFillsOddDivision() (gas: 444783)
MerkleStorageInvalidatorIntTest:test_MultipleFillsOneFill() (gas: 707710)
MerkleStorageInvalidatorIntTest:test_MultipleFillsTwoFills() (gas: 921433)
MerkleStorageInvalidatorTest:test_ShortenedProofIsNotValid() (gas: 230552)
MerkleStorageInvalidatorIntTest:test_MultipleFillsOneFill() (gas: 707674)
MerkleStorageInvalidatorIntTest:test_MultipleFillsTwoFills() (gas: 921440)
MerkleStorageInvalidatorTest:test_ShortenedProofIsNotValid() (gas: 230605)
TimelocksLibTest:test_NoTimelocksOverflow() (gas: 156200)
TimelocksLibTest:test_getStartTimestamps() (gas: 16207)
TimelocksLibTest:test_setDeployedAt() (gas: 5741)
62 changes: 62 additions & 0 deletions test/integration/EscrowFactory.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@
pragma solidity 0.8.23;

import { TakerTraits } from "limit-order-protocol/contracts/libraries/TakerTraitsLib.sol";
import { Merkle } from "murky/src/Merkle.sol";
import { Address } from "solidity-utils/contracts/libraries/AddressLib.sol";

import { IEscrowFactory } from "contracts/interfaces/IEscrowFactory.sol";

import { BaseSetup } from "../utils/BaseSetup.sol";
import { CrossChainTestLib } from "../utils/libraries/CrossChainTestLib.sol";
import { ResolverReentrancy } from "../utils/mocks/ResolverReentrancy.sol";

contract IntegrationEscrowFactoryTest is BaseSetup {
function setUp() public virtual override {
Expand Down Expand Up @@ -148,5 +150,65 @@ contract IntegrationEscrowFactoryTest is BaseSetup {
);
}

function test_NoResolverReentrancy() public {
ResolverReentrancy badResolver = new ResolverReentrancy(escrowFactory, limitOrderProtocol, address(this));
resolvers[0] = address(badResolver);
vm.deal(address(badResolver), 100 ether);

uint256 partsAmount = 100;
uint256 secretsAmount = partsAmount + 1;
bytes32[] memory hashedSecrets = new bytes32[](secretsAmount);
bytes32[] memory hashedPairs = new bytes32[](secretsAmount);
for (uint64 i = 0; i < secretsAmount; i++) {
// Note: This is not production-ready code. Use cryptographically secure random to generate secrets.
hashedSecrets[i] = keccak256(abi.encodePacked(i));
hashedPairs[i] = keccak256(abi.encodePacked(i, hashedSecrets[i]));
}
Merkle merkle = new Merkle();
bytes32 root = merkle.getRoot(hashedPairs);
bytes32 rootPlusAmount = bytes32(partsAmount << 240 | uint240(uint256(root)));
uint256 idx = 0;
uint256 makingAmount = MAKING_AMOUNT / partsAmount;
bytes32[] memory proof = merkle.getProof(hashedPairs, idx);
assert(merkle.verifyProof(root, proof, hashedPairs[idx]));

vm.warp(1710288000); // set current timestamp
(timelocks, timelocksDst) = CrossChainTestLib.setTimelocks(srcTimelocks, dstTimelocks);


CrossChainTestLib.SwapData memory swapData = _prepareDataSrcHashlock(rootPlusAmount, false, true);

swapData.immutables.hashlock = hashedSecrets[idx];
swapData.immutables.amount = makingAmount - 2;

(uint8 v, bytes32 r, bytes32 s) = vm.sign(alice.privateKey, swapData.orderHash);
bytes32 vs = bytes32((uint256(v - 27) << 255)) | s;

bytes memory interaction = abi.encodePacked(address(badResolver));
bytes memory interactionFull = abi.encodePacked(interaction, escrowFactory, abi.encode(proof, idx, hashedSecrets[idx]));

(TakerTraits takerTraits, bytes memory args) = CrossChainTestLib.buildTakerTraits(
true, // makingAmount
false, // unwrapWeth
false, // skipMakerPermit
false, // usePermit2
address(0), // target
swapData.extension, // extension
interactionFull,
0 // threshold
);

vm.expectRevert(IEscrowFactory.InvalidPartialFill.selector);
badResolver.deploySrc(
swapData.immutables,
swapData.order,
r,
vs,
makingAmount - 2,
takerTraits,
args
);
}

/* solhint-enable func-name-mixedcase */
}
91 changes: 91 additions & 0 deletions test/utils/mocks/ResolverReentrancy.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
// SPDX-License-Identifier: MIT

pragma solidity 0.8.23;

import { Ownable } from "openzeppelin-contracts/contracts/access/Ownable.sol";

import { IOrderMixin } from "limit-order-protocol/contracts/interfaces/IOrderMixin.sol";
import { TakerTraits } from "limit-order-protocol/contracts/libraries/TakerTraitsLib.sol";

import { IBaseEscrow } from "../../../contracts/interfaces/IBaseEscrow.sol";
import { IEscrowFactory } from "../../../contracts/interfaces/IEscrowFactory.sol";
import { TimelocksLib } from "../../../contracts/libraries/TimelocksLib.sol";

contract ResolverReentrancy is Ownable {
uint256 private constant _ARGS_INTERACTION_LENGTH_OFFSET = 200;
IEscrowFactory private immutable _FACTORY;
IOrderMixin private immutable _LOP;
bytes32 private _r;
bytes32 private _vs;
TakerTraits private _takerTraits;
IBaseEscrow.Immutables private _immutables;

error AccessDenied();

/// @notice Only limit order protocol can call this contract.
modifier onlyLOP() {
if (msg.sender != address(_LOP)) {
revert AccessDenied();
}
_;
}

constructor(IEscrowFactory factory, IOrderMixin lop, address initialOwner) Ownable(initialOwner) {
_FACTORY = factory;
_LOP = lop;
}

receive() external payable {} // solhint-disable-line no-empty-blocks

/**
* @notice See {IResolverExample-deploySrc}.
*/
function deploySrc(
IBaseEscrow.Immutables calldata immutables,
IOrderMixin.Order calldata order,
bytes32 r,
bytes32 vs,
uint256 amount,
TakerTraits takerTraits,
bytes calldata args
) external onlyOwner {
IBaseEscrow.Immutables memory immutablesMem = immutables;
immutablesMem.timelocks = TimelocksLib.setDeployedAt(immutables.timelocks, block.timestamp);
address computed = _FACTORY.addressOfEscrowSrc(immutablesMem);
(bool success,) = address(computed).call{ value: immutablesMem.safetyDeposit }("");
if (!success) revert IBaseEscrow.NativeTokenSendingFailure();

// _ARGS_HAS_TARGET = 1 << 251
takerTraits = TakerTraits.wrap(TakerTraits.unwrap(takerTraits) | uint256(1 << 251));
bytes memory argsMem = abi.encodePacked(computed, args);
_immutables = immutables;
_r = r;
_vs = vs;
_takerTraits = takerTraits;
_LOP.fillOrderArgs(order, r, vs, amount, takerTraits, argsMem);
}

function takerInteraction(
IOrderMixin.Order calldata order,
bytes calldata extension,
bytes32 /* orderHash */,
address /* taker */,
uint256 /* makingAmount */,
uint256 /* takingAmount */,
uint256 /* remainingMakingAmount */,
bytes calldata extraData
) external onlyLOP {
_immutables.amount = 1;
address computed = _FACTORY.addressOfEscrowSrc(_immutables);
(bool success,) = address(computed).call{ value: _immutables.safetyDeposit }("");
if (!success) revert IBaseEscrow.NativeTokenSendingFailure();

_takerTraits = TakerTraits.wrap(
TakerTraits.unwrap(_takerTraits) &
~(uint256(type(uint24).max) << _ARGS_INTERACTION_LENGTH_OFFSET) |
(extraData.length << _ARGS_INTERACTION_LENGTH_OFFSET)
);
bytes memory argsMem = abi.encodePacked(computed, extension, extraData);
_LOP.fillOrderArgs(order, _r, _vs, 1, _takerTraits, argsMem);
}
}

0 comments on commit ef6956a

Please sign in to comment.