Minting is not possible when a property has no items

[code423n4]

Lines of code

https://github.com/code-423n4/2022-09-nouns-builder/blob/main/src/token/metadata/MetadataRenderer.sol#L194

Vulnerability details

Impact

If a property without items was added, minting becomes impossible. To enable minting again, an item must be added to the property, which is only possible through a new governance proposal.

Proof of Concept

Consider the following test case:

function testRevert_MintingWithPropertyWithoutItems() public {
    // Deploying a token and a metadata renderer
    address tknImpl = address(new Token(address(this)));
    Token tkn = Token(address(new ERC1967Proxy(tknImpl, "")));

    address rndrImpl = address(new MetadataRenderer(address(this)));
    MetadataRenderer rndr = MetadataRenderer(address(new ERC1967Proxy(rndrImpl, "")));

    bytes memory initString = abi.encode("Test", "Test", "Test", "Test", "Test");
    IManager.FounderParams[] memory founders = new IManager.FounderParams[](1);
    founders[0] = IManager.FounderParams({ wallet: address(this), ownershipPct: 1, vestExpiry: 4 weeks });

    tkn.initialize(founders, initString, address(rndr), address(this));
    rndr.initialize(initString, address(tkn), address(this), address(this));

    // Exploit starts here:

    // Creating a property without items and adding it to the token.
    string[] memory names = new string[](1);
    MetadataRendererTypesV1.ItemParam[] memory items = new MetadataRendererTypesV1.ItemParam[](0);
    MetadataRendererTypesV1.IPFSGroup memory ipfsGroup = MetadataRendererTypesV1.IPFSGroup({ baseUri: "base", extension: "ext" });

    names[0] = "uh-oh";
    // Founder forgot to add items.

    rndr.addProperties(names, items, ipfsGroup);
    assertEq(rndr.propertiesCount(), 1);

    // Minting a token with a property without items and getting an error.
    vm.expectRevert(stdError.divisionError);
    tkn.mint();

    // Adding an item to the property. At this point, this can only be done through a governance proposal.
    names = new string[](0);
    items = new MetadataRendererTypesV1.ItemParam[](1);
    items[0] = MetadataRendererTypesV1.ItemParam({ propertyId: 0, name: "fixed", isNewProperty: false });
    rndr.addProperties(names, items, ipfsGroup);

    // Success.
    tkn.mint();
    assertEq(tkn.totalSupply(), 2);
}

Recommended Mitigation Steps

Short term, in the addProperties function, ensure that each newly added property has at least one item.

Long term, after adding properties and items in the addProperties function, ensure that next token can be minted and rendered without errors.

[GalloDaSballo]

Consistently with #523 the Warden has shown how to cause mint to revert due to handling of properties and items, in this case by having a property without any.

Because this finding shows a different way to cause a revert, I will file it separately. Because it shows the same type of revert as #523 I'll judge it in the same way as Medium Severity