code-423n4 / 2024-06-panoptic-findings

1 stars 0 forks source link

getChainName()'s implementation is somewhat broken on the Blast chain. #23

Closed howlbot-integration[bot] closed 1 month ago

howlbot-integration[bot] commented 1 month ago

Lines of code

https://github.com/code-423n4/2024-06-panoptic/blob/153f0d82440b7e63075d55b0659706531431145f/contracts/base/FactoryNFT.sol#L181-L204

Vulnerability details

Proof of Concept

Take a look at https://github.com/code-423n4/2024-06-panoptic/blob/153f0d82440b7e63075d55b0659706531431145f/contracts/base/FactoryNFT.sol#L181-L204

    function getChainName() internal view returns (string memory) {
        if (block.chainid == 1) {
            return "Ethereum Mainnet";
        } else if (block.chainid == 56) {
            return "BNB Smart Chain Mainnet";
        } else if (block.chainid == 42161) {
            return "Arbitrum One";
        } else if (block.chainid == 8453) {
            return "Base";
        } else if (block.chainid == 43114) {
            return "Avalanche C-Chain";
        } else if (block.chainid == 137) {
            return "Polygon Mainnet";
        } else if (block.chainid == 10) {
            return "OP Mainnet";
        } else if (block.chainid == 42220) {
            return "Celo Mainnet";
        } else if (block.chainid == 238) {//@audit
            return "Blast Mainnet";
        } else {
            return LibString.toString(block.chainid);
        }
    }

Evidently attempt of getting the chain name checks the current block chainId and then attaches it, it's name.

Now note that this function is used in multiple instances in protocol, from generating the SVG of the NFTs and what not, as confirmed by this search command: https://github.com/search?q=repo%3Acode-423n4%2F2024-06-panoptic%20getChainName&type=code

Problem however, is that this implementation doesn't work as expected on the blast chain due to the protocol using a wrong chain ID for blast mainnet.

Going to the official Blast docs: https://docs.blast.io/building/network-information

We can see that that the correct chainId for the blast mainnet should be 81457, see the below:

The above would mean that generating SVG info would not work as expected on the Blast mainnet unlike other chains.

Impact

Generating SVGs would be broken for the Blast mainnet.

Recommended Mitigation Steps

Consider applying these changes to https://github.com/code-423n4/2024-06-panoptic/blob/153f0d82440b7e63075d55b0659706531431145f/contracts/base/FactoryNFT.sol#L181-L204

    function getChainName() internal view returns (string memory) {
        if (block.chainid == 1) {
            return "Ethereum Mainnet";
        } else if (block.chainid == 56) {
            return "BNB Smart Chain Mainnet";
        } else if (block.chainid == 42161) {
            return "Arbitrum One";
        } else if (block.chainid == 8453) {
            return "Base";
        } else if (block.chainid == 43114) {
            return "Avalanche C-Chain";
        } else if (block.chainid == 137) {
            return "Polygon Mainnet";
        } else if (block.chainid == 10) {
            return "OP Mainnet";
        } else if (block.chainid == 42220) {
            return "Celo Mainnet";
-        } else if (block.chainid == 238) {
+        } else if (block.chainid == 81457) {
            return "Blast Mainnet";
        } else {
            return LibString.toString(block.chainid);
        }
    }

Assessed type

Context

c4-judge commented 1 month ago

Picodes marked the issue as unsatisfactory: Out of scope

Picodes commented 1 month ago

Out of scope per the scoping precisions of the readme