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

Protocol does not reset `nextAccountId` when setting new trading account token address

ke1cam

Summary

When setting new tradingAccountToken, using setTradingAccountToken function, internal nextAccountId is not reset.

Root Cause

https://github.com/Cyfrin/2024-07-zaros/blob/d687fe96bb7ace8652778797052a38763fbcbb1b/src/perpetuals/branches/GlobalConfigurationBranch.sol#L179-L188

function setTradingAccountToken(address tradingAccountToken) external onlyOwner {
if (tradingAccountToken == address(0)) {
revert Errors.TradingAccountTokenNotDefined();
}
​
GlobalConfiguration.Data storage globalConfiguration = GlobalConfiguration.load();
globalConfiguration.tradingAccountToken = tradingAccountToken;
​
emit LogSetTradingAccountToken(msg.sender, tradingAccountToken);
}

Vulnerability details

setTradingAccountToken is used to set new trading account token address. TokenId is expected to start from tokenId = 1.

function createTradingAccount(
bytes memory referralCode,
bool isCustomReferralCode
)
public
virtual
returns (uint128 tradingAccountId)
{
// fetch storage slot for global config
GlobalConfiguration.Data storage globalConfiguration = GlobalConfiguration.load();
​
// increment next account id & output
tradingAccountId = ++globalConfiguration.nextAccountId;
​
...
​
}

Proof of Concept

Not reseting nextAccountId can lead to some issues in the future. Consider this example scenario. Well funded attacker (for example other perpetual protocol) could try to mint total supply of the trading account token. In this scenario protocol customers are not able to access the perpetual markets. Protocol decides to change the trading account token address but users still can't mint new token because internal nextAccountId was not reset. While trying to mint new token it will revert as maximum tokenId has been reached even though no NFT were minted.

Impact

Protocol can be bricked.

Recommended Mitigation Steps

Reset nextAccountId when setting new trading account token address.

Example pseduocode:

function setTradingAccountToken(address tradingAccountToken) external onlyOwner {
if (tradingAccountToken == address(0)) {
revert Errors.TradingAccountTokenNotDefined();
}
​
GlobalConfiguration.Data storage globalConfiguration = GlobalConfiguration.load();
globalConfiguration.tradingAccountToken = tradingAccountToken;
​
+ globalConfiguration.nextAccountId = 0;
​
emit LogSetTradingAccountToken(msg.sender, tradingAccountToken);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge
about 1 year ago
inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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