Lines of code

Vulnerability details

Impact

Success value not checked. Result can fail silently. Msg.value can be lost.

Proof of Concept

Function execute does not check the validity of success. If execute is called and msg.value is greater than value, then excess msg.value will be stucked in contract.

LlamaExecutor.sol

  function execute(address target, uint256 value, bool isScript, bytes calldata data)
    external
    returns (bool success, bytes memory result)
  {
    if (msg.sender != LLAMA_CORE) revert OnlyLlamaCore();
    (success, result) = isScript ? target.delegatecall(data) : target.call{value: value}(data);
  }

Tools Used

Manual Reivew

Recommended Mitigation Steps

Check msg.value and success result. Otherwise, change function visibility to internal since execute is used mainly in other contracts.

  function execute(address target, uint256 value, bool isScript, bytes calldata data)
>   external
    returns (bool success, bytes memory result)
  {
+   if (msg.value != value) revert IncorrectMsgValue();
    if (msg.sender != LLAMA_CORE) revert OnlyLlamaCore();
    (success, result) = isScript ? target.delegatecall(data) : target.call{value: value}(data);
  }
+   if(!success) revert FailedActionExecution(result)

Assessed type

call/delegatecall

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

All reactions