Skip to content

Commit 3d2de47

Browse files
C4 S-426 Fee parameter validation (#77)
* Fee parameter validation * Update snapshot
1 parent 8c86e47 commit 3d2de47

File tree

3 files changed

+302
-52
lines changed

3 files changed

+302
-52
lines changed

.gas-snapshot

Lines changed: 46 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -6,48 +6,52 @@ DelegatecallGuardTest:test_direct_call_reverts_NotDelegateCall() (gas: 8420)
66
DelegatecallGuardTest:test_multiple_delegatecall_guards() (gas: 298825)
77
DelegatecallGuardTest:test_onlyDelegatecall_modifier_usage() (gas: 98867)
88
DelegatecallGuardTest:test_self_address_immutable() (gas: 2877)
9-
TrailsIntentEntrypointDeploymentTest:test_DeployIntentEntrypoint_SameAddress() (gas: 833102)
10-
TrailsIntentEntrypointDeploymentTest:test_DeployIntentEntrypoint_Success() (gas: 825615)
11-
TrailsIntentEntrypointDeploymentTest:test_DeployedIntentEntrypoint_HasCorrectConfiguration() (gas: 828414)
12-
TrailsIntentEntrypointTest:testAssemblyCodeExecution() (gas: 89250)
13-
TrailsIntentEntrypointTest:testConstructor() (gas: 3309)
14-
TrailsIntentEntrypointTest:testConstructorAndDomainSeparator() (gas: 7706)
15-
TrailsIntentEntrypointTest:testDepositToIntentAlreadyUsed() (gas: 114117)
16-
TrailsIntentEntrypointTest:testDepositToIntentCannotReuseDigest() (gas: 94779)
17-
TrailsIntentEntrypointTest:testDepositToIntentExpiredDeadline() (gas: 56051)
18-
TrailsIntentEntrypointTest:testDepositToIntentReentrancyProtection() (gas: 114095)
19-
TrailsIntentEntrypointTest:testDepositToIntentRequiresNonZeroAmount() (gas: 27393)
20-
TrailsIntentEntrypointTest:testDepositToIntentRequiresValidToken() (gas: 30776)
21-
TrailsIntentEntrypointTest:testDepositToIntentTransferFromFails() (gas: 58409)
22-
TrailsIntentEntrypointTest:testDepositToIntentWithPermitAlreadyUsed() (gas: 132684)
23-
TrailsIntentEntrypointTest:testDepositToIntentWithPermitExpiredDeadline() (gas: 36472)
24-
TrailsIntentEntrypointTest:testDepositToIntentWithPermitReentrancyProtection() (gas: 124106)
25-
TrailsIntentEntrypointTest:testDepositToIntentWithPermitRequiresNonZeroAmount() (gas: 35186)
26-
TrailsIntentEntrypointTest:testDepositToIntentWithPermitRequiresPermitAmount() (gas: 60755)
27-
TrailsIntentEntrypointTest:testDepositToIntentWithPermitRequiresValidToken() (gas: 35912)
28-
TrailsIntentEntrypointTest:testDepositToIntentWithPermitTransferFromFails() (gas: 59984)
29-
TrailsIntentEntrypointTest:testDepositToIntentWithPermitWrongSigner() (gas: 40463)
30-
TrailsIntentEntrypointTest:testDepositToIntentWithoutPermit_RequiresIntentAddress() (gas: 31463)
31-
TrailsIntentEntrypointTest:testDepositToIntentWrongSigner() (gas: 57297)
32-
TrailsIntentEntrypointTest:testDepositToIntent_WithNonStandardERC20AndFee_Success() (gas: 809490)
33-
TrailsIntentEntrypointTest:testDepositToIntent_WithNonStandardERC20_InsufficientAllowance_Reverts() (gas: 772309)
34-
TrailsIntentEntrypointTest:testDepositToIntent_WithNonStandardERC20_InsufficientBalance_Reverts() (gas: 772246)
35-
TrailsIntentEntrypointTest:testDepositToIntent_WithNonStandardERC20_Success() (gas: 780354)
36-
TrailsIntentEntrypointTest:testExactApprovalFlow() (gas: 150357)
37-
TrailsIntentEntrypointTest:testExecuteIntentWithFee() (gas: 153584)
38-
TrailsIntentEntrypointTest:testExecuteIntentWithPermit() (gas: 123206)
39-
TrailsIntentEntrypointTest:testExecuteIntentWithPermitExpired() (gas: 36391)
40-
TrailsIntentEntrypointTest:testExecuteIntentWithPermitInvalidSignature() (gas: 37707)
41-
TrailsIntentEntrypointTest:testExecuteIntentWithPermit_permit_frontrun() (gas: 128090)
42-
TrailsIntentEntrypointTest:testFeeCollectorReceivesFees() (gas: 147736)
43-
TrailsIntentEntrypointTest:testFeeCollectorReceivesFeesWithoutPermit() (gas: 117408)
44-
TrailsIntentEntrypointTest:testInfiniteApprovalFlow() (gas: 144644)
45-
TrailsIntentEntrypointTest:testIntentTypehashConstant() (gas: 5685)
46-
TrailsIntentEntrypointTest:testInvalidNonceReverts() (gas: 54678)
47-
TrailsIntentEntrypointTest:testNonceIncrementsOnDeposit() (gas: 90429)
48-
TrailsIntentEntrypointTest:testPermitAmountExcessiveWithFee() (gas: 60548)
49-
TrailsIntentEntrypointTest:testPermitAmountInsufficientWithFee() (gas: 59580)
50-
TrailsIntentEntrypointTest:testVersionConstant() (gas: 10359)
9+
TrailsIntentEntrypointDeploymentTest:test_DeployIntentEntrypoint_SameAddress() (gas: 823979)
10+
TrailsIntentEntrypointDeploymentTest:test_DeployIntentEntrypoint_Success() (gas: 816517)
11+
TrailsIntentEntrypointDeploymentTest:test_DeployedIntentEntrypoint_HasCorrectConfiguration() (gas: 819316)
12+
TrailsIntentEntrypointTest:testAssemblyCodeExecution() (gas: 89389)
13+
TrailsIntentEntrypointTest:testConstructor() (gas: 3375)
14+
TrailsIntentEntrypointTest:testConstructorAndDomainSeparator() (gas: 7775)
15+
TrailsIntentEntrypointTest:testDepositToIntentAlreadyUsed() (gas: 114213)
16+
TrailsIntentEntrypointTest:testDepositToIntentCannotReuseDigest() (gas: 94919)
17+
TrailsIntentEntrypointTest:testDepositToIntentExpiredDeadline() (gas: 56099)
18+
TrailsIntentEntrypointTest:testDepositToIntentReentrancyProtection() (gas: 114191)
19+
TrailsIntentEntrypointTest:testDepositToIntentRequiresNonZeroAmount() (gas: 27419)
20+
TrailsIntentEntrypointTest:testDepositToIntentRequiresValidToken() (gas: 30824)
21+
TrailsIntentEntrypointTest:testDepositToIntentTransferFromFails() (gas: 58487)
22+
TrailsIntentEntrypointTest:testDepositToIntentWithFeeAmountButNoCollector_Reverts() (gas: 112145)
23+
TrailsIntentEntrypointTest:testDepositToIntentWithFeeCollectorButNoAmount_Reverts() (gas: 90113)
24+
TrailsIntentEntrypointTest:testDepositToIntentWithPermitAlreadyUsed() (gas: 132840)
25+
TrailsIntentEntrypointTest:testDepositToIntentWithPermitExpiredDeadline() (gas: 36541)
26+
TrailsIntentEntrypointTest:testDepositToIntentWithPermitReentrancyProtection() (gas: 124239)
27+
TrailsIntentEntrypointTest:testDepositToIntentWithPermitRequiresNonZeroAmount() (gas: 35255)
28+
TrailsIntentEntrypointTest:testDepositToIntentWithPermitRequiresPermitAmount() (gas: 60824)
29+
TrailsIntentEntrypointTest:testDepositToIntentWithPermitRequiresValidToken() (gas: 35981)
30+
TrailsIntentEntrypointTest:testDepositToIntentWithPermitTransferFromFails() (gas: 60031)
31+
TrailsIntentEntrypointTest:testDepositToIntentWithPermitWithFeeAmountButNoCollector_Reverts() (gas: 140717)
32+
TrailsIntentEntrypointTest:testDepositToIntentWithPermitWithFeeCollectorButNoAmount_Reverts() (gas: 120296)
33+
TrailsIntentEntrypointTest:testDepositToIntentWithPermitWrongSigner() (gas: 40532)
34+
TrailsIntentEntrypointTest:testDepositToIntentWithoutPermit_RequiresIntentAddress() (gas: 31533)
35+
TrailsIntentEntrypointTest:testDepositToIntentWrongSigner() (gas: 57349)
36+
TrailsIntentEntrypointTest:testDepositToIntent_WithNonStandardERC20AndFee_Success() (gas: 809616)
37+
TrailsIntentEntrypointTest:testDepositToIntent_WithNonStandardERC20_InsufficientAllowance_Reverts() (gas: 772365)
38+
TrailsIntentEntrypointTest:testDepositToIntent_WithNonStandardERC20_InsufficientBalance_Reverts() (gas: 772324)
39+
TrailsIntentEntrypointTest:testDepositToIntent_WithNonStandardERC20_Success() (gas: 780514)
40+
TrailsIntentEntrypointTest:testExactApprovalFlow() (gas: 150550)
41+
TrailsIntentEntrypointTest:testExecuteIntentWithFee() (gas: 153720)
42+
TrailsIntentEntrypointTest:testExecuteIntentWithPermit() (gas: 123362)
43+
TrailsIntentEntrypointTest:testExecuteIntentWithPermitExpired() (gas: 36460)
44+
TrailsIntentEntrypointTest:testExecuteIntentWithPermitInvalidSignature() (gas: 37754)
45+
TrailsIntentEntrypointTest:testExecuteIntentWithPermit_permit_frontrun() (gas: 128224)
46+
TrailsIntentEntrypointTest:testFeeCollectorReceivesFees() (gas: 147872)
47+
TrailsIntentEntrypointTest:testFeeCollectorReceivesFeesWithoutPermit() (gas: 117534)
48+
TrailsIntentEntrypointTest:testInfiniteApprovalFlow() (gas: 144843)
49+
TrailsIntentEntrypointTest:testIntentTypehashConstant() (gas: 5732)
50+
TrailsIntentEntrypointTest:testInvalidNonceReverts() (gas: 54704)
51+
TrailsIntentEntrypointTest:testNonceIncrementsOnDeposit() (gas: 90568)
52+
TrailsIntentEntrypointTest:testPermitAmountExcessiveWithFee() (gas: 60617)
53+
TrailsIntentEntrypointTest:testPermitAmountInsufficientWithFee() (gas: 59627)
54+
TrailsIntentEntrypointTest:testVersionConstant() (gas: 10428)
5155
TrailsRouterDeploymentTest:test_DeployTrailsRouter_SameAddress() (gas: 1856117)
5256
TrailsRouterDeploymentTest:test_DeployTrailsRouter_Success() (gas: 1846511)
5357
TrailsRouterDeploymentTest:test_DeployedRouter_HasCorrectConfiguration() (gas: 1846307)

src/TrailsIntentEntrypoint.sol

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ contract TrailsIntentEntrypoint is ReentrancyGuard, ITrailsIntentEntrypoint {
3737
error IntentExpired();
3838
error InvalidIntentSignature();
3939
error InvalidNonce();
40+
error InvalidFeeParameters();
4041
error PermitAmountMismatch();
4142

4243
// -------------------------------------------------------------------------
@@ -109,15 +110,7 @@ contract TrailsIntentEntrypoint is ReentrancyGuard, ITrailsIntentEntrypoint {
109110
// Permit may have been frontrun. Continue with transferFrom attempt.
110111
}
111112

112-
IERC20(token).safeTransferFrom(user, intentAddress, amount);
113-
114-
// Pay fee if specified (fee token is same as deposit token)
115-
if (feeAmount > 0 && feeCollector != address(0)) {
116-
IERC20(token).safeTransferFrom(user, feeCollector, feeAmount);
117-
emit FeePaid(user, token, feeAmount, feeCollector);
118-
}
119-
120-
emit IntentDeposit(user, intentAddress, amount);
113+
_processDeposit(user, token, amount, intentAddress, feeAmount, feeCollector);
121114
}
122115

123116
/// @inheritdoc ITrailsIntentEntrypoint
@@ -137,11 +130,27 @@ contract TrailsIntentEntrypoint is ReentrancyGuard, ITrailsIntentEntrypoint {
137130
_verifyAndMarkIntent(
138131
user, token, amount, intentAddress, deadline, nonce, feeAmount, feeCollector, sigV, sigR, sigS
139132
);
133+
_processDeposit(user, token, amount, intentAddress, feeAmount, feeCollector);
134+
}
140135

136+
function _processDeposit(
137+
address user,
138+
address token,
139+
uint256 amount,
140+
address intentAddress,
141+
uint256 feeAmount,
142+
address feeCollector
143+
) internal {
141144
IERC20(token).safeTransferFrom(user, intentAddress, amount);
142145

143146
// Pay fee if specified (fee token is same as deposit token)
144-
if (feeAmount > 0 && feeCollector != address(0)) {
147+
bool feeAmountSupplied = feeAmount > 0;
148+
bool feeCollectorSupplied = feeCollector != address(0);
149+
if (feeAmountSupplied != feeCollectorSupplied) {
150+
// Must supply both feeAmount and feeCollector, or neither
151+
revert InvalidFeeParameters();
152+
}
153+
if (feeAmountSupplied && feeCollectorSupplied) {
145154
IERC20(token).safeTransferFrom(user, feeCollector, feeAmount);
146155
emit FeePaid(user, token, feeAmount, feeCollector);
147156
}

test/TrailsIntentEntrypoint.t.sol

Lines changed: 237 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2114,4 +2114,241 @@ contract TrailsIntentEntrypointTest is Test {
21142114
bytes32 digest = keccak256(abi.encodePacked("\x19\x01", entrypoint.DOMAIN_SEPARATOR(), hash));
21152115
return vm.sign(userPrivateKey, digest);
21162116
}
2117+
2118+
// =========================================================================
2119+
// Fee Parameter Validation Tests
2120+
// =========================================================================
2121+
2122+
/**
2123+
* @notice Test that depositToIntent reverts when feeAmount is provided but feeCollector is not
2124+
* @dev Validates InvalidFeeParameters error when feeAmount > 0 but feeCollector == address(0)
2125+
*/
2126+
function testDepositToIntentWithFeeAmountButNoCollector_Reverts() public {
2127+
vm.startPrank(user);
2128+
2129+
address intentAddress = address(0x5678);
2130+
uint256 amount = 50 * 10 ** token.decimals();
2131+
uint256 feeAmount = 5 * 10 ** token.decimals(); // Fee amount provided
2132+
uint256 deadline = block.timestamp + 3600;
2133+
uint256 nonce = entrypoint.nonces(user);
2134+
2135+
bytes32 intentHash = keccak256(
2136+
abi.encode(
2137+
entrypoint.TRAILS_INTENT_TYPEHASH(),
2138+
user,
2139+
address(token),
2140+
amount,
2141+
intentAddress,
2142+
deadline,
2143+
block.chainid,
2144+
nonce,
2145+
feeAmount,
2146+
address(0) // No fee collector
2147+
)
2148+
);
2149+
2150+
bytes32 digest = keccak256(abi.encodePacked("\x19\x01", entrypoint.DOMAIN_SEPARATOR(), intentHash));
2151+
(uint8 v, bytes32 r, bytes32 s) = vm.sign(userPrivateKey, digest);
2152+
2153+
token.approve(address(entrypoint), amount + feeAmount);
2154+
2155+
vm.expectRevert(TrailsIntentEntrypoint.InvalidFeeParameters.selector);
2156+
entrypoint.depositToIntent(
2157+
user, address(token), amount, intentAddress, deadline, nonce, feeAmount, address(0), v, r, s
2158+
);
2159+
2160+
vm.stopPrank();
2161+
}
2162+
2163+
/**
2164+
* @notice Test that depositToIntent reverts when feeCollector is provided but feeAmount is not
2165+
* @dev Validates InvalidFeeParameters error when feeAmount == 0 but feeCollector != address(0)
2166+
*/
2167+
function testDepositToIntentWithFeeCollectorButNoAmount_Reverts() public {
2168+
vm.startPrank(user);
2169+
2170+
address intentAddress = address(0x5678);
2171+
address feeCollector = address(0x9999); // Fee collector provided
2172+
uint256 amount = 50 * 10 ** token.decimals();
2173+
uint256 feeAmount = 0; // No fee amount
2174+
uint256 deadline = block.timestamp + 3600;
2175+
uint256 nonce = entrypoint.nonces(user);
2176+
2177+
bytes32 intentHash = keccak256(
2178+
abi.encode(
2179+
entrypoint.TRAILS_INTENT_TYPEHASH(),
2180+
user,
2181+
address(token),
2182+
amount,
2183+
intentAddress,
2184+
deadline,
2185+
block.chainid,
2186+
nonce,
2187+
feeAmount,
2188+
feeCollector
2189+
)
2190+
);
2191+
2192+
bytes32 digest = keccak256(abi.encodePacked("\x19\x01", entrypoint.DOMAIN_SEPARATOR(), intentHash));
2193+
(uint8 v, bytes32 r, bytes32 s) = vm.sign(userPrivateKey, digest);
2194+
2195+
token.approve(address(entrypoint), amount);
2196+
2197+
vm.expectRevert(TrailsIntentEntrypoint.InvalidFeeParameters.selector);
2198+
entrypoint.depositToIntent(
2199+
user, address(token), amount, intentAddress, deadline, nonce, feeAmount, feeCollector, v, r, s
2200+
);
2201+
2202+
vm.stopPrank();
2203+
}
2204+
2205+
/**
2206+
* @notice Test that depositToIntentWithPermit reverts when feeAmount is provided but feeCollector is not
2207+
* @dev Validates InvalidFeeParameters error when feeAmount > 0 but feeCollector == address(0)
2208+
*/
2209+
function testDepositToIntentWithPermitWithFeeAmountButNoCollector_Reverts() public {
2210+
vm.startPrank(user);
2211+
2212+
address intentAddress = address(0x5678);
2213+
uint256 amount = 50 * 10 ** token.decimals();
2214+
uint256 feeAmount = 5 * 10 ** token.decimals(); // Fee amount provided
2215+
uint256 totalAmount = amount + feeAmount;
2216+
uint256 deadline = block.timestamp + 3600;
2217+
uint256 nonce = entrypoint.nonces(user);
2218+
2219+
// Create permit signature for total amount (deposit + fee)
2220+
bytes32 permitHash = keccak256(
2221+
abi.encodePacked(
2222+
"\x19\x01",
2223+
token.DOMAIN_SEPARATOR(),
2224+
keccak256(
2225+
abi.encode(
2226+
keccak256("Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)"),
2227+
user,
2228+
address(entrypoint),
2229+
totalAmount,
2230+
token.nonces(user),
2231+
deadline
2232+
)
2233+
)
2234+
)
2235+
);
2236+
2237+
(uint8 permitV, bytes32 permitR, bytes32 permitS) = vm.sign(userPrivateKey, permitHash);
2238+
2239+
// Create intent signature
2240+
bytes32 intentHash = keccak256(
2241+
abi.encode(
2242+
entrypoint.TRAILS_INTENT_TYPEHASH(),
2243+
user,
2244+
address(token),
2245+
amount,
2246+
intentAddress,
2247+
deadline,
2248+
block.chainid,
2249+
nonce,
2250+
feeAmount,
2251+
address(0) // No fee collector
2252+
)
2253+
);
2254+
2255+
bytes32 intentDigest = keccak256(abi.encodePacked("\x19\x01", entrypoint.DOMAIN_SEPARATOR(), intentHash));
2256+
(uint8 sigV, bytes32 sigR, bytes32 sigS) = vm.sign(userPrivateKey, intentDigest);
2257+
2258+
vm.expectRevert(TrailsIntentEntrypoint.InvalidFeeParameters.selector);
2259+
entrypoint.depositToIntentWithPermit(
2260+
user,
2261+
address(token),
2262+
amount,
2263+
totalAmount,
2264+
intentAddress,
2265+
deadline,
2266+
nonce,
2267+
feeAmount,
2268+
address(0), // No fee collector
2269+
permitV,
2270+
permitR,
2271+
permitS,
2272+
sigV,
2273+
sigR,
2274+
sigS
2275+
);
2276+
2277+
vm.stopPrank();
2278+
}
2279+
2280+
/**
2281+
* @notice Test that depositToIntentWithPermit reverts when feeCollector is provided but feeAmount is not
2282+
* @dev Validates InvalidFeeParameters error when feeAmount == 0 but feeCollector != address(0)
2283+
*/
2284+
function testDepositToIntentWithPermitWithFeeCollectorButNoAmount_Reverts() public {
2285+
vm.startPrank(user);
2286+
2287+
address intentAddress = address(0x5678);
2288+
address feeCollector = address(0x9999); // Fee collector provided
2289+
uint256 amount = 50 * 10 ** token.decimals();
2290+
uint256 feeAmount = 0; // No fee amount
2291+
uint256 deadline = block.timestamp + 3600;
2292+
uint256 nonce = entrypoint.nonces(user);
2293+
2294+
// Create permit signature for just the amount (no fee)
2295+
bytes32 permitHash = keccak256(
2296+
abi.encodePacked(
2297+
"\x19\x01",
2298+
token.DOMAIN_SEPARATOR(),
2299+
keccak256(
2300+
abi.encode(
2301+
keccak256("Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)"),
2302+
user,
2303+
address(entrypoint),
2304+
amount,
2305+
token.nonces(user),
2306+
deadline
2307+
)
2308+
)
2309+
)
2310+
);
2311+
2312+
(uint8 permitV, bytes32 permitR, bytes32 permitS) = vm.sign(userPrivateKey, permitHash);
2313+
2314+
// Create intent signature
2315+
bytes32 intentHash = keccak256(
2316+
abi.encode(
2317+
entrypoint.TRAILS_INTENT_TYPEHASH(),
2318+
user,
2319+
address(token),
2320+
amount,
2321+
intentAddress,
2322+
deadline,
2323+
block.chainid,
2324+
nonce,
2325+
feeAmount,
2326+
feeCollector
2327+
)
2328+
);
2329+
2330+
bytes32 intentDigest = keccak256(abi.encodePacked("\x19\x01", entrypoint.DOMAIN_SEPARATOR(), intentHash));
2331+
(uint8 sigV, bytes32 sigR, bytes32 sigS) = vm.sign(userPrivateKey, intentDigest);
2332+
2333+
vm.expectRevert(TrailsIntentEntrypoint.InvalidFeeParameters.selector);
2334+
entrypoint.depositToIntentWithPermit(
2335+
user,
2336+
address(token),
2337+
amount,
2338+
amount,
2339+
intentAddress,
2340+
deadline,
2341+
nonce,
2342+
feeAmount,
2343+
feeCollector, // Fee collector provided with no fee amount
2344+
permitV,
2345+
permitR,
2346+
permitS,
2347+
sigV,
2348+
sigR,
2349+
sigS
2350+
);
2351+
2352+
vm.stopPrank();
2353+
}
21172354
}

0 commit comments

Comments
 (0)