Lines of code
<https://github.com/code-423n4/2023-06-llama/blob/9d422c264b57657098c2784aa951852cad32e01c/src/LlamaCore.sol#L259-L268&gt;
<https://github.com/code-423n4/2023-06-llama/blob/9d422c264b57657098c2784aa951852cad32e01c/src/LlamaCore.sol#L516-L562&gt;
<https://github.com/code-423n4/2023-06-llama/blob/9d422c264b57657098c2784aa951852cad32e01c/src/LlamaCore.sol#L565-L584&gt;
<https://github.com/code-423n4/2023-06-llama/blob/9d422c264b57657098c2784aa951852cad32e01c/src/LlamaCore.sol#L365-L367&gt;
<https://github.com/code-423n4/2023-06-llama/blob/9d422c264b57657098c2784aa951852cad32e01c/src/LlamaCore.sol#L398-L400&gt;

Vulnerability details

Impact

  /// @notice Mapping of policyholders to function selectors to current nonces for EIP-712 signatures.
  /// @dev This is used to prevent replay attacks by incrementing the nonce for each operation (createAction,
  /// castApproval and castDisapproval) signed by the policyholder.
  mapping(address => mapping(bytes4 => uint256)) public nonces;

• Important to note that the comment itself describes the intended purpose of the nonces mapping in relation to preventing replay attacks

• To prevent replay attacks, it is crucial to increment the nonce for each operation that requires a signature from the policyholder. The provided code snippet does not include the actual code to increment the nonce values in the nonces mapping. Therefore, it appears to be missing the implementation necessary to fulfill the purpose mentioned in the comment.

• createAction,castApproval and castDisapproval functions not implemented the nonces in protocol implementations as mentioned in comments

• Without the actual incrementing of nonces, the code as shown would indeed be vulnerable to replay attacks. Replay attacks could occur if an attacker intercepts a signed message from a policyholder and reuses that same signed message to execute the same operation multiple times, taking advantage of the lack of nonce incrementation.

Proof of Concept

 function createAction(
    uint8 role,
    ILlamaStrategy strategy,
    address target,
    uint256 value,
    bytes calldata data,
    string memory description
  ) external returns (uint256 actionId) {
    actionId = _createAction(msg.sender, role, strategy, target, value, data, description);
  }


 function castApproval(uint8 role, ActionInfo calldata actionInfo, string calldata reason) external {
    return _castApproval(msg.sender, role, actionInfo, reason);
  }


function castDisapproval(uint8 role, ActionInfo calldata actionInfo, string calldata reason) external {
    return _castDisapproval(msg.sender, role, actionInfo, reason);
  }



newAction.infoHash = _infoHash(actionId, policyholder, role, strategy, target, value, data);




 /// @dev Returns the hash of the createAction parameters.
  function _infoHash(
    uint256 id,
    address creator,
    uint8 creatorRole,
    ILlamaStrategy strategy,
    address target,
    uint256 value,
    bytes calldata data
  ) internal pure returns (bytes32) {
    return keccak256(abi.encodePacked(id, creator, creatorRole, strategy, target, value, data));
  }

• The _infoHash function should incorporate the nonce value during the hashing process to ensure the uniqueness of the hash for each action.

• The nonce parameter has been added to the function signature, and it is included in the keccak256 hashing process alongside the other parameters.

• When calling the _infoHash function from within the _createAction function, make sure to pass the appropriate nonce value for each action. The nonce should be incremented and updated in the nonces mapping for the respective policyholder and function selector to prevent replay attacks.

ATTACK STEPS:

Step 1: Assume a policyholder creates an action by providing parameters such as id, creator, creatorRole, strategy, target, value, and data.

Step 2: The contract generates a hash of the action parameters using the _infoHash function, which does not include the nonce.

Step 3: An attacker intercepts the hashed action parameters.

Step 4: The attacker resubmits the intercepted hash to the smart contract.

Step 5: Since the contract does not increment the nonce, it cannot differentiate between the original hash and the intercepted one.

Step 6: The smart contract processes the intercepted hash, treating it as a valid action request.

Step 7: The attacker successfully replays the action multiple times by reusing the intercepted hash, even though it was originally intended to be executed only once.

Tools Used

Manual Audit

Recommended Mitigation Steps

Use nonces mapping with the respective policyholder and function selector to prevent replay attacks. Every new createAction, castApproval ,castDisapproval actions should increment the nonces

Assessed type

Other

---

The text was updated successfully, but these errors were encountered:

All reactions