No validation check for whether the msg.value is not less than the value parameter in the execute function

[code423n4]

Lines of code

https://github.com/code-423n4/2023-06-lukso/blob/9dbc96410b3052fc0fd9d423249d1fa42958cae8/contracts/LSP0ERC725Account/LSP0ERC725AccountCore.sol#L228

Vulnerability details

Impact

Users can send msg.value lesser than the supplied value parameter leading to incorrect accounting.

Proof of Concept

There is no check for whether msg.value >= value in the execute function of the LSP0ERC725AccountCore contract. LSP0ERC725Account is in scope and inherits from LSP0ERC725AccountCore.

function execute(
        uint256 operationType,
        address target,
        uint256 value,
        bytes memory data
    ) public payable virtual override returns (bytes memory) {
        if (msg.value != 0) {
            emit ValueReceived(msg.sender, msg.value);
        }
//@audit no validation to check if msg.value >=value.
        address _owner = owner();

        // If the caller is the owner perform execute directly
        if (msg.sender == _owner) {
            return ERC725XCore._execute(operationType, target, value, data);
        }

        // If the caller is not the owner, call {lsp20VerifyCall} on the owner
        // Depending on the magicValue returned, a second call is done after execution
        bool verifyAfter = LSP20CallVerification._verifyCall(_owner);

        // Perform the execution
        bytes memory result = ERC725XCore._execute(
            operationType,
            target,
            value,
            data
        );

...

}

Also, the executeBatch of the same LSP0ERC725AccountCore did not validate that the msg.value is not less than the values parameter. This can be fixed by the way it is validated in the executeBatch function of the LSP6KeyManagerCore.sol contract.

Tools Used

Manual Review

Recommended Mitigation Steps

Add validation to check that msg.value >= value in the execute function.

Assessed type

ETH-Transfer

[c4-pre-sort]

minhquanym marked the issue as low quality report

[c4-pre-sort]

minhquanym marked the issue as primary issue

[minhquanym]

Seems invalid as LSP0ERC725Account can receive and hold ETH

[c4-judge]

trust1995 marked the issue as unsatisfactory: Invalid