code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/liquid-staking/LPToken.sol#L66-L70

Vulnerability details

Impact

The LP tokens have a mechanism that prevent certain interactions if some LP tokens have been recently added to the calling address. This can be used by a malicious actor to grief an address by sending tiny amounts of LP tokens to that address, either in certain intervals or by watching the mempools for transactions from that address and frontrunning those.

The features that are affected are claiming rewards, burning LPs for Ether, rotating LP tokens, and withdrawing LP tokens.

Proof of Concept

The LPToken._afterTokenTransfer hook sets the lastInteractedTimestamp for both _from and _to:

lastInteractedTimestamp[_from] = block.timestamp;
lastInteractedTimestamp[_to] = block.timestamp;

This lastInteractedTimestamp is used to restrict the execution of certain functions such as SavEthVault.burnLPToken:

bool isStaleLiquidity = _lpToken.lastInteractedTimestamp(msg.sender) + 30 minutes < block.timestamp;
...
require(isStaleLiquidity, "Liquidity is still fresh");

This allows transfers of tiny amounts to an address to prevent that address from calling functions that use the lastInteractedTimestamp in require statements.

Tools Used

Manual Review

Recommended Mitigation Steps

A workaround could be by involving a checkpointing mechanism that stores balances for a blocknumber whenever the balance changes. However that would increase implementation complexity by a fair margin.

c4-judge commented 1 year ago

dmvt marked the issue as duplicate of #49