sherlock-audit / 2024-01-telcoin-judging

6 stars 5 forks source link

Krace - Minting new tokens becomes permanently impossible unless the last NFT is burned #134

Closed sherlock-admin closed 9 months ago

sherlock-admin commented 9 months ago

Krace

high

Minting new tokens becomes permanently impossible unless the last NFT is burned

Summary

The mint function uses the totalSupply to mint the next NFT, however, if not the last NFT is burned, the totalSupply will be equal to the Id of the last NFT, causing a revert in mint.

Vulnerability Detail

The mint function uses the totalSupply to mint the next NFT, but the totalSupply can be decreased by burn, mint may mints existed NFT and reverts.

    function mint(
        address newMember
    ) external onlyRole(GOVERNANCE_COUNCIL_ROLE) {
        if (totalSupply() != 0) {
            _retrieve();
        }

        balances.push(0);
        _mint(newMember, totalSupply());
    }

For example, if two NFTs [0,1] are in the contract, and the NFT0 is burned, you cannot mint new NFTs. Add the test to telcoin-audit/test/sablier/CouncilMember.test.ts and run it with npx hardhat test.

diff --git a/telcoin-audit/test/sablier/CouncilMember.test.ts b/telcoin-audit/test/sablier/CouncilMember.test.ts
index 675b89d..c25cc62 100644
--- a/telcoin-audit/test/sablier/CouncilMember.test.ts
+++ b/telcoin-audit/test/sablier/CouncilMember.test.ts
@@ -232,6 +232,18 @@ describe("CouncilMember", () => {
                 // // mint(2) => 0 TEL
                 expect(await councilMember.balances(2)).to.equal(0);
             });
+            it("burn and mint", async () => {
+                await expect(councilMember.mint(member.address)).to.not.reverted;
+                await expect(councilMember.mint(member.address)).to.not.reverted;
+                const y = await councilMember.totalSupply();
+                console.log("total supply ", y);
+                await expect(councilMember.burn(0, member.address)).to.not.reverted;
+                //await expect(councilMember.burn(1, support.address)).to.not.reverted;
+                const x = await councilMember.totalSupply();
+                console.log("total supply ", x);
+                await (councilMember.mint(support.address));
+                //await expect(councilMember.burn(2, support.address)).to.revertedWith("CouncilMember: must maintain council");
+            });
         });

         describe("burn", () => {

Impact

Minting new tokens becomes permanently impossible unless the last NFT is burned.

Code Snippet

https://github.com/sherlock-audit/2024-01-telcoin/blob/0954297f4fefac82d45a79c73f3a4b8eb25f10e9/telcoin-audit/contracts/sablier/core/CouncilMember.sol#L173-L182

Tool used

hardhat

Recommendation

It's recommended to use a self-increasing counter to calculate the next NFT's ID.

Duplicate of #199

sherlock-admin2 commented 9 months ago

1 comment(s) were left on this issue during the judging contest.

takarez commented:

valid because { also a dupp of 109 with same underlying cause and minimal impact}