Skip to content

Commit 65e99f9

Browse files
tmigonetmigone
committed
fix: prevent payers from being able to bypass escrow mechanism (TRST-H01)
Signed-off-by: Tomás Migone <[email protected]>
1 parent 0c0d090 commit 65e99f9

File tree

3 files changed

+131
-27
lines changed

3 files changed

+131
-27
lines changed

‎packages/horizon/contracts/interfaces/ITAPCollector.sol

+6
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,12 @@ interface ITAPCollector is IPaymentsCollector {
137137
*/
138138
error TAPCollectorInvalidRAVSigner();
139139

140+
/**
141+
* Thrown when the RAV is for a data service the service provider has no provision for
142+
* @param dataService The address of the data service
143+
*/
144+
error TAPCollectorUnauthorizedDataService(address dataService);
145+
140146
/**
141147
* Thrown when the caller is not the data service the RAV was issued to
142148
* @param caller The address of the caller

‎packages/horizon/contracts/payments/collectors/TAPCollector.sol

+11
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// SPDX-License-Identifier: GPL-3.0-or-later
22
pragma solidity 0.8.27;
33

4+
import { IHorizonStaking } from "../../interfaces/IHorizonStaking.sol";
45
import { IGraphPayments } from "../../interfaces/IGraphPayments.sol";
56
import { ITAPCollector } from "../../interfaces/ITAPCollector.sol";
67

@@ -118,6 +119,7 @@ contract TAPCollector is EIP712, GraphDirectory, ITAPCollector {
118119
* @notice Initiate a payment collection through the payments protocol
119120
* See {IGraphPayments.collect}.
120121
* @dev Caller must be the data service the RAV was issued to.
122+
* @dev Service provider must have an active provision with the data service to collect payments
121123
* @notice REVERT: This function may revert if ECDSA.recover fails, check ECDSA library for details.
122124
*/
123125
function collect(IGraphPayments.PaymentTypes paymentType, bytes memory data) external override returns (uint256) {
@@ -130,6 +132,15 @@ contract TAPCollector is EIP712, GraphDirectory, ITAPCollector {
130132
address signer = _recoverRAVSigner(signedRAV);
131133
require(authorizedSigners[signer].payer != address(0), TAPCollectorInvalidRAVSigner());
132134

135+
// Check the service provider has an active provision with the data service
136+
// This prevents an attack where the payer can deny the service provider from collecting payments
137+
// by using a signer as data service to syphon off the tokens in the escrow to an account they control
138+
uint256 tokensAvailable = _graphStaking().getProviderTokensAvailable(
139+
signedRAV.rav.serviceProvider,
140+
signedRAV.rav.dataService
141+
);
142+
require(tokensAvailable > 0, TAPCollectorUnauthorizedDataService(signedRAV.rav.dataService));
143+
133144
return _collect(paymentType, authorizedSigners[signer].payer, signedRAV, dataServiceCut);
134145
}
135146

‎packages/horizon/test/payments/tap-collector/collect/collect.t.sol

+114-27
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,9 @@ import { IGraphPayments } from "../../../../contracts/interfaces/IGraphPayments.
99
import { TAPCollectorTest } from "../TAPCollector.t.sol";
1010

1111
contract TAPCollectorCollectTest is TAPCollectorTest {
12-
1312
/*
14-
* HELPERS
15-
*/
13+
* HELPERS
14+
*/
1615

1716
function _getQueryFeeEncodedData(
1817
uint256 _signerPrivateKey,
@@ -47,38 +46,122 @@ contract TAPCollectorCollectTest is TAPCollectorTest {
4746
* TESTS
4847
*/
4948

50-
function testTAPCollector_Collect(uint256 tokens) public useGateway useSigner {
49+
function testTAPCollector_Collect(
50+
uint256 tokens
51+
) public useIndexer useProvisionDataService(users.verifier, 100, 0, 0) useGateway useSigner {
5152
tokens = bound(tokens, 1, type(uint128).max);
52-
53+
5354
_approveCollector(address(tapCollector), tokens);
5455
_depositTokens(address(tapCollector), users.indexer, tokens);
55-
56+
5657
bytes memory data = _getQueryFeeEncodedData(signerPrivateKey, users.indexer, users.verifier, uint128(tokens));
5758

5859
resetPrank(users.verifier);
5960
_collect(IGraphPayments.PaymentTypes.QueryFee, data);
6061
}
6162

62-
function testTAPCollector_Collect_Multiple(uint256 tokens, uint8 steps) public useGateway useSigner {
63+
function testTAPCollector_Collect_Multiple(
64+
uint256 tokens,
65+
uint8 steps
66+
) public useIndexer useProvisionDataService(users.verifier, 100, 0, 0) useGateway useSigner {
6367
steps = uint8(bound(steps, 1, 100));
6468
tokens = bound(tokens, steps, type(uint128).max);
65-
69+
6670
_approveCollector(address(tapCollector), tokens);
6771
_depositTokens(address(tapCollector), users.indexer, tokens);
6872

6973
resetPrank(users.verifier);
7074
uint256 payed = 0;
7175
uint256 tokensPerStep = tokens / steps;
7276
for (uint256 i = 0; i < steps; i++) {
73-
bytes memory data = _getQueryFeeEncodedData(signerPrivateKey, users.indexer, users.verifier, uint128(payed + tokensPerStep));
77+
bytes memory data = _getQueryFeeEncodedData(
78+
signerPrivateKey,
79+
users.indexer,
80+
users.verifier,
81+
uint128(payed + tokensPerStep)
82+
);
7483
_collect(IGraphPayments.PaymentTypes.QueryFee, data);
7584
payed += tokensPerStep;
7685
}
7786
}
7887

88+
function testTAPCollector_Collect_RevertWhen_NoProvision(uint256 tokens) public useGateway useSigner {
89+
tokens = bound(tokens, 1, type(uint128).max);
90+
91+
_approveCollector(address(tapCollector), tokens);
92+
_depositTokens(address(tapCollector), users.indexer, tokens);
93+
94+
bytes memory data = _getQueryFeeEncodedData(signerPrivateKey, users.indexer, users.verifier, uint128(tokens));
95+
96+
resetPrank(users.verifier);
97+
bytes memory expectedError = abi.encodeWithSelector(
98+
ITAPCollector.TAPCollectorUnauthorizedDataService.selector,
99+
users.verifier
100+
);
101+
vm.expectRevert(expectedError);
102+
tapCollector.collect(IGraphPayments.PaymentTypes.QueryFee, data);
103+
}
104+
105+
function testTAPCollector_Collect_RevertWhen_ProvisionEmpty(
106+
uint256 tokens
107+
) public useIndexer useProvisionDataService(users.verifier, 100, 0, 0) useGateway useSigner {
108+
// thaw tokens from the provision
109+
resetPrank(users.indexer);
110+
staking.thaw(users.indexer, users.verifier, 100);
111+
112+
tokens = bound(tokens, 1, type(uint128).max);
113+
114+
resetPrank(users.gateway);
115+
_approveCollector(address(tapCollector), tokens);
116+
_depositTokens(address(tapCollector), users.indexer, tokens);
117+
118+
bytes memory data = _getQueryFeeEncodedData(signerPrivateKey, users.indexer, users.verifier, uint128(tokens));
119+
120+
resetPrank(users.verifier);
121+
bytes memory expectedError = abi.encodeWithSelector(
122+
ITAPCollector.TAPCollectorUnauthorizedDataService.selector,
123+
users.verifier
124+
);
125+
vm.expectRevert(expectedError);
126+
tapCollector.collect(IGraphPayments.PaymentTypes.QueryFee, data);
127+
}
128+
129+
function testTAPCollector_Collect_PreventSignerAttack(
130+
uint256 tokens
131+
) public useIndexer useProvisionDataService(users.verifier, 100, 0, 0) useGateway useSigner {
132+
tokens = bound(tokens, 1, type(uint128).max);
133+
134+
resetPrank(users.gateway);
135+
_approveCollector(address(tapCollector), tokens);
136+
_depositTokens(address(tapCollector), users.indexer, tokens);
137+
138+
// The sender authorizes another signer
139+
(address anotherSigner, uint256 anotherSignerPrivateKey) = makeAddrAndKey("anotherSigner");
140+
uint256 proofDeadline = block.timestamp + 1;
141+
bytes memory anotherSignerProof = _getSignerProof(proofDeadline, anotherSignerPrivateKey);
142+
_authorizeSigner(anotherSigner, proofDeadline, anotherSignerProof);
143+
144+
// And crafts a RAV using the new signer as the data service
145+
bytes memory data = _getQueryFeeEncodedData(
146+
anotherSignerPrivateKey,
147+
users.indexer,
148+
anotherSigner,
149+
uint128(tokens)
150+
);
151+
152+
// the call should revert because the service provider has no provision with the "data service"
153+
resetPrank(anotherSigner);
154+
bytes memory expectedError = abi.encodeWithSelector(
155+
ITAPCollector.TAPCollectorUnauthorizedDataService.selector,
156+
anotherSigner
157+
);
158+
vm.expectRevert(expectedError);
159+
tapCollector.collect(IGraphPayments.PaymentTypes.QueryFee, data);
160+
}
161+
79162
function testTAPCollector_Collect_RevertWhen_CallerNotDataService(uint256 tokens) public useGateway useSigner {
80163
tokens = bound(tokens, 1, type(uint128).max);
81-
164+
82165
resetPrank(users.gateway);
83166
_approveCollector(address(tapCollector), tokens);
84167
_depositTokens(address(tapCollector), users.indexer, tokens);
@@ -95,9 +178,11 @@ contract TAPCollectorCollectTest is TAPCollectorTest {
95178
tapCollector.collect(IGraphPayments.PaymentTypes.QueryFee, data);
96179
}
97180

98-
function testTAPCollector_Collect_RevertWhen_InconsistentRAVTokens(uint256 tokens) public useGateway useSigner {
181+
function testTAPCollector_Collect_RevertWhen_InconsistentRAVTokens(
182+
uint256 tokens
183+
) public useIndexer useProvisionDataService(users.verifier, 100, 0, 0) useGateway useSigner {
99184
tokens = bound(tokens, 1, type(uint128).max);
100-
185+
101186
_approveCollector(address(tapCollector), tokens);
102187
_depositTokens(address(tapCollector), users.indexer, tokens);
103188
bytes memory data = _getQueryFeeEncodedData(signerPrivateKey, users.indexer, users.verifier, uint128(tokens));
@@ -106,37 +191,37 @@ contract TAPCollectorCollectTest is TAPCollectorTest {
106191
_collect(IGraphPayments.PaymentTypes.QueryFee, data);
107192

108193
// Attempt to collect again
109-
vm.expectRevert(abi.encodeWithSelector(
110-
ITAPCollector.TAPCollectorInconsistentRAVTokens.selector,
111-
tokens,
112-
tokens
113-
));
194+
vm.expectRevert(
195+
abi.encodeWithSelector(ITAPCollector.TAPCollectorInconsistentRAVTokens.selector, tokens, tokens)
196+
);
114197
tapCollector.collect(IGraphPayments.PaymentTypes.QueryFee, data);
115198
}
116199

117200
function testTAPCollector_Collect_RevertWhen_SignerNotAuthorized(uint256 tokens) public useGateway {
118201
tokens = bound(tokens, 1, type(uint128).max);
119-
202+
120203
_approveCollector(address(tapCollector), tokens);
121204
_depositTokens(address(tapCollector), users.indexer, tokens);
122-
205+
123206
bytes memory data = _getQueryFeeEncodedData(signerPrivateKey, users.indexer, users.verifier, uint128(tokens));
124207

125208
resetPrank(users.verifier);
126209
vm.expectRevert(abi.encodeWithSelector(ITAPCollector.TAPCollectorInvalidRAVSigner.selector));
127210
tapCollector.collect(IGraphPayments.PaymentTypes.QueryFee, data);
128211
}
129212

130-
function testTAPCollector_Collect_ThawingSigner(uint256 tokens) public useGateway useSigner {
213+
function testTAPCollector_Collect_ThawingSigner(
214+
uint256 tokens
215+
) public useIndexer useProvisionDataService(users.verifier, 100, 0, 0) useGateway useSigner {
131216
tokens = bound(tokens, 1, type(uint128).max);
132-
217+
133218
_approveCollector(address(tapCollector), tokens);
134219
_depositTokens(address(tapCollector), users.indexer, tokens);
135220

136221
// Start thawing signer
137222
_thawSigner(signer);
138223
skip(revokeSignerThawingPeriod + 1);
139-
224+
140225
bytes memory data = _getQueryFeeEncodedData(signerPrivateKey, users.indexer, users.verifier, uint128(tokens));
141226

142227
resetPrank(users.verifier);
@@ -145,33 +230,35 @@ contract TAPCollectorCollectTest is TAPCollectorTest {
145230

146231
function testTAPCollector_Collect_RevertIf_SignerWasRevoked(uint256 tokens) public useGateway useSigner {
147232
tokens = bound(tokens, 1, type(uint128).max);
148-
233+
149234
_approveCollector(address(tapCollector), tokens);
150235
_depositTokens(address(tapCollector), users.indexer, tokens);
151236

152237
// Start thawing signer
153238
_thawSigner(signer);
154239
skip(revokeSignerThawingPeriod + 1);
155240
_revokeAuthorizedSigner(signer);
156-
241+
157242
bytes memory data = _getQueryFeeEncodedData(signerPrivateKey, users.indexer, users.verifier, uint128(tokens));
158243

159244
resetPrank(users.verifier);
160245
vm.expectRevert(abi.encodeWithSelector(ITAPCollector.TAPCollectorInvalidRAVSigner.selector));
161246
tapCollector.collect(IGraphPayments.PaymentTypes.QueryFee, data);
162247
}
163248

164-
function testTAPCollector_Collect_ThawingSignerCanceled(uint256 tokens) public useGateway useSigner {
249+
function testTAPCollector_Collect_ThawingSignerCanceled(
250+
uint256 tokens
251+
) public useIndexer useProvisionDataService(users.verifier, 100, 0, 0) useGateway useSigner {
165252
tokens = bound(tokens, 1, type(uint128).max);
166-
253+
167254
_approveCollector(address(tapCollector), tokens);
168255
_depositTokens(address(tapCollector), users.indexer, tokens);
169256

170257
// Start thawing signer
171258
_thawSigner(signer);
172259
skip(revokeSignerThawingPeriod + 1);
173260
_cancelThawSigner(signer);
174-
261+
175262
bytes memory data = _getQueryFeeEncodedData(signerPrivateKey, users.indexer, users.verifier, uint128(tokens));
176263

177264
resetPrank(users.verifier);

0 commit comments

Comments
 (0)