DataIndex Manual Review Findings

DIX-01M: Incorrect Mask Specification

Type	Severity	Location
Logical Fault		DataIndex.sol:L18, L94-L99

Description:

The PREFIX_MASK is meant to act as a safeguard against duplicate diid_ values that point to the same account in the DataIndex::ownerOf function, however, the present definition of the prefix is incorrect.

Specifically, it is 10 bytes instead of 12 bytes long permitting 2**16 - 1 DIID permutations that point to the same account to be supplied to the DataIndex::ownerOf function.

Impact:

Multiple unique DIID values can point to the same account incorrectly in the current DataIndex implementation.

Example:

Copy
93///@inheritdoc IIDManager
94function ownerOf(bytes32 diid_) external view returns (uint32, address) {
95    if (diid_ & PREFIX_MASK != 0) revert IncorrectIdentifier(diid_); // Require first 12 bytes empty, leaving only 20 bytes of address non-empty
96    address account = address(uint160(uint256(diid_)));
97    if (account == address(0)) revert IncorrectIdentifier(diid_);
98    return (ChainidTools.chainid(), account);
99}

Recommendation:

We advise the prefix to be corrected by adding four more F characters, ensuring that the upper bits are properly ensured to be clean during data validation within the DataIndex::ownerOf function.

Alleviation:

The PREFIX_MASK declaration has been corrected per our recommendation, ensuring upper bits are correctly mandated as clear in the DataIndex::ownerOf function.

DIX-02M: Irrevocable Write Authorizations

Type	Severity	Location
Logical Fault		DataIndex.sol:L44-L50, L57, L84

Description:

The DataIndex::allowDataManager function will permit an administrator of a particular DataPoint to authorize a data manager to write to it.

A problem with the current approach is the fact that the revocation of an administrator at the DataPointRegistry level will not cause their authorizations through the DataIndex to be revoked, permitting a malicious administrator to expend significant gas to approve many data managers so as to hinder the de-authorization efforts of the DataPoint owner.

Impact:

The removal of a malicious administrator of a DataPoint will not cause their approvals to be revoked, instead requiring them to be manually removed one-by-one which is ill-advised and can significantly increase the time-to-resolution of a misbehaving administrator.

Example:

Copy
72///@inheritdoc IDataIndex
73function allowDataManager(DataPoint dp, address dm, bool approved) external onlyDPOwner(dp) {
74    _dmApprovals[dp][dm] = approved;
75    emit DataPointDMApprovalChanged(dp, dm, approved);
76}
77
78///@inheritdoc IDataIndex
79function read(address dobj, DataPoint dp, bytes4 operation, bytes calldata data) external view returns (bytes memory) {
80    return IDataObject(dobj).read(dp, operation, data);
81}
82
83///@inheritdoc IDataIndex
84function write(address dobj, DataPoint dp, bytes4 operation, bytes calldata data) external onlyApprovedDM(dp) returns (bytes memory) {
85    return IDataObject(dobj).write(dp, operation, data);
86}

Recommendation:

We advise the system to be refactored, converting the _dmApprovals mapping to point to an address data entry instead of a bool entry.

This will permit the system to store the administrator that authorized a particular data manager to mutate a particular DataPoint, allowing them to be validated during a DataIndex::write call as continuing to be an administrator of the DataPoint to prevent the scenario outlined by this exhibit.

Alleviation:

The authorization system in use by the DataIndex implementation has been refactored in line with our recommendations, ensuring that the administrator who authorized a particular user is still an active administrator of the data point the authorization was assigned for.

As a result, authorizations are immediately revoked whenever an administrator is de-authorized thus addressing the described security concern.