Submission Details
Severity:
Missing Fee Normalization Leads to 1e18x Fee Overcharge in UpliftOnlyExample
Summary
The UpliftOnlyExample contract contains a critical fee calculation error where fees are overcharged by a factor of 1e18 due to missing decimal normalization when calculating fees with feePerLP
Vulnerability Details
// In UpliftOnlyExample.sol
// Branch 1: When amount <= amountLeft
if (feeDataArray[i].amount <= localData.amountLeft) {
uint256 depositAmount = feeDataArray[i].amount;
localData.feeAmount += (depositAmount * feePerLP); // @audit missing /1e18
// ... rest of the code
}
// Branch 2: When amount > amountLeft
else {
localData.feeAmount += (feePerLP * localData.amountLeft); // @audit missing /1e18
// ... rest of the code
}
uint256 feePerLP;
// if the pool has increased in value since the deposit, the fee is calculated based on the deposit value
if (localData.lpTokenDepositValueChange > 0) {
feePerLP =
(uint256(localData.lpTokenDepositValueChange) * (uint256(feeDataArray[i].upliftFeeBps) * 1e18)) /
10000;
}
// if the pool has decreased in value since the deposit, the fee is calculated based on the base value - see wp
else {
//in most cases this should be a normal swap fee amount.
//there always myst be at least the swap fee amount to avoid deposit/withdraw attack surgace.
@> feePerLP = (uint256(minWithdrawalFeeBps) * 1e18) / 10000;
}
// if the deposit is less than the amount left to burn, burn the whole deposit and move on to the next
if (feeDataArray[i].amount <= localData.amountLeft) {
uint256 depositAmount = feeDataArray[i].amount;
@> localData.feeAmount += (depositAmount * feePerLP);
localData.amountLeft -= feeDataArray[i].amount;
lpNFT.burn(feeDataArray[i].tokenID);
delete feeDataArray[i];
feeDataArray.pop();
if (localData.amountLeft == 0) {
break;
}
} else {
feeDataArray[i].amount -= localData.amountLeft;
@> localData.feeAmount += (feePerLP * localData.amountLeft);
break;
}
}
Coded Scenario POC
// Given values:
feePerLP = 0.1e18; // 10% fee (100000000000000000)
amount = 1000 tokens;
amountLeft = 2000 tokens;
// Case 1: amount <= amountLeft (1000 <= 2000)
depositAmount = 1000;
fee = depositAmount * feePerLP
= 1000 * 100000000000000000
= 100000000000000000000 // @audit 100e18 tokens instead of 100 tokens
// Case 2: amount > amountLeft
amountLeft = 500;
fee = feePerLP * amountLeft
= 100000000000000000 * 500
= 50000000000000000000 // @audit 50e18 tokens instead of 50 tokens
// Correct calculations should be:
correct_fee1 = (depositAmount * feePerLP) / 1e18
= (1000 * 100000000000000000) / 1000000000000000000
= 100 tokens // 10% of 1000
correct_fee2 = (feePerLP * amountLeft) / 1e18
= (100000000000000000 * 500) / 1000000000000000000
= 50 tokens // 10% of 500
Impact
If transactions succeed, users lose significantly more funds than intended
Tools Used
Chisel, Github
Recommendations
// Fix 1: Add 1e18 division in first branch
if (feeDataArray[i].amount <= localData.amountLeft) {
uint256 depositAmount = feeDataArray[i].amount;
localData.feeAmount += (depositAmount * feePerLP) / 1e18; // @audit-ok added /1e18
// ... rest of the code
}
// Fix 2: Add 1e18 division in else branch
else {
localData.feeAmount += (feePerLP * localData.amountLeft) / 1e18; // @audit-ok added /1e18
// ... rest of the code
}
Updates
Lead Judging Commences
n0kto about 1 year ago
Submission Judgement Published Assigned finding tags:
finding_Uplift_feePerLP_overscale_1e18_when_benefit
Likelihood: High, when benefit. Impact: High, exitFee will be too high, loss of funds or DoS removal.
Prize pool breakdown
Total prize
49,600 OP
nSLOC
3,00017
OP / LOC
44,640
4,960
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.