Tigris Trade contest - Englave's results

Lines of code

https://github.com/code-423n4/2022-12-tigris/blob/main/contracts/Lock.sol#L113 https://github.com/code-423n4/2022-12-tigris/blob/main/contracts/BondNFT.sol#L351

Vulnerability details

Impact

BondNFT contract allows adding an unlimited amount of assets using addAsset function. claimGovFees is a public function, which is a part of lock, claimDebt, claim and release functions, so in case of Denial of Service vulnerability, all this functionality would be blocked. It's risky to rely on the unlimited length during iterations, as it could cause an "Out of Gas" exception.

Proof of Concept

To reproduce the issue you need to add a lot of assets, which sounds not real for production, but is still a valid issue, as it's possible to do.

   it("DoS assets", async function () {
      for (let step = 0; step < 1000; step++) {
        let contractToken = await ethers.getContractFactory("StableToken");
        let token = await contractToken.deploy("te" + step, "test" + step);
        await token.connect(owner).setMinter(owner.address, true);
        await token.connect(user).approve(Lock.address, ethers.utils.parseEther("3000"));
        await token.connect(owner).approve(Lock.address, ethers.utils.parseEther("100000000000"));
        await token.connect(owner).approve(Bond.address, ethers.utils.parseEther("100000000000"));
        await token.connect(owner).approve(GovNFT.address, ethers.utils.parseEther("100000000000"));
        await bond.connect(owner).addAsset(token.address);
      }
      
      await lock.connect(owner).claimGovFees(); //Will iterate over all 1000 assets and ran out of Gas.
    });

Tools Used

Visual inspection; Test PoC.

Recommended Mitigation Steps

The simplest solution is to define the upper limit for assets.

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

2. StableVault deposits are limited to 18 decimals During deposit and withdraw to/from StableVault contract, it mints/burns the same amount of stable tokens with respect to decimals. The current implementation supports only tokens with <= 18 decimals. It’s not possible to list a token with a bigger amount of decimals because it would cause overflow exceptions during stable token decimals calculation: 18-IERC20Mintable(_token).decimals(). Path: ./contracts/StableVault.sol : deposit()