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

Zaros receives less fees due to a precision loss in convertUd60x18ToTokenAmount method when doing a liquidation or filing an order.

nfmelendez

Summary

Zaros is getting less fees than expected because of a precision loss against the protocol in MarginCollateralConfiguration::convertUd60x18ToTokenAmount when decimals are less than Constants.SYSTEM_DECIMALS.

Vulnerability Details

The deducting fees using TradingAccount::deductAccountMargin, Zaros uses prb math have a lot of precision but at the moment of sending the fees to the recipient it converts requiredMarginInCollateralX18(type UD60x18) to amountToTransfer of type uint256 to execute then the safeTransfer. However in the process the precision loss is againts the Zaros therefore it recives less fees than expected.

Proof of concept

  1. add fs_permissions = [{ access = "read-write", path = "./"}] to foundry.toml so vm can write to a file the output of the fuzz because console2.log doesn't work.

  2. add import "forge-std/Vm.sol"; to TradingAccount.sol line 22

  3. add TradingAccount.sol line 157 add a script that write the output of the fuzz to a file

​
// Fetch the vm of foundry
Vm vm = Vm(address(uint160(uint256(keccak256("hevm cheat code")))));
// increase decimals so it can substract with same precision
UD60x18 a = ud60x18(amountToTransfer * (10 **(18 - marginCollateralConfiguration.decimals)));
// If substraction is positive means there was a precision loss againts Zaros
if(requiredMarginInCollateralX18.sub(a).gt(ud60x18(0))) {
// is the file to print
string memory path = "precisionLoss.txt";
address c = collateralType;
// print the collateral address
vm.writeLine(path, vm.toString(c));
vm.writeLine(path, "HIGH PRESICION marginCollateralBalanceX18 is");
vm.writeLine(path, vm.toString(requiredMarginInCollateralX18.unwrap()));
vm.writeLine(path, "LOW PRESICION");
vm.writeLine(path, vm.toString(a.unwrap()));
vm.writeLine(path, "ZAROS LOSS:");
vm.writeLine(path, vm.toString(requiredMarginInCollateralX18.sub(a).unwrap()));
}

  1. Execute forge test --mt="testFuzz_WhenThereIsNotCollateralLiquidationPriority"

The result will be:

Output:

- collateral = 0x059C5B023B577e82C346721653420711bFbdE3f9
- HIGH PRESICION marginCollateralBalanceX18 is 4999999950001964
- LOW PRESICION amountToTransfer is 4999000000000000
- Zaros loss is 999950001964

Initial parameters:

- marginValueUsd=106623011085010260500099094077633627624735592532786174215613957951654853776537
- feeAmount=1963
- randomFeeAmount1=1263969325
- randomFeeAmount2=4948

Lines of code

https://github.com/Cyfrin/2024-07-zaros/blob/main/src/perpetuals/leaves/MarginCollateralConfiguration.sol#L57-L62

Impact

Zaros deduct 2 diffent fees and a pnl reduction when liquidate and then deduct another 2 different fees when fill an order so the precision loss will be happening all the time.
liquidation deductions are settlementFee, orderFee and pnlUsdX18 and filling an order are orderFee and settlementFee

Tools Used

  • VS Code

Recommendations

It is a standard in the industry to charge fees with the precision in favor of the protocol for accounting, to avoid free operations with dust transactions and avoid an attack vector that can generate loss of funds when having free operations and user incentives at the same time. See Exactly case M-3: Theft of unassigned earnings from a fixed pool

Updates

Lead Judging Commences

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

convertUd60x18ToTokenAmount heavily truncates

Support

FAQs

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