Zaros Part 1
Zaros
DeFiFoundry
60,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

NFTAccount.sol Access Control limits functionality

impish

https://github.com/Cyfrin/2024-07-zaros/blob/d687fe96bb7ace8652778797052a38763fbcbb1b/src/account-nft/AccountNFT.sol#L23-L28

Description

Ownership of NFTAccount can never be transfered due to faulty access control on NFTAccount.sol

Impact

Since no EOA or external contract can accessNFTAcount.sol::_update, no trading account can transfer ownership. Should the security of any individual user's wallet be compromised externally then they would be unable to transfer ownership of their trading accounts and any accomponying positions to a new and secured wallet. Assets may not be directly risked, but protocol functionality is blocked.

Vulnerability Details

function _update(address to, uint256 tokenId, address auth) internal virtual override returns (address) {
address previousOwner = super._update(to, tokenId, auth);
IPerpsEngine(owner()).notifyAccountTransfer(to, tokenId.toUint128());
​
return previousOwner;
}

Since this function is declared to be internal and there is no public/external method within NFTAccount.sol wrapping the above, this functionality cannot be accessed by any externally owned account.

Tools Used

Manual Review

Mitigation

Simply change the internal keyword to external to reflect intended functionality. Also may want to consider changing function signature from _update to update to match normative conventions.

Updates

Lead Judging Commences

inallhonesty Lead Judge
about 1 year ago
inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.