BundleRegistry Manual Review Findings

BRY-01M: Inexistent Initialization of Base Implementation

Type	Severity	Location
Language Specific		BundleRegistry.sol:L11-L17

Description:

The contract does not properly initialize the base logic implementation permitting it to be taken over by a malicious party.

Impact:

While not an active security threat, it can evolve into one if any form of delegatecall capability is introduced in one of the dependencies of the contract that could cause it to invoke a selfdestruct instruction.

Example:

Copy
11contract BundleRegistry is
12    IBundleRegistry,
13    Initializable,
14    OwnableUpgradeable,
15    UUPSUpgradeable,
16    PausableUpgradeable
17{

Recommendation:

We advise a constructor to be introduced that simply invokes the initializer modifier to ensure that the logic implementation cannot be initialized maliciously.

Alleviation (200f275c40cbd4798f4a416c044ea726755d4741):

A constructor was introduced that properly invokes the initializer modifier and disallows initialization of the logic implementation, alleviating this exhibit in full.

BRY-02M: Weak Validation of IPFS CIDv0

Type	Severity	Location
Input Sanitization		BundleRegistry.sol:L109-L110

Description:

The IPFS link is validated using the CIDv0 standard, however, only the first character is validated which is insufficient.

Impact:

It is currently possible to set invalid IPFS CIDv0 identifiers even though the system is meant to validate against them.

Example:

Copy
101/// @dev Checks if the passed string is a IPFS link or not.
102/// @param source String that needs to checked.
103/// @return true if the string passed is IPFS, else it will return false.
104function isIPFS(string memory source) internal pure returns (bool) {
105    bytes memory sourceToBytes = bytes(source);
106    bytes memory firstChar = new bytes(1);
107    firstChar[0] = sourceToBytes[0];
108    return
109        keccak256(firstChar) == keccak256(bytes("Q")) &&
110        sourceToBytes.length == 46;
111}

Recommendation:

We advise a more robust validation system to be put in place, ensuring that the string starts with Qm and does not end with O, I, or l as otherwise it is an invalidly defined CIDv0 identified. For more details, kindly consult this reference post.

Alleviation (200f275c40cbd4798f4a416c044ea726755d4741):

The isIPFS evaluation function was enhanced to apply stricter validation of the CIDv0 IPFS links provided according to our recommendation.