Tigris Trade contest - 0Kage's results

Lines of code

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

Vulnerability details

Impact

'addToPosition' function adds additional margin to an existing trade. Margin value sent to '_handleDeposit' function sends margin adjusted for 'fees' instead of sending full margin. This results in a lesser margin withdrawn from traders account - since this directly impacts the margin amount deposited in stable vault, I have assigned 'HiGH' risk to this finding

In Line 275, margin value sent to '_handleDeposit' is '_add Margin -_fee' instead of '_addMargin'.

In Line 651, note that the 'margin' amount is transfered from trader to this contract. Since we sent a 'margin adjusted for fees', a lesser margin will be deducted from trader account.

This messes with the fees calculations as tgUSD is minted to govNFT holders with no collateral backing the tokens. On a cumulative basis, when traders add Margin, this shortfall keeps growing

Proof of Concept

Bob is a trader with an existing position on ETH/USDT pair. Bob adds an additional 100k USDT as margin. At a 0.1% mint fee, Bob will have a fees of 100$. Current contract is only sending 99,900 USDT margin amount to '_handleDeposit' which subsequently withdraws this amount from trader account.

100$ that was supposed to be rewarded to Gov NFT holders never made it to the protocol. Essentially the tgUSD minted to referrer/ bots is unbacked because of this shortfall

Tools Used

Recommended Mitigation Steps

Strangely in Line 180 , when a new market order is created, logic is correctly handled. '_tradeinfo.margin' is sent to '_handleDeposit' instead of '_marginAfterfees'. However exact same logic in 'addPosition' is not followed.

I recommend to pass '_addMargin' instead of '_addMargin -_fee' to the 'handleDeposit' function in line 275 for correct accounting

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;