Llama - MiniGlome's results

Lines of code

https://github.com/code-423n4/2023-06-llama/blob/aac904d31639c1b4b4e97f1c76b9c0f40b8e5cee/src/LlamaCore.sol#L334 https://github.com/code-423n4/2023-06-llama/blob/aac904d31639c1b4b4e97f1c76b9c0f40b8e5cee/src/LlamaExecutor.sol#L34

Vulnerability details

Description

When the executeAction calls the executor the msg.value is not pass to the executor contract. Resulting in the executor not having the founds needed by the action. The execute function of the LlamaExecutor is not even payable but will try to send value to the target, which will always fail if value > 0 because there is no ETH in the smart contract.

File: LlamaExecutor.sol

L34: (success, result) = isScript ? target.delegatecall(data) : target.call{value: value}(data);

Proof of concept

Here is a foundry test to reproduce the bug

function test_ExecuteWithMsgValue() public {
    uint256 value = 3 ether;
    bytes memory data = abi.encodeCall(MockProtocol.receiveEth, ());
    vm.prank(actionCreatorAaron);
    uint256 actionId =
      mpCore.createAction(uint8(Roles.ActionCreator), mpStrategy1, address(mockProtocol), value, data, "");
    ActionInfo memory _actionInfo = ActionInfo(
      actionId, actionCreatorAaron, uint8(Roles.ActionCreator), mpStrategy1, address(mockProtocol), value, data
    );

    vm.warp(block.timestamp + 1);

    _approveAction(approverAdam, _actionInfo);
    _approveAction(approverAlicia, _actionInfo);

    vm.warp(block.timestamp + 6 days);

    mpCore.queueAction(_actionInfo);

    vm.warp(block.timestamp + 5 days);

    vm.deal(actionCreatorAaron, value);

    vm.prank(actionCreatorAaron);
    (bool status, bytes memory _data) =
      address(mpCore).call{value: value}((abi.encodeCall(mpCore.executeAction, (_actionInfo))));
    assertTrue(status); // Call revert here but it should not
  }

Recommendation

The execute function should be made payable and the executeAction should call execute with {value: msg.value}

L334: executor.execute{value: msg.value}(actionInfo.target, actionInfo.value, action.isScript, actionInfo.data);

Assessed type

call/delegatecall