Cross-Function Reentrancy in a Staking Contract
A guarded withdraw function and an unguarded transfer function share state. The attacker re-enters the one nobody locked.
The lab contract lets users stake ETH and transfer staked balances. unstake() carries a nonReentrant modifier, which gives a false sense of safety: transferStake() does not, and both read and write the same stakes mapping.
criticalXV-DEMO-04 · Verified by testStake can be transferred during the unstake callback and withdrawn twice
Stake can be transferred during the unstake callback and withdrawn twice
Description
unstake() sends ETH before zeroing stakes[msg.sender]. During the callback, the attacker calls transferStake() to move the still-recorded balance to an accomplice, who then unstakes it again.
Impact
Each round pays out twice the attacker's stake, taken from other stakers' funds.
Recommendation
Zero the balance before the external call, and put every function that touches stakes under the same lock.
Measured in the lab: with an honest staker holding 10 ETH, the exploit contract staked 1 ETH and the attacker and accomplice withdrew 2 ETH between them, leaving the contract holding 9 ETH against the honest staker's 10 ETH claim.
Against FixedStaking the same exploit reverts. A randomized test of FixedStaking — 200 sequences of 25 random stake, transfer and unstake calls across 5 accounts — found no state where the contract's balance fell below the sum of recorded stakes.
The lab code and tests for this case study live in labs/web3 of the Xevry website repository.
cd labs/web3npm installnpx hardhat test test/StakingReentrancy.test.jsFiles: labs/web3/contracts/Staking.sol · labs/web3/test/StakingReentrancy.test.js
Let's discuss your security needs.
Tell us what you're building. We'll respond with clear questions, an honest view of whether we're the right fit, and a scoped proposal if we are.