Canto Identity Subprotocols contest - d3e4's results

Lines of code

https://github.com/code-423n4/2023-03-canto-identity/blob/077372297fc419ea7688ab62cc3fd4e8f4e24e66/canto-pfp-protocol/src/ProfilePicture.sol#L59 https://github.com/code-423n4/2023-03-canto-identity/blob/077372297fc419ea7688ab62cc3fd4e8f4e24e66/canto-pfp-protocol/src/ProfilePicture.sol#L99

Vulnerability details

Impact

Besides having to re-register the protocol, it will also have to be redeployed.

Proof of Concept

A protocol is registered by name in the SubprotocolRegistry. Quoting the Canto Identity Protocol contest details: "In theory, someone can front-run a call to SubprotocolRegistry.register with the same name, causing the original call to fail. There is a registration fee (100 $NOTE) and the damage is very limited (original registration call fails, user can just re-register under a different name), so this attack is not deemed feasible.". However, the ProfilePicture subprotocol suffers from the additional issue of also having to be redeployed, making this exploit more severe.

In ProfilePicture, subprotocolName is set on construction.

constructor(address _cidNFT, string memory _subprotocolName) ERC721("Profile Picture", "PFP") {
    cidNFT = ICidNFT(_cidNFT);
    subprotocolName = _subprotocolName;

subprotocolName is used to retrieve the cidNFTID to which the subprotocols NFT is added.

uint256 cidNFTID = cidNFT.getPrimaryCIDNFT(subprotocolName, _pfpID);
IAddressRegistry addressRegistry = cidNFT.addressRegistry();
if (cidNFTID == 0 || addressRegistry.getAddress(cidNFTID) != ERC721(nftContract).ownerOf(nftID)) {
    nftContract = address(0);
    nftID = 0; // Strictly not needed because nftContract has to be always checked, but reset nevertheless to 0
}

This means that this subprotocol must to be registered under this name, or it will not work. So if ProfilePicture is deployed but someone else manages to register it in the SubprotocolRegistry before the owner (recipient of fees) of ProfilePicture (by frontrunning or otherwise), it cannot simply be re-registered under a different name, but would have to be redeployed with a new name under which it then can be registered.

Tools Used

Code inspection

Recommended Mitigation Steps

Since the subprotocol is meant to be registered with the CID protocol, its deployment and registration should be atomic. Or the name can be initialized after deployment and registration, and set by a (temporary) owner to the name it then has been registered with.

Lines of code

https://github.com/code-423n4/2023-03-canto-identity/blob/077372297fc419ea7688ab62cc3fd4e8f4e24e66/canto-namespace-protocol/src/Utils.sol#L136-L174 https://github.com/code-423n4/2023-03-canto-identity/blob/077372297fc419ea7688ab62cc3fd4e8f4e24e66/canto-namespace-protocol/src/Utils.sol#L256-L261

Vulnerability details

Impact

Only a tiny fraction of all Zalgo distortions are accessible.

Proof of Concept

In characterToUnicodeBytes, for font class 7 (i.e. Zalgo), the _characterModifier determines the Zalgo distortion. The distortion is pseudo-randomly calculated by using _characterModifier as a seed and iteratively passing it through iteratePRNG, each intermediate value being used to set, in turn, each component of the distortion.

} else if (_fontClass == 7) {
    // Zalgo
    uint8 asciiStartingIndex = 97;
    uint256 numAbove = (_characterModifier % 7) + 1;
    // We do not reuse the same seed for the following generations to avoid any symmetries, e.g. that 2 chars above would also always result in 2 chars below
    _characterModifier = iteratePRNG(_characterModifier);
    uint256 numMiddle = _characterModifier % 2;
    _characterModifier = iteratePRNG(_characterModifier);
    uint256 numBelow = (_characterModifier % 7) + 1;
    // ...

This has the effect that the initial seed value generates a stream of values, equidistributed within their respective ranges.

A Zalgo tile is defined by a letter and a modification consisting of a combination of characters above, over and below the letter. The modification consists of a selection of 1 up to 7 characters from a set of 46 characters above the letter, 0 or 1 from 5 over the letter, and 1 to 7 from 47 below the letter. Thus there is a total of $(46^1 + 46^2 + ... + 46^7)(5^0 + 5^1)(47^1 + 47^2 + ... + 47^7) = 1.38e24$ different Zalgo modifications possible per letter.

However, iteratePRNG modulates its return value by 2038074743:

function iteratePRNG(uint256 _currState) internal pure returns (uint256 iteratedState) {
    unchecked {
        iteratedState = _currState * 15485863;
        iteratedState = (iteratedState * iteratedState * iteratedState) % 2038074743;
    }
}

This means that after the first pass through iteratePRNG we only have 2038074743 different possibilities which will determine the rest of the distortion. The original _characterModifier is first used to set to uint256 numAbove = (_characterModifier % 7) + 1;. The remaining combinations are therefore 1.38e24 / 7 = 1.98e+23, which is vastly greater than 2038074743.

characterToUnicodeBytes can thus only return 7 * 2038074743 = 14266523201 different distortions per letter, which is only a tiny fraction of the actual number of distortions.

Note that it is similarly an issue that the range of _characterModifier is only 0..255, which issue is reportedly separately.

Tools Used

Code inspection

Recommended Mitigation Steps

iteratePRNG must have a range of at least 1.98e+23 different values.

Lines of code

https://github.com/code-423n4/2023-03-canto-identity/blob/077372297fc419ea7688ab62cc3fd4e8f4e24e66/canto-namespace-protocol/src/Tray.sol#L163 https://github.com/code-423n4/2023-03-canto-identity/blob/077372297fc419ea7688ab62cc3fd4e8f4e24e66/canto-namespace-protocol/src/Tray.sol#L264-L268 https://github.com/code-423n4/2023-03-canto-identity/blob/077372297fc419ea7688ab62cc3fd4e8f4e24e66/canto-namespace-protocol/src/Utils.sol#L136-L174

Vulnerability details

Impact

Only 256 Zalgo distortions are possible, which is a miniscule fraction of the actual combinations possible.

Proof of Concept

A Zalgo tile is defined by a letter and a modification consisting of a combination of characters above, over and below the letter. The modification consists of a selection of 1 up to 7 characters from a set of 46 characters above the letter, 0 or 1 from 5 over the letter, and 1 to 7 from 47 below the letter. Thus there is a total of $(46^1 + 46^2 + ... + 46^7)(5^0 + 5^1)(47^1 + 47^2 + ... + 47^7) = 1.38e24$ different Zalgo modifications possible per letter.

However, the distortions are uniquely determined by characterModifier which is a uint8. So only 256 different distortions per letter are possible.

When a tray is bought, _drawing sets the tile,

function buy(uint256 _amount) external {
// ...
            trayTiledata[j] = _drawing(uint256(lastHash));
// ...
}

and, if luck would have it that it is a Zalgo tile, characterModifier is set to a pseudo-random uint8, i.e. in the range 0..255.

function _drawing(uint256 _seed) private pure returns (TileData memory tileData) {
// ...
            if (tileData.fontClass == 7) {
                // Set seed for Zalgo to ensure same characters will be always generated for this tile
                uint256 zalgoSeed = Utils.iteratePRNG(_seed);
                tileData.characterModifier = uint8(zalgoSeed % 256);
// ...
}

This characterModifier uniquely determines the distortion in characterToUnicodeBytes at L136-L174:

} else if (_fontClass == 7) {
    // Zalgo
    uint8 asciiStartingIndex = 97;
    uint256 numAbove = (_characterModifier % 7) + 1;
    // We do not reuse the same seed for the following generations to avoid any symmetries, e.g. that 2 chars above would also always result in 2 chars below
    _characterModifier = iteratePRNG(_characterModifier);
    uint256 numMiddle = _characterModifier % 2;
    _characterModifier = iteratePRNG(_characterModifier);
    uint256 numBelow = (_characterModifier % 7) + 1;
    bytes memory character = abi.encodePacked(bytes1(asciiStartingIndex + uint8(_characterIndex)));
    for (uint256 i; i < numAbove; ++i) {
        _characterModifier = iteratePRNG(_characterModifier);
        uint256 characterIndex = (_characterModifier % ZALGO_NUM_ABOVE) * 2;
        character = abi.encodePacked(
            character,
            ZALGO_ABOVE_LETTER[characterIndex],
            ZALGO_ABOVE_LETTER[characterIndex + 1]
        );
    }
    for (uint256 i; i < numMiddle; ++i) {
        _characterModifier = iteratePRNG(_characterModifier);
        uint256 characterIndex = (_characterModifier % ZALGO_NUM_OVER) * 2;
        character = abi.encodePacked(
            character,
            ZALGO_OVER_LETTER[characterIndex],
            ZALGO_OVER_LETTER[characterIndex + 1]
        );
    }
    for (uint256 i; i < numBelow; ++i) {
        _characterModifier = iteratePRNG(_characterModifier);
        uint256 characterIndex = (_characterModifier % ZALGO_NUM_BELOW) * 2;
        character = abi.encodePacked(
            character,
            ZALGO_BELOW_LETTER[characterIndex],
            ZALGO_BELOW_LETTER[characterIndex + 1]
        );
    }
    return character;
} else {

Note that there is also a similar bottleneck issue with iteratePRNG, which is reported as a separate issue.

Tools Used

Code inspection

Recommended Mitigation Steps

Let characterModifier have a range of at least 0..1.38e24. Since it therefore cannot be a uint8, simply let it be a uint256 (or at least a uint88).