The long open interest may exceed the max open interest limit

[code423n4]

Lines of code

https://github.com/code-423n4/2022-12-tigris/blob/main/contracts/PairsContract.sol#L142

Vulnerability details

Impact

In Contract PairsContract, when updating the max open interest limit _idToOi[_asset][_tigAsset].maxOi, the contract did not check the long open interests of existing assets. This may result in the fact that some long open interests of existing assets will be higher than the allowed max open interest limit. This confuses the market.

Proof of Concept

Put the below test under describe('Protocol-only functions', function () { in test file: 02.PairsContract.js.

	it('Succeed if reducing max open interest limit to values lower than existing long open interest', async function () {
		await pairscontract.connect(owner).addAsset(99, 'XYZ/ABC', ethers.constants.AddressZero, ethers.utils.parseEther('1'), ethers.utils.parseEther('100'), 1e10, 3e9);
		await pairscontract.connect(owner).setProtocol(NewTrading.address);
		await pairscontract.connect(owner).setMaxOi(99, StableToken.address, ethers.utils.parseEther('2000')); //@audit - set initial max open interest limit
		await pairscontract.connect(NewTrading).modifyLongOi(99, StableToken.address, true, ethers.utils.parseEther('500'));  //@audit - set long open interest of asset
		let [longOi, shortOi] = await pairscontract.idToOi(99, StableToken.address);
		expect(longOi).to.equal(ethers.utils.parseEther('500'));
		await expect(pairscontract.connect(owner).setMaxOi(99, StableToken.address, ethers.utils.parseEther('1'))).to.be.not.reverted; //@audit - update max open interest limit to a level lower than exist long open interest
	});

Tools Used

Manual audit.

Recommended Mitigation Steps

Set the max open interest limit as constant/immutable, or adopt some strategies that can keep consistency between existing long open interests and the max open interest limit.

[c4-judge]

GalloDaSballo marked the issue as duplicate of #370

[c4-judge]

GalloDaSballo marked the issue as duplicate of #377

[c4-judge]

GalloDaSballo changed the severity to 2 (Med Risk)

[c4-judge]

GalloDaSballo marked the issue as satisfactory