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

Lack of Permission Check in notifyAccountTransfer Function

Caesar

Relevant GitHub Links

https://github.com/Cyfrin/2024-07-zaros/blob/main/src/perpetuals/branches/TradingAccountBranch.sol#L405-L410

Summary

The notifyAccountTransfer function does not have a permission check to ensure that only the Account NFT contract can call it. This could potentially allow unauthorized contracts or users to call this function and update the owner of a trading account.

Vulnerability Details

The notifyAccountTransfer function lacks comprehensive access control, as it only checks if the caller is the TradingAccountToken. However, it does not ensure the integrity and authentication of the caller in other critical functions.

Impact

  • Unauthorized Ownership Transfer: Malicious actors can call this function to change the ownership of trading accounts, leading to potential account hijacking.

Tools Used

Manual

Recommendations

Add a permission check to ensure that only the Account NFT contract can call the notifyAccountTransfer function.

function notifyAccountTransfer(address to, uint128 tradingAccountId) external {
+ // Ensure that only the Account NFT contract can call this function
+ require(msg.sender == address(getTradingAccountToken()), "Only the Account NFT contract can call this function");
​
TradingAccount.Data storage tradingAccount = TradingAccount.loadExisting(tradingAccountId);
tradingAccount.owner = to;
}
Updates

Lead Judging Commences

inallhonesty Lead Judge
over 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.