fix: subgraphService access control (OZ C-04) #990
Conversation
fix: subgraphService access control (OZ C-04)
🚨 Report Summary
For more details view the full report in OpenZeppelin Code Inspector |
packages/subgraph-service/contracts/interfaces/ISubgraphService.sol
Outdated
Show resolved
Hide resolved
| Allocation.State memory allocation = allocations.get(allocationId); | ||
| require(allocation.indexer == indexer, SubgraphServiceInvalidAllocationIndexer(indexer, allocationId)); |
There was a problem hiding this comment.
I think this check is not needed. The allocation id is retrieved from a signed RAV which is already being validated later.
There was a problem hiding this comment.
I mean tokens collected get directly sent to the indexer specified in the RAV so they cant be stolen. And it would make no sense for an indexer to collect someone elses RAV because they would be locking their stake and get zero tokens out of it.
There was a problem hiding this comment.
As discussed I'll leave this check in case an indexer receives a voucher with a wrong allocationId so they don't accidentally lock in. their stake to collect another indexer's voucher. Also added a comment with this information.
packages/subgraph-service/contracts/interfaces/ISubgraphService.sol
Outdated
Show resolved
Hide resolved
packages/subgraph-service/contracts/utilities/AllocationManager.sol
Outdated
Show resolved
Hide resolved
packages/subgraph-service/contracts/utilities/AllocationManager.sol
Outdated
Show resolved
Hide resolved
packages/subgraph-service/contracts/utilities/AllocationManager.sol
Outdated
Show resolved
Hide resolved
|
Just realized we also need to ensure |
I'll work next on resize on a new PR |
|
|
||
| // release expired stake claims | ||
| _releaseStake(indexer, 0); | ||
| _releaseStake(_indexer, 0); |
There was a problem hiding this comment.
nit: this change is not needed since indexer = _indexer :p
pcarranzav
changed the title
|
I'd suggest fixing resizeAllocation on this PR instead of a separate one, otherwise we risk forgetting about it... or make it super super clear that this PR doesn't fully resolve C-04 Edit: just realized it's in #992 - I'd suggest mentioning this in the PR description |
|
@pcarranzav I'll mention it on the other PR, thank you! 👍 |
tmigone
force-pushed
the
mde/pr989-fix-oz-c-03
branch
from
fb00c7a to
3999eb1
Compare
tmigone
force-pushed
the
mde/pr990-fix-oz-c-04
branch
from
d09457b to
320b0d9
Compare
tmigone
force-pushed
the
mde/pr989-fix-oz-c-03
branch
from
3999eb1 to
a183d7e
Compare
tmigone
force-pushed
the
mde/pr990-fix-oz-c-04
branch
from
320b0d9 to
0364b41
Compare
No description provided.