Submitted by MiloTruck
To transfer ownership of the LSP0ERC725AccountCore contract, the owner has to call transferOwnership() to nominate a pending owner. Afterwards, the pending owner must call acceptOwnership() to become the new owner.
When called by the owner, transferOwnership() executes the following logic:
LSP0ERC725AccountCore.sol#L560-L580
The currentOwner == owner() check ensures that pendingNewOwner did not call acceptOwnership() in the universalReceiver() callback. However, a malicious contract can bypass this check by doing the following in its universalReceiver() function:
This defeats the entire purpose of a two-step ownership transfer, which should ensure that the LSP0 account cannot be lost in a single call if the owner accidentally calls transferOwnership() with the wrong address.
Should transferOwnership() be called with the wrong address, the address could potentially bypass the two-step ownership transfer process to destroy the LSP0 account in a single transaction.
The following Foundry test demonstrates how an attacker can drain the LYX balance of an LSP0 account in a single transaction when set to the pending owner in transferOwnership():
Add a inTransferOwnership state variable, which ensures that acceptOwnership() cannot be called while transferOwnership() is in execution, similar to a reentrancy guard:
Call/delegatecall
CJ42 (LUKSO) confirmed
