DeFiFoundry
60,000 USDC
View results
Submission Details
Severity: low
Valid

Referral Program is exploitable by passing non-custom referralCode in `TradingAccountBranch.sol::createTradingAccount()`

Summary

When a new user/address creates an account for the FIRST time, they either put someone else referralCode or don't put any referral at all. But in this case userA can put the addressX as referralCode which doesn't have any account in the protocol, later addressX can create account and put the userA as referralCode
Eg:- userA can refer userB and userB can refer userA, which is contradictory.
Eg:- userA refers userB, userB refers userC, userC refers userA. This also contradicts.

Vulnerability Details

There are 2 ways to exploit this:-
Github:- https://github.com/Cyfrin/2024-07-zaros/blob/d687fe96bb7ace8652778797052a38763fbcbb1b/src/perpetuals/branches/TradingAccountBranch.sol#L265
1. TradingAccountBranch.sol::createTradingAccount()else condition executes when referralCode is non-custom. When user creates a new tradingAccount, the code is not checking if the account exist or not for the provided referralCode, means we can fake a referral code and the later create a new tradingAccount using the address that we provided as referralCode and pass the address of user as the referralCode.
2. userA and userB both have tradingAccount in the protocol, they creates a new tradingAccount and put each other's address as referralCode

Both path can be used to exploit the referral program.
Eg:- userA refers userB, userB refers userA.
Eg:- userA refers userB, userB refers userC, userC refers userA.
This itself is contradictory but possible in the protocol.

PoC

function test_createTradingAccountExploitReferrals() external {
// @note this test proves that A refers B and B refers A
vm.stopPrank();
address userA = address(1);
address userB = address(2);
vm.startPrank(userA);
vm.expectEmit();
emit TradingAccountBranch.LogReferralSet(userA, userB, abi.encode(userB), false);
uint128 tradingAccountIdA = perpsEngine.createTradingAccount(abi.encode(userB), false);
vm.stopPrank();
vm.startPrank(userB);
vm.expectEmit();
emit TradingAccountBranch.LogReferralSet(userB, userA, abi.encode(userA), false);
uint128 tradingAccountIdB = perpsEngine.createTradingAccount(abi.encode(userA), false);
vm.stopPrank();
}

Run this test inside https://github.com/Cyfrin/2024-07-zaros/blob/d687fe96bb7ace8652778797052a38763fbcbb1b/test/integration/perpetuals/trading-account-branch/createTradingAccount/createTradingAccount.t.sol#L10
This test runs successfully, and we can confirm userA refers userB and userB refers userA using the emit events.

Impact

This will disrupts the entire referral program as everyone will use their own referral code, making the referral program useless and less and less people will promote the protocol as they will not get any refferal rewards.

Tools Used

Manual Review

Recommendations

Make the refferals based on per account/address and not based on tradingAccount.
TradingAccountBranch.sol:-

+ struct ReferralData{
+ bool accountExist;
+ }
+ mapping(address => ReferralData) addressToReferralData;
.
.
.
function createTradingAccount(bytes memory referralCode,bool isCustomReferralCode){
.
.
.
if (referralCode.length != 0 && referral.referralCode.length == 0) {
+ if(addressToReferralData[msg.sender].accountExist == true){
+ revert("Old user cannot set referralCode");
+ }
if (isCustomReferralCode) {
.
.
.
}else{
address referrer = abi.decode(referralCode, (address));
+ if(addressToReferralData[referrer].accountExist == false){
+ revert("Referrer doesn't have a trading account");
+ }
.
.
.
}
}
+ addressToReferralData[msg.sender].accountExist = true;
}
Updates

Lead Judging Commences

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

Give us feedback!