Context:
The LockManager contract has a function lockOnBehalf(), which allows anyone else to lock on behalf of a user (used by Migration manager mainly).
Issue:
The issue is that an attacker can pass 0 as the _quantity parameter and the call will simply go through.
Impact:
Since on every lock, the unlock time is extended by the lock duration (see here), the attacker can make these calls every now and then to block users from unlocking their tokens ever. Additionally, the gas price on Blast is low, which makes it easier for the attacker.
This is also an attack idea by the sponsor in the README here.
Proof of Concept
Assume user has locked 100 tokens using the lock() function previously. The unlockTime = 100 and lockDuration set by the user = anywhere between 30 days to 90 days.
Attacker calls the lockOnBehalf() with 0 as value for the _quantity parameter and the _onBehalfOf parameter as the user's address.
The call is not blocked anywhere in the internal _lock() function (Tip: CTRL/CMD + F the _quantity parameter in the _lock() function to see where it is used and why it is not blocked). Other than the protocol specific check for allowance here, the ERC20 standard allows zero value transfers so the call will go through.
On Line 391, we can see how the unlockTime would be extended from the current time (by atleast 30 days since that's the minLockDuration). This would prevent unlocks and thus brick the user funds in the contract.
Lines of code
https://github.com/code-423n4/2024-05-munchables/blob/57dff486c3cd905f21b330c2157fe23da2a4807d/src/managers/LockManager.sol#L275
Vulnerability details
Summary
Context: The LockManager contract has a function lockOnBehalf(), which allows anyone else to lock on behalf of a user (used by Migration manager mainly).
Issue: The issue is that an attacker can pass 0 as the
_quantity
parameter and the call will simply go through.Impact: Since on every lock, the unlock time is extended by the lock duration (see here), the attacker can make these calls every now and then to block users from unlocking their tokens ever. Additionally, the gas price on Blast is low, which makes it easier for the attacker.
This is also an attack idea by the sponsor in the README here.
Proof of Concept
Assume user has locked 100 tokens using the lock() function previously. The unlockTime = 100 and lockDuration set by the user = anywhere between 30 days to 90 days.
Attacker calls the lockOnBehalf() with 0 as value for the
_quantity
parameter and the_onBehalfOf
parameter as the user's address.The call is not blocked anywhere in the internal _lock() function (Tip: CTRL/CMD + F the _quantity parameter in the _lock() function to see where it is used and why it is not blocked). Other than the protocol specific check for allowance here, the ERC20 standard allows zero value transfers so the call will go through.
On Line 391, we can see how the unlockTime would be extended from the current time (by atleast 30 days since that's the minLockDuration). This would prevent unlocks and thus brick the user funds in the contract.
Tools Used
Manual Review
Recommended Mitigation Steps
Implement an approval mechanism, which allows a user to approve a contract or EOA to lock tokens on behalf of them.
Assessed type
DoS