Tigris Trade contest - chaduke's results

Lines of code

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

Vulnerability details

Impact

Detailed description of the impact of this finding. _checkDelay is not properly implemented due to failure to check the open case in the following line:

if (_delay.actionType == _type) {

As a result, the purpose of profit-taking in the same or close blocks cannot be prevented as the documentation requires "This is to prevent profitable opening and closing in the same tx with two different prices in the "valid signature pool""

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept. https://github.com/code-423n4/2022-12-tigris/blob/588c84b7bb354d20cbca6034544c4faa46e6a80e/contracts/Trading.sol#L857-L868

Tools Used

Remix

Recommended Mitigation Steps

This is probabally a typo the fix is easy as follows:

function _checkDelay(uint _id, bool _type) internal {
        unchecked {
            Delay memory _delay = blockDelayPassed[_id];
            if (_type) { // @audit< this the open case
                blockDelayPassed[_id].delay = block.number + blockDelay;
            } else {         // @audit this is the close case
                if (block.number < _delay.delay) revert("0"); //Wait
                blockDelayPassed[_id].delay = block.number + blockDelay;
                blockDelayPassed[_id].actionType = _type;
            }
        }
    }

Lines of code

https://github.com/code-423n4/2022-12-tigris/blob/496e1974ee3838be8759e7b4096dbee1b8795593/contracts/Trading.sol#L734-L738

Vulnerability details

Impact

Detailed description of the impact of this finding. The calculation of _feePaid is not correct, it does not reflect the referral fee.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept. https://github.com/code-423n4/2022-12-tigris/blob/496e1974ee3838be8759e7b4096dbee1b8795593/contracts/Trading.sol#L734-L738

Tools Used

Remix

Recommended Mitigation Steps

The corrrect calculation is:

_feePaid =
                _positionSize
                * (_fees.referralFees + _fees.botFees) // get total fee%
                / DIVISION_CONSTANT // divide by 100%
                + _daoFeesPaid;

Lines of code

https://github.com/code-423n4/2022-12-tigris/blob/588c84b7bb354d20cbca6034544c4faa46e6a80e/contracts/PairsContract.sol#L33-L37

Vulnerability details

Impact

Detailed description of the impact of this finding. A compromised or malicious owner of PairsContract can manipulate chainlinkFeed price

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept. A compromised or malicious owner of PairsContract can manipulate chainlinkFeed price as follows:

1. call setAssetChainlinkFeed(uint256 _asset, address _MalicousFeed), now the _maliciousFeed becomes the fake chainlinkFeed
2. false price can be provided by the _maliciousFeed and used in the advantage of the attacker to gain profit in trading

Tools Used

Remix

Recommended Mitigation Steps

The owner should be not allowed to change the chainlinkFeed, all chainlinkFeeds should be defined as constants and explicit. Use proxy pattern to include new feeds when necessary.

Judge has assessed an item in Issue #50 as M risk. The relevant finding follows:

QA10. https://github.com/code-423n4/2022-12-tigris/blob/588c84b7bb354d20cbca6034544c4faa46e6a80e/contracts/StableVault.sol#L44 The deposit() function only works for tokens that have no more than 18 decimals. This needs to be documented.