code-423n4 / 2023-04-eigenlayer-findings

1 stars 1 forks source link

Unverified signature can bypass deposit check. #232

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-04-eigenlayer/blob/5e4872358cd2bda1936c29f460ece2308af4def6/src/contracts/core/StrategyManager.sol#L289-L298

Vulnerability details

Impact

The depositIntoStrategyWithSignature function does not properly verify that the msg.sender matches the signer of the signature. This allows an attacker to submit a valid signature that matches an authorized staker address, and then call the depositIntoStrategyWithSignature function with their own address to bypass the verification check and deposit tokens into a strategy they do not have authorization to access.

Proof of Concept

The vulnerable code can be found in the depositIntoStrategyWithSignature function: StrategyManager.sol#L289-L298

        if (Address.isContract(staker)) {
            require(IERC1271(staker).isValidSignature(digestHash, signature) == ERC1271_MAGICVALUE,
                "StrategyManager.depositIntoStrategyWithSignature: ERC1271 signature verification failed");
        } else {
            require(ECDSA.recover(digestHash, signature) == staker,
                "StrategyManager.depositIntoStrategyWithSignature: signature not from staker");
        }

        shares = _depositIntoStrategy(staker, strategy, token, amount);
    }

As we can see, the function checks for a valid signature using either the isValidSignature method of the IERC1271 contract or by recovering the signer's address using ECDSA.recover, but it does not verify whether the recovered address matches the msg.sender who is calling the function.

An attacker can craft a valid signature that matches an authorized staker address, and then call the depositIntoStrategyWithSignature function with their own address as the msg.sender. This way, the attacker can bypass the signature verification check and deposit tokens into a strategy they do not have authorization to access.

Tools Used

vscode

Recommended Mitigation Steps

The depositIntoStrategyWithSignature function should be updated to verify that the recovered signer's address matches the msg.sender before allowing the deposit to proceed by adding an additional check to the function, such as:

address recoveredSigner = ECDSA.recover(digestHash, signature);
require(recoveredSigner == msg.sender, "StrategyManager.depositIntoStrategyWithSignature: signer address does not match msg.sender");

This way, even if an attacker crafts a valid signature, they will not be able to deposit tokens into a strategy unless they also have control over the authorized staker's private key.

Assessed type

Access Control

c4-pre-sort commented 1 year ago

0xSorryNotSorry marked the issue as low quality report

c4-judge commented 1 year ago

GalloDaSballo marked the issue as unsatisfactory: Invalid