Repository navigation
Harden Operator↔Sponsorship interactions - #1093
Merged
Merged
Conversation
jtakalai
approved these changes
Oct 10, 2026
jtakalai
left a comment
There was a problem hiding this comment.
Looks to me like it does what it says on the tin. Preventing fishermaning own Operator is definitely good, preventing staking for others makes sense given the meaning of staking in the network.
hpihkala
added this pull request to stack #1095
October 10, 2026 10:09
hpihkala
force-pushed
the
other-fixes
branch
2 times, most recently
from
October 10, 2026 10:40
9455ee7 to
7f987ad
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Harden Operator↔Sponsorship interactions:
triggerAnotherOperatorWithdrawdouble-minting and staking on behalf of othersFollow-up to the
withdrawEarningsFromSponsorshipsfix (#1092). While auditing all token flows betweenOperatorandSponsorshipcontracts we found two further issues. Neither is as severe as the original drain, but both let an outsider tamper with another party's accounting, so we fix them in the same release.1.
Operator.triggerAnotherOperatorWithdrawaccepts any address as the "other Operator"Problem
QueueModule._triggerAnotherOperatorWithdraw(otherOperator, sponsorships)measures the DATA balance delta around the callotherOperator.withdrawEarningsFromSponsorships(sponsorships)and mints the delta to the owner as a fisherman reward (via_delegate). It did not check whatotherOperatoractually is:withdrawEarningsFromSponsorships, re-enter the Operator and e.g. delegate DATA to an address of its choosing. That DATA is minted once as a delegation and then minted again to the owner as a fisherman reward, as it also shows up in the outer balance delta. With a 10000 DATA attacker deposit the owner's position inflates from 1380 to 10702 DATA, the attacker's 10000 is worth 5093, and the delegator falls from 9000 to 4584.Only the Operator's owner / nodes can call
triggerAnotherOperatorWithdraw, so this is an insider attack against the pool's delegators rather than an external drain — but it breaks the invariant that owner and delegators share the pool pro rata.Fix
QueueModule._triggerAnotherOperatorWithdrawnow rejects:otherOperatorAddress == address(this)→CannotTriggerOwnWithdraw()OperatorFactory(deploymentTimestamp(addr) == 0) →AccessDeniedStreamrOperatorOnly()Real Operators run the already-fixed
withdrawEarningsFromSponsorships(balance-delta based,NotMyStakedSponsorshipcheck), so a genuine Operator cannot be used to smuggle extra DATA in either.2. Anyone can stake on behalf of any address in
SponsorshipProblem
Both staking entry points accepted a beneficiary that was different from the payer:
Sponsorship.stake(address operator, uint amount)pulled DATA frommsg.senderbut creditedoperator.onTokenTransfer(ERC677transferAndCall) decoded a 20- or 32-byte address fromdataand credited that address.Hence any third party could, with their own DATA:
stakedInto/indexOfSponsorshipsbookkeeping knows nothing about it. It will not run the stream, so it can be flagged and slashed there, andwithdrawEarningsFromSponsorshipsrejects the Sponsorship asNotMyStakedSponsorship.Operator.onTokenTransfertreats DATA coming back from a staked Sponsorship as earnings, so the surprise stake also distorts the pool's value accounting when it's eventually unstaked.(Note the Sponsorship side does not even get a chance to notice:
OperatorContractOnlyJoinPolicyonly checks that the beneficiary is an Operator, not who paid.)Fix
Sponsorshipnow requires the staker to be the beneficiary:stake(operator, amount)reverts withCannotStakeForOthers(operator, staker)unlessoperator == _msgSender().onTokenTransferreverts with the same error if the decoded address differs from the ERC677sender, in both the 20-byte and 32-byte branches.Operators are unaffected:StakeModule._stakealready callssponsorship.stake(address(this), amount).Tests
Operator.test.ts: "triggerAnotherOperatorWithdraw does NOT accept the Operator itself or a non-Operator as the other Operator" — self-call reverts withCannotTriggerOwnWithdraw, a fake re-entering contract (TestFakeOperator) reverts withAccessDeniedStreamrOperatorOnly, and the honest path still yields the expected pro-rata split (owner 1038 / delegator 9342).Sponsorship.test.ts: "will NOT let stake on behalf of someone else" — coversapprove+stake(), 20-byte and 32-bytetransferAndCalldata, both for a fresh join and for topping up an existing stake. Verified to fail on the previousSponsorship.sol.Sponsorship.test.ts,MaxOperatorsJoinPolicy.test.ts,OperatorContractOnlyJoinPolicy.test.ts). The "Multiple stakings and sponsorings" scenario additionally had its un-awaitedadvanceToTimestampcalls (which raced against the following tx, hence the old "withdraw goes to block +1" comments) replaced by an awaitedevm_setNextBlockTimestamp, so intermediate earnings expectations are now exact round numbers; final payouts are unchanged.testcontracts/TestFakeOperator.sol.Full
test/hardhat/OperatorTokenomicssuite: 289 passing.solhintandeslintclean.Deployment
Both
OperatorandSponsorshipare EIP-1167 clones with no upgrade path, so existing contracts cannot be patched; only newly created ones pick up the fixes.Operatortemplate +NodeModule,QueueModule,StakeModuleOperatorFactory.updateTemplates(...)(scripts/upgradeOperatorTemplate.ts). OnlyQueueModulechanged in this PR, but the modules inheritOperatorfor storage layout and are always rolled together.Operator.version()stays at 4 (same release as thewithdrawEarningsfix; the version-history comment lists this fix under 4).SponsorshiptemplateSponsorshipFactory.updateTemplate(newTemplate).OperatorFactory,SponsorshipFactory,StreamrConfig, policies