CouncilMember:burn renders the contract inoperable after the first execution
The CouncilMember contract suffers from a critical vulnerability that misaligns the balances array after a successful burn, rendering the contract inoperable.\\nThe root cause of the vulnerability is that the `burn` function incorrectly manages the `balances` array, shortening it by one each time an ERC721 token is burned while the latest minted NFT still withholds its unique `tokenId` which maps to the previous value of `balances.length`.\\n```\\n// File: telcoin-audit/contracts/sablier/core/CouncilMember.sol\\n   function burn(\\n        // rest of code\\n       balances.pop(); // <= FOUND: balances.length decreases, while latest minted nft withold its unique tokenId\\n       _burn(tokenId);\\n   }\\n```\\n\\nThis misalignment between existing `tokenIds` and the `balances` array results in several critical impacts:\\nHolders with `tokenId` greater than the length of balances cannot claim.\\nSubsequent burns of `tokenId` greater than balances length will revert.\\nSubsequent mint operations will revert due to `tokenId` collision. As `totalSupply` now collides with the existing `tokenId`.\\n```\\n// File: telcoin-audit/contracts/sablier/core/CouncilMember.sol\\n   function mint(\\n        // rest of code\\n       balances.push(0);\\n       _mint(newMember, totalSupply());// <= FOUND\\n   }\\n```\\n\\nThis mismanagement creates a cascading effect, collectively rendering the contract inoperable. Following POC will demonstrate the issue more clearly in codes.\\nPOC\\nRun `git apply` on the following patch then run `npx hardhat test` to run the POC.\\n```\\ndiff --git a/telcoin-audit/test/sablier/CouncilMember.test.ts b/telcoin-audit/test/sablier/CouncilMember.test.ts\\nindex 675b89d..ab96b08 100644\\n--- a/telcoin-audit/test/sablier/CouncilMember.test.ts\\n+++ b/telcoin-audit/test/sablier/CouncilMember.test.ts\\n@@ -1,13 +1,14 @@\\n import { expect } from "chai";\\n import { ethers } from "hardhat";\\n import { SignerWithAddress } from "@nomicfoundation/hardhat-ethers/signers";\\n-import { CouncilMember, TestTelcoin, TestStream } from "../../typechain-types";\\n+import { CouncilMember, TestTelcoin, TestStream, ERC721Upgradeable__factory } from "../../typechain-types";\\n \\n describe("CouncilMember", () => {\\n     let admin: SignerWithAddress;\\n     let support: SignerWithAddress;\\n     let member: SignerWithAddress;\\n     let holder: SignerWithAddress;\\n+    let lastCouncilMember: SignerWithAddress;\\n     let councilMember: CouncilMember;\\n     let telcoin: TestTelcoin;\\n     let stream: TestStream;\\n@@ -18,7 +19,7 @@ describe("CouncilMember", () => {\\n     let supportRole: string = ethers.keccak256(ethers.toUtf8Bytes("SUPPORT_ROLE"));\\n \\n     beforeEach(async () => {\\n-        [admin, support, member, holder, target] = await ethers.getSigners();\\n+        [admin, support, member, holder, target, lastCouncilMember] = await ethers.getSigners();\\n \\n         const TestTelcoinFactory = await ethers.getContractFactory("TestTelcoin", admin);\\n         telcoin = await TestTelcoinFactory.deploy(admin.address);\\n@@ -182,6 +183,22 @@ describe("CouncilMember", () => {\\n                 it("the correct removal is made", async () => {\\n                     await expect(councilMember.burn(1, support.address)).emit(councilMember, "Transfer");\\n                 });\\n+                it.only("inoperable contract after burn", async () => {\\n+                    await expect(councilMember.mint(lastCouncilMember.address)).to.not.reverted;\\n+\\n+                    // This 1st burn will cause contract inoperable due to tokenId & balances misalignment\\n+                    await expect(councilMember.burn(1, support.address)).emit(councilMember, "Transfer");\\n+\\n+                    // Impact 1. holder with tokenId > balances length cannot claim\\n+                    await expect(councilMember.connect(lastCouncilMember).claim(3, 1)).to.revertedWithPanic("0x32"); // @audit-info 0x32: Array accessed at an out-of-bounds or negative index\\n+\\n+                    // Impact 2. subsequent burns of tokenId > balances length will revert\\n+                    await expect(councilMember.burn(3, lastCouncilMember.address)).to.revertedWithPanic("0x32"); \\n+\\n+                    // Impact 3. subsequent mint will revert due to tokenId collision\\n+                    await expect(councilMember.mint(lastCouncilMember.address)).to.revertedWithCustomError(councilMember, "ERC721InvalidSender");\\n+\\n+                });\\n             });\\n         });\\n \\n```\\n\\nResult\\nCouncilMember mutative burn Success ✔ inoperable contract after burn (90ms) 1 passing (888ms)\\nThe Passing execution of the POC confirmed that operations such as `claim`, `burn` & `mint` were all reverted which make the contract inoperable.
It is recommended to avoid popping out balances to keep alignment with uniquely minted tokenId. Alternatively, consider migrating to ERC1155, which inherently manages a built-in balance for each NFT.
The severity of the vulnerability is high due to the high likelihood of occurence and the critical impacts on the contract's operability and token holders' ability to interact with their assets.
```\\n// File: telcoin-audit/contracts/sablier/core/CouncilMember.sol\\n   function burn(\\n        // rest of code\\n       balances.pop(); // <= FOUND: balances.length decreases, while latest minted nft withold its unique tokenId\\n       _burn(tokenId);\\n   }\\n```\\n