_handleOpenFees returns an incorrect value for _feePaid. This directly impacts margin calculations

[code423n4]

Lines of code

https://github.com/code-423n4/2022-12-tigris/blob/588c84b7bb354d20cbca6034544c4faa46e6a80e/contracts/Trading.sol#L178
https://github.com/code-423n4/2022-12-tigris/blob/588c84b7bb354d20cbca6034544c4faa46e6a80e/contracts/Trading.sol#L734

Vulnerability details

Impact

Formula for fee paid in Line 734 is incorrect leading to incorrect margin calculations. Since this directly impacts the trader margin and associated fee calculations, I've marked as HIGH risk

On initiating a market order, Margin is adjusted for the fees that is charged by protocol. This adjustment is in Line 178 of Trading. Fees computed by _handleOpenFees is deducted from Initial margin posted by user.

formula misses to account the 2*referralFee component while calculaing _feePaid

Proof of Concept

Note that _feePaid as per formula in Line 734 is the sum of _daoFeesPaid', and sum of burnerFee&botFee. _daoFeesPaidis calculated from_fees.daoFeeswhich itself is calculated by subtracting2*referralFeeandbotFee`.

So when we add back burnerFee and botFee to _feePaid, we are missing to add back the 2*referralFee which was earlier excluded when calculating _daoFeesPaid. While botFee is added back correctly, same adjustment is not being done viz-a-viz referral fee.

This results in under calculating the _feePaid and impacts the rewards paid to the protocol NFT holders.

Tools Used

Recommended Mitigation Steps

Suggest replacing the formula in line 734 with below (adding back _fees.referralFees*2)

            _feePaid =
                _positionSize
                * (_fees.burnFees + _fees.botFees + _fees.referralFees*2 ) 
                / DIVISION_CONSTANT // divide by 100%
                + _daoFeesPaid;

[c4-judge]

GalloDaSballo marked the issue as duplicate of #476

[c4-judge]

GalloDaSballo changed the severity to 2 (Med Risk)

[c4-judge]

GalloDaSballo marked the issue as satisfactory

[c4-judge]

GalloDaSballo marked the issue as not a duplicate

[c4-judge]

GalloDaSballo marked the issue as primary issue

[c4-judge]

GalloDaSballo marked the issue as selected for report

[GalloDaSballo]

The warden has shown a mistake in how fees are calculated, the impact will cause a loss of yield to the protocol, however no convincing argument was made as to how this can cause a loss to depositors or users (loss of principal), for this reason, I believe Medium Severity to be the most appropriate

[GainsGoblin]

Mitigation: code-423n4/2022-12-tigris#2 (comment)

[c4-sponsor]

GainsGoblin marked the issue as sponsor confirmed