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

Position owner can still refer himself

SBSecurity

Summary

Despite having a check to not allow the account creator to reference himself, this is still possible due to the fact that accounts are transferrable ERC721 tokens.

Vulnerability Details

It was mentioned that the main idea of referrals is for KOLs, i.e. to attract more activity by incentivizing users to refer the product, and in exchange they will receive a 1.2x boost of their points as mentioned in the Zaros documentation. The problem is that users can game the system by transferring the account to the same person who is passed as a referral. This can also be leveraged by creating multiple accounts with the same referral and transferring all of them.

Here we can see TradingAccountBranch::notifyAccountTransfer and how it doesn’t check for referrals at all:

function notifyAccountTransfer(address to, uint128 tradingAccountId) external {
_onlyTradingAccountToken();
TradingAccount.Data storage tradingAccount = TradingAccount.loadExisting(tradingAccountId);
tradingAccount.owner = to;
}

Here is a coded PoC:

forge test --match-test test_self_referral
file: LiquidateAccounts_Integration_Test
​
function test_self_referral() public {
//Scenario 1: custom referral
address initialReferrer = makeAddr("FreePoints");
changePrank({ msgSender: users.owner.account });
perpsEngine.createCustomReferralCode(initialReferrer, "free");
​
uint128 tradingAccountId = perpsEngine.createTradingAccount("free", true);
​
changePrank({ msgSender: address(tradingAccountToken) });
perpsEngine.notifyAccountTransfer(initialReferrer, tradingAccountId);
​
//Scenario 2: random referral
address accountOwner = makeAddr("accOwner");
address newReferral = makeAddr("self");
​
changePrank({ msgSender: accountOwner });
uint128 tradingAccountId2 = perpsEngine.createTradingAccount(abi.encode(newReferral), false);
​
changePrank({ msgSender: address(tradingAccountToken) });
perpsEngine.notifyAccountTransfer(newReferral, tradingAccountId2);
}

Impact

Bypassing referral restrictions that prevent trader from referring himself.

Tools Used

Manual Review

Recommendations

Add checks whether the referrer is the new admin of the positions in notifyAccountTransfer

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Referrals should be set per trading account id instead of per trader

Support

FAQs

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