Skip to content

H-01 + double allocation attack preventions #21

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mgretzke merged 3 commits into from
Dec 9, 2025
Merged

H-01 + double allocation attack preventions #21

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6425ab8
fixes and test
mgretzke Aug 26, 2025
c02ec31
clean up
mgretzke Aug 26, 2025
3c299f6
Merge branch 'v1-audit1' into H01-missing-recipient-in-transient-stor…
mgretzke Dec 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions src/allocators/OnChainAllocator.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -535,24 +535,24 @@ contract OnChainAllocator is IOnChainAllocator {

function _getAndUpdateNonce(address calling, address sponsor) internal returns (uint256 nonce) {
assembly ("memory-safe") {
calling := xor(calling, mul(sponsor, iszero(calling)))
mstore(0x00, calling)
sponsor := mul(sponsor, iszero(calling))
mstore(0x00, sponsor)
mstore(0x20, nonces.slot)
let nonceSlot := keccak256(0x00, 0x40)
let nonce96 := sload(nonceSlot)
nonce := or(shl(96, calling), add(nonce96, 1))
nonce := or(shl(96, sponsor), add(nonce96, 1))
sstore(nonceSlot, add(nonce96, 1))
}
}

function _getNonce(address calling, address sponsor) internal view returns (uint256 nonce) {
assembly ("memory-safe") {
calling := xor(calling, mul(sponsor, iszero(calling)))
mstore(0x00, calling)
sponsor := mul(sponsor, iszero(calling))
mstore(0x00, sponsor)
mstore(0x20, nonces.slot)
let nonceSlot := keccak256(0x00, 0x40)
let nonce96 := sload(nonceSlot)
nonce := or(shl(96, calling), add(nonce96, 1))
nonce := or(shl(96, sponsor), add(nonce96, 1))
}
}

Expand Down
4 changes: 2 additions & 2 deletions test/ERC7683Allocator.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1264,7 +1264,7 @@ contract ERC7683Allocator_openForDeposit is MockAllocator {

assertEq(ERC6909(address(compactContract)).balanceOf(user, usdcId), defaultAmount);

compact_.nonce = _composeNonceUint(relayer, 1);
compact_.nonce = _composeNonceUint(address(0), 1);
compact_.commitments[0].amount = defaultAmount;

(bytes32 mandateHash,) = _hashMandate(mandate_);
Expand All @@ -1281,7 +1281,7 @@ contract ERC7683Allocator_openForDeposit is MockAllocator {
usdc.mint(address(erc7683Allocator), amount);

BatchCompact memory compact_ = _getCompact();
compact_.nonce = _composeNonceUint(relayer, 1);
compact_.nonce = _composeNonceUint(address(0), 1);

Mandate memory mandate_ = _getMandate();
IOriginSettler.GaslessCrossChainOrder memory order_ = _getGaslessCrossChainOrder(compact_, mandate_, true);
Expand Down
163 changes: 151 additions & 12 deletions test/OnChainAllocator.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -984,9 +984,10 @@ contract OnChainAllocatorTest is Test, TestHelper {
recipient, idsAndAmounts, arbiter, defaultExpiration, BATCH_COMPACT_TYPEHASH, bytes32(0), ''
);

assertEq(returnedNonce, _composeNonceUint(caller, 1));
assertEq(returnedNonce, _composeNonceUint(address(0), 1));
// storage nonce is only incremented in executeAllocation
assertEq(allocator.nonces(caller), 0);
assertEq(allocator.nonces(address(0)), 0);
}

function test_prepareAllocation_revert_InvalidExpiration() public {
Expand Down Expand Up @@ -1036,8 +1037,9 @@ contract OnChainAllocatorTest is Test, TestHelper {
vm.snapshotGasLastCall('onchain_execute_single');

// nonce is scoped to (callerContract, recipient)
assertEq(allocator.nonces(address(allocationCaller)), 1);
uint256 expectedNonce = _composeNonceUint(address(allocationCaller), 1);
assertEq(allocator.nonces(address(allocationCaller)), 0);
assertEq(allocator.nonces(address(0)), 1);
uint256 expectedNonce = _composeNonceUint(address(0), 1);

// compute claim hash and check authorization
Lock[] memory commitments = _idsAndAmountsToCommitments(idsAndAmounts);
Expand All @@ -1051,6 +1053,148 @@ contract OnChainAllocatorTest is Test, TestHelper {
);
}

/// forge-config: default.isolate = false
function test_executeAllocation_revert_multiplePreparations() public {
uint256[2][] memory idsAndAmounts = _idsAndAmountsFor(address(usdc), defaultAmount);

uint256[2][] memory idsAndAmountsCrooked = new uint256[2][](2);
idsAndAmountsCrooked[0][0] = _toId(Scope.Multichain, ResetPeriod.TenMinutes, address(allocator), address(0)); // additionally use another ids to receive a unique identifier slot
idsAndAmountsCrooked[0][1] = defaultAmount;
idsAndAmountsCrooked[1] = idsAndAmounts[0]; // Crooked idsAndAmounts is also using USDC as the same ID, among others

usdc.mint(address(this), defaultAmount);
usdc.approve(address(compact), defaultAmount);

// prepare for the recipient using caller 1
vm.prank(recipient);
uint256 nonce1 = allocator.prepareAllocation(
recipient, idsAndAmounts, arbiter, defaultExpiration, BATCH_COMPACT_TYPEHASH, bytes32(0), ''
);

// prepare for second recipient - DIFFERENT CALLER TO RECEIVE A DIFFERENT NONCE SLOT
vm.prank(address(this));
uint256 nonce2 = allocator.prepareAllocation(
recipient, idsAndAmountsCrooked, arbiter, defaultExpiration, BATCH_COMPACT_TYPEHASH, bytes32(0), ''
);

assertEq(nonce1, _composeNonceUint(address(0), 1));
assertEq(nonce2, _composeNonceUint(address(0), 1));

// We do a single deposit of usdc with defaultAmount. This would mean, that we SHOULD only able to allocate defaultAmount of usdc
ITheCompact(compact).batchDeposit{value: defaultAmount}(idsAndAmountsCrooked, recipient);

// Crooked registrations: we register two claims that would each use all of the usdc (so defaultAmount * 2 combined)
Lock[] memory commitments = _idsAndAmountsToCommitments(idsAndAmounts);
bytes32 claimHash1Crooked =
_createClaimHash(recipient, arbiter, nonce1, defaultExpiration, commitments, bytes32(0));
Lock[] memory commitmentsCrooked = _idsAndAmountsToCommitments(idsAndAmountsCrooked);
bytes32 claimHash2Crooked =
_createClaimHash(recipient, arbiter, nonce2, defaultExpiration, commitmentsCrooked, bytes32(0));
vm.prank(recipient);
ITheCompact(compact).register(claimHash1Crooked, BATCH_COMPACT_TYPEHASH);
vm.prank(recipient);
ITheCompact(compact).register(claimHash2Crooked, BATCH_COMPACT_TYPEHASH);

// execute for first allocation
vm.prank(recipient);
allocator.executeAllocation(
recipient, idsAndAmounts, arbiter, defaultExpiration, BATCH_COMPACT_TYPEHASH, bytes32(0), ''
);

// execute for second allocation which must fail
vm.prank(address(this));
vm.expectRevert(abi.encodeWithSelector(AllocatorLib.InvalidPreparation.selector));
allocator.executeAllocation(
recipient, idsAndAmountsCrooked, arbiter, defaultExpiration, BATCH_COMPACT_TYPEHASH, bytes32(0), ''
);

assertTrue(
allocator.isClaimAuthorized(
claimHash1Crooked, arbiter, recipient, nonce1, defaultExpiration, idsAndAmounts, ''
)
);
assertFalse(
allocator.isClaimAuthorized(
claimHash2Crooked, arbiter, recipient, nonce2, defaultExpiration, idsAndAmounts, ''
)
);
}

/// forge-config: default.isolate = false
function test_executeAllocation_revert_multipleDifferentPreparations(address recipient1, address recipient2)
public
{
vm.assume(recipient1 != address(0));
vm.assume(recipient2 != address(0));

uint256[2][] memory idsAndAmounts = _idsAndAmountsFor(address(usdc), defaultAmount);

uint256 previousBalance = 1_000_000;
usdc.mint(address(this), previousBalance + 2 * defaultAmount);
usdc.approve(address(compact), previousBalance + 2 * defaultAmount);

// deposit funds to recipient1
compact.depositERC20(
address(usdc),
_toLockTag(address(allocator), Scope.Multichain, ResetPeriod.TenMinutes),
previousBalance,
recipient1
);

// Check nonce previous to the allocation
assertEq(allocator.nonces(address(0)), 0);

// prepare for first recipient
vm.prank(recipient1);
uint256 nonce1 = allocator.prepareAllocation(
recipient1, idsAndAmounts, arbiter, defaultExpiration, BATCH_COMPACT_TYPEHASH, bytes32(0), ''
);

// prepare for second recipient
vm.prank(recipient2);
uint256 nonce2 = allocator.prepareAllocation(
recipient2, idsAndAmounts, arbiter, defaultExpiration, BATCH_COMPACT_TYPEHASH, bytes32(0), ''
);

assertEq(nonce1, _composeNonceUint(address(0), 1));
assertEq(nonce2, _composeNonceUint(address(0), 1));
// The provided nonces should be the same because they use the same pool of nonces, independent of the recipient or caller
assertEq(nonce1, nonce2);

// compute claim hash and check authorization
Lock[] memory commitments = _idsAndAmountsToCommitments(idsAndAmounts);
bytes32 claimHash1 = _createClaimHash(recipient1, arbiter, nonce1, defaultExpiration, commitments, bytes32(0));
bytes32 claimHash2 = _createClaimHash(recipient2, arbiter, nonce2, defaultExpiration, commitments, bytes32(0));

ITheCompact(compact).batchDepositAndRegisterFor(
recipient1, idsAndAmounts, arbiter, nonce1, defaultExpiration, BATCH_COMPACT_TYPEHASH, bytes32(0)
);

ITheCompact(compact).batchDepositAndRegisterFor(
recipient2, idsAndAmounts, arbiter, nonce2, defaultExpiration, BATCH_COMPACT_TYPEHASH, bytes32(0)
);

// execute for first recipient
vm.prank(recipient1);
allocator.executeAllocation(
recipient1, idsAndAmounts, arbiter, defaultExpiration, BATCH_COMPACT_TYPEHASH, bytes32(0), ''
);

// execute for second recipient
vm.prank(recipient2);
vm.expectRevert(abi.encodeWithSelector(AllocatorLib.InvalidPreparation.selector));
allocator.executeAllocation(
recipient2, idsAndAmounts, arbiter, defaultExpiration, BATCH_COMPACT_TYPEHASH, bytes32(0), ''
);

assertTrue(
allocator.isClaimAuthorized(claimHash1, arbiter, recipient1, nonce1, defaultExpiration, idsAndAmounts, '')
);
assertFalse(
allocator.isClaimAuthorized(claimHash2, arbiter, recipient2, nonce2, defaultExpiration, idsAndAmounts, '')
);
}

function test_executeAllocation_revert_InvalidPreparation() public {
uint256 amount = defaultAmount;
uint256[2][] memory idsAndAmounts = _idsAndAmountsFor(address(usdc), amount);
Expand Down Expand Up @@ -1082,12 +1226,7 @@ contract OnChainAllocatorTest is Test, TestHelper {
// Compute the claimHash that AllocatorLib will recompute during execute.
Lock[] memory commitments = _idsAndAmountsToCommitments(idsAndAmounts);
bytes32 expectedClaimHash = _createClaimHash(
recipient,
arbiter,
_composeNonceUint(address(allocationCaller), 1),
defaultExpiration,
commitments,
bytes32(0)
recipient, arbiter, _composeNonceUint(address(0), 1), defaultExpiration, commitments, bytes32(0)
);
vm.prank(user);
vm.expectRevert(
Expand Down Expand Up @@ -1143,7 +1282,7 @@ contract OnChainAllocatorTest is Test, TestHelper {
vm.snapshotGasLastCall('onchain_execute_double');

// authorization with the measured amounts
uint256 expectedNonce = _composeNonceUint(address(allocationCaller), 1);
uint256 expectedNonce = _composeNonceUint(address(0), 1);

assertTrue(
allocator.isClaimAuthorized(
Expand Down Expand Up @@ -1316,7 +1455,7 @@ contract OnChainAllocatorTest is Test, TestHelper {
idsAndAmounts[0][0] = _toId(Scope.Multichain, ResetPeriod.TenMinutes, address(allocator), address(usdc));
idsAndAmounts[0][1] = defaultAmount;

assertEq(nonce, _composeNonceUint(caller, 1));
assertEq(nonce, _composeNonceUint(address(0), 1));
assertEq(registeredAmounts.length, 1);
assertEq(registeredAmounts[0], defaultAmount);
assertEq(ERC6909(address(compact)).balanceOf(recipient, idsAndAmounts[0][0]), defaultAmount);
Expand Down Expand Up @@ -1345,7 +1484,7 @@ contract OnChainAllocatorTest is Test, TestHelper {
idsAndAmounts[0][0] = _toId(Scope.Multichain, ResetPeriod.TenMinutes, address(allocator), address(usdc));
idsAndAmounts[0][1] = defaultAmount;

assertEq(nonce, _composeNonceUint(caller, 1));
assertEq(nonce, _composeNonceUint(address(0), 1));
assertEq(registeredAmounts.length, 1);
assertEq(registeredAmounts[0], defaultAmount);
assertEq(ERC6909(address(compact)).balanceOf(recipient, idsAndAmounts[0][0]), defaultAmount);
Expand Down