code423n4 commented 1 year ago

Lines of code

https://github.com/Layr-Labs/eigenlayer-contracts/blob/master/src/contracts/core/StrategyManager.sol#L293

Vulnerability details

Impact

Signature Replay attack possible on depositing Assets into strategies by staker.

Proof of Concept

In depositIntoStrategyWithSignature method, Openzeppelin's ECDSA Library is used to check that the signature is a valid ECDSA signature from staker. In case, it is correct, the method goes on to deposit the assets into the strategy.

File: StrategyManager.sol

293:  require(ECDSA.recover(digestHash, signature) == staker, // @audit-issue Signature Replay Possible here
294:            "StrategyManager.depositIntoStrategyWithSignature: signature not from staker");

297:  shares = _depositIntoStrategy(staker, strategy, token, amount);

Link to code

Here the version used of ECDSA is 4.7.0 which contains the Signature Malleability Issue that openzeppelin have acknowledged here. As per them:

The functions ECDSA.recover and ECDSA.tryRecover are vulnerable to a kind of signature malleability due to accepting EIP-2098 compact signatures in addition to the traditional 65 byte signature format.

Openzeppelin supported Signature Representations in ECDSA v4.7.0:

Standard 65 Bytes signature format
EIP-2098 compact signature (64 Bytes)

This version used if-else Condition as to determine the signature format. But there was a case where a caller may take a signature that has already been submitted, submit it again in a different format, and bypass this protection.

This is only an issue for the functions that take a single bytes argument, and not the functions that take r, v, s or r, vs as separate arguments. But here, in depositIntoStrategyWithSignature, it exactly uses single bytes signature argument making it a clear attack vector for Signature Malleability.