code-423n4 / 2023-06-lukso-findings

3 stars 1 forks source link

All Ether sent to LSP0ERC725Account will be permanently locked #110

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

All Ether sent to LSP0ERC725Account will be permanently locked because it inherits the receive function from the LSP0ERC725AccountCore contract but does not have a withdraw function.

Proof of Concept

All Ether sent to LSP0ERC725Account will be permanently locked because it inherits the receive function from the LSP0ERC725AccountCore contract but does not have a withdraw function.

contract LSP0ERC725Account is LSP0ERC725AccountCore {
abstract contract LSP0ERC725AccountCore is
    ERC725XCore,
    ERC725YCore,
    IERC1271,
    ILSP0ERC725Account,
    ILSP1UniversalReceiver,
    LSP14Ownable2Step,
    LSP17Extendable,
    LSP20CallVerification
{
...//ommitted code
receive() external payable virtual {//@audit can receive ether but no withdraw.
        if (msg.value != 0) {
            emit ValueReceived(msg.sender, msg.value);
        }
    }
...//omitted code

Tools Used

Manual Review

Recommended Mitigation Steps

Consider adding a withdraw function to the LSP0ERC725Account contract or the LSP0ERC725AccountCore contract.

Assessed type

ETH-Transfer

c4-pre-sort commented 1 year ago

minhquanym marked the issue as low quality report

c4-pre-sort commented 1 year ago

minhquanym marked the issue as primary issue

minhquanym commented 1 year ago

Seems invalid. receive() reverts when msg.value > 0

c4-judge commented 1 year ago

trust1995 marked the issue as unsatisfactory: Invalid