Tadle
Tadle
DeFiFoundry
27,750 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

RefferalInfo feature is implemented incorerctly.

baz1ka

Relevant GitHub Links

https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/SystemConfig.sol#L41-L80

Vulnerability Details

RefferalInfo feature is implemented incorrectly:

  1. It allows everyone except referrer to change ReferrerInfo which allows other users to reduce referrerRate and increase authorityRate to get bigger funds refund.

  2. Referral feature can use only referrer which fully breaks the purpose of referral system.

Lets take a look at code in PreMarkets.createTaker() where referrer system is used:

ISystemConfig systemConfig = tadleFactory.getSystemConfig();
{
MarketPlaceInfo memory marketPlaceInfo = systemConfig
.getMarketPlaceInfo(makerInfo.marketPlace);
marketPlaceInfo.checkMarketPlaceStatus(
block.timestamp,
MarketPlaceStatus.Online
);
}
​
@> ReferralInfo memory referralInfo = systemConfig.getReferralInfo(
_msgSender()
);
​
uint256 platformFeeRate = systemConfig.getPlatformFeeRate(_msgSender());

In this line we are getting info for referrer (referrer and referral bonus). Code call SystemConfig.getReferralInfo() with _referrer argument being msg.sender which is not correct as msg.sender should be a referral. Now let's take a look where this referralInfo is used PreMarkets_updateReferralBonus():

uint256 referrerReferralBonus = platformFee.mulDiv(
referralInfo.referrerRate,
Constants.REFERRAL_RATE_DECIMAL_SCALER,
Math.Rounding.Floor
);
​
/**
* @dev update referrer referral bonus
* @dev update authority referral bonus
*/
tokenManager.addTokenBalance(
TokenBalanceType.ReferralBonus,
referralInfo.referrer,
makerInfo.tokenAddress,
referrerReferralBonus
);
​
uint256 authorityReferralBonus = platformFee.mulDiv(
referralInfo.authorityRate,
Constants.REFERRAL_RATE_DECIMAL_SCALER,
Math.Rounding.Floor
);
​
tokenManager.addTokenBalance(
TokenBalanceType.ReferralBonus,
_msgSender(),
makerInfo.tokenAddress,
authorityReferralBonus
);

As said earlier msg.sender is referrer because it gets referrer data using SystemConfig.getReferralInfo(). In the code snippet above we see updating fees for referralInfo.referrer and _msgSender() which is incorrect because as was said earlier msg.sender is also referrer. So that's how only referrer can use referrer bonus but referral can not possibly have any referral bonus.

Impact

With incorrectly implemented referrer system there would be no incentivize for referrer to invite other users to use this platform instead of others which directly reduces the profit from commissions for protocol.

Recommended Mitigation Steps

Create additional mapping to store which user is connected to which referrer and allow only referrer to change referrer info:

SystemConfigStorage:

diff --git a/src/storage/SystemConfigStorage.sol b/src/storage/SystemConfigStorage.sol
index aa48cba..a202f76 100644
--- a/src/storage/SystemConfigStorage.sol
+++ b/src/storage/SystemConfigStorage.sol
@@ -31,6 +31,9 @@ contract SystemConfigStorage is UpgradeableStorage {
/// @dev user refferral info, detail see ReferralInfo.
mapping(address => ReferralInfo) public referralInfoMap;
+ /// @dev mapping to store information about which user is connected to which referrer
+ mapping(address user => address referrer) public referrerMap;
+
/// @dev marketPlace info, detail see MarketPlaceInfo.
mapping(address => MarketPlaceInfo) public marketPlaceInfoMap;
@@ -38,5 +41,5 @@ contract SystemConfigStorage is UpgradeableStorage {
/// variables without shifting down storage in the inheritance chain.
/// See https://docs.openzeppelin.com/contracts/4.x/upgradeable#storage_gaps
/// start from slot 56, end at slot 149
- uint256[94] private __gap;
+ uint256[93] private __gap;
}

SystemConfig:

diff --git a/src/core/SystemConfig.sol b/src/core/SystemConfig.sol
index 9b7eb4d..1fbdf09 100644
--- a/src/core/SystemConfig.sol
+++ b/src/core/SystemConfig.sol
@@ -30,6 +30,16 @@ contract SystemConfig is SystemConfigStorage, Rescuable, ISystemConfig {
baseReferralRate = _baseReferralRate;
}
+ /**
+ * @dev can be called only by owner so user can not change referrer
+ */
+ function updateReferralInfo(
+ address _referrer,
+ address _referral
+ ) external onlyOwner {
+ referrerMap[_referrer] = _referral;
+ }
+
/**
* @notice Update referrer setting
* @param _referrer Referrer address
@@ -43,7 +53,7 @@ contract SystemConfig is SystemConfigStorage, Rescuable, ISystemConfig {
uint256 _referrerRate,
uint256 _authorityRate
) external {
- if (_msgSender() == _referrer) {
+ if (_msgSender() != _referrer) {
revert InvalidReferrer(_referrer);
}
@@ -225,11 +235,12 @@ contract SystemConfig is SystemConfigStorage, Rescuable, ISystemConfig {
return userPlatformFeeRate[_user];
}
- /// @dev Get referral info by referrer
+ /// @dev Get referrer info by referral
function getReferralInfo(
- address _referrer
+ address _referral
) external view returns (ReferralInfo memory) {
- return referralInfoMap[_referrer];
+ address referrer = referrerMap[_referral];
+ return referralInfoMap[referrer];
}
Updates

Lead Judging Commences

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

finding-SystemConfig-updateReferrerInfo-msgSender

Valid high severity. There are two impacts here due to the wrong setting of the `refferalInfoMap` mapping. 1. Wrong refferal info is always set, so the refferal will always be delegated to the refferer address instead of the caller 2. Anybody can arbitrarily change the referrer and referrer rate of any user, resulting in gaming of the refferal system I prefer #1500 description the most, be cause it seems to be the only issue although without a poc to fully describe all of the possible impacts

Support

FAQs

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