code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-06-llama/blob/main/src/LlamaCore.sol#L284-L300 https://github.com/code-423n4/2023-06-llama/blob/main/src/LlamaCore.sol#L714-L742

Vulnerability details

Impact

In LlamaCore.sol we have createActionBySig() function:

function createActionBySig(
    address policyholder,
    uint8 role,
    ILlamaStrategy strategy,
    address target,
    uint256 value,
    bytes calldata data,
    string memory description,
    uint8 v,
    bytes32 r,
    bytes32 s
  ) external returns (uint256 actionId) { //@audit -  not check deadline
    bytes32 digest = _getCreateActionTypedDataHash(policyholder, role, strategy, target, value, data, description);
    address signer = ecrecover(digest, v, r, s);
    if (signer == address(0) || signer != policyholder) revert InvalidSignature();
    actionId = _createAction(signer, role, strategy, target, value, data, description);
  }

This function is for creating an action via an off-chain signature. It checks whether the signature is valid and matches the policyholder's address, and if so, it creates an action. createActionBySig call _getCreateActionTypedDataHash() which generates a unique hash representing a specific action. But in both functions, the signed data doesn't include the chainId and because of this cross-chain replay attacks are possible.

Proof of Concept

From the docs we see:

Is it multi-chain?: It should be able to be deployed an any popular EVM chain

According to EIP4337 standard to prevent replay attacks the signature should depend on chainId. But in these functions, chainId is not used. A valid signature that was used on one chain could be copied by an attacker and propagated onto another chain.

Absolutely the same applies to the functions castApprovalBySig and castDisapprovalBySig.

Tools Used

Manual review

Recommended Mitigation Steps

Add chainId in createActionHash():

bytes32 createActionHash = keccak256(
      abi.encode(
        CREATE_ACTION_TYPEHASH,
        policyholder,
        role,
        address(strategy),
        target,
        value,
        keccak256(data),
        keccak256(bytes(description)),
        nonce,
        block.chainId
      )