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

3 stars 1 forks source link

Pending owner can be the wrong recipient of ownership #106

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#L624-L640

Vulnerability details

Impact

An attacker can call the acceptOwnership function with their address as the pending owner before the legitimate pending owner has a chance to call the function

Proof of Concept

The transferOwnership() function allows the current owner to set a pending owner. The acceptOwnership() function is supposed to be called by the pending owner to accept ownership. However, there are no verification in the acceptOwnership() function to verify that the caller is indeed the pending owner. This means that any address can call this function and become the owner of the contract. An attacker can exploit this vulnerability by performing the following steps:

Tools Used

Manual

Recommended Mitigation Steps

Finetune the acceptOwnership() function to verify that the caller is the pending owner before transferring ownership. This can be done by comparing the caller's address with the pending owner's address before executing the ownership transfer

Assessed type

Token-Transfer

c4-pre-sort commented 1 year ago

minhquanym marked the issue as primary issue

c4-pre-sort commented 1 year ago

minhquanym marked the issue as low quality report

minhquanym commented 1 year ago

Seems invalid

c4-sponsor commented 1 year ago

CJ42 marked the issue as sponsor disputed

CJ42 commented 1 year ago

This issue is invalid, the check is performed in the internal function _acceptOwnership().

https://github.com/lukso-network/lsp-smart-contracts/blob/32ad32f942888398bf99b039e98e238c3146c1b3/contracts/LSP14Ownable2Step/LSP14Ownable2Step.sol#L137-L140

c4-judge commented 1 year ago

trust1995 marked the issue as unsatisfactory: Invalid