Announcing audit results for the Rightshare smart contracts

Note: This article was originally published in Lendroid’s blog site on Apr 29, 2020.

It is now possible to share access to NFTs — lease, collaborate, advertise, With 100% branch coverage of the source code; comprehensive fixes; next-level collaboration with Quantstamp.

We are proud to announce the audit results for Rightshare v1 contracts in this post. Rightshare is the first of its kind to enable leasing of NFTs in a trustless manner. The smart contracts are written in solidity and have been audited by Quantstamp.

When the smart contracts were submitted for an initial review, the source code had an 85% branch coverage. Following the initial review, we fixed the vulnerabilities identified and also increased our branch coverage to 100%. During this process, out of 13, two critical, one medium, two low, five informational and three undetermined risk issues were identified and resolved.

Security vulnerabilities and fixes

As users continue to trust emerging DAOs and dApps with various functionalities, ranging from safe DeFi to governance and community participation, it becomes paramount to ensure best practices are followed when developing smart contracts to enable such functionalities. We highlight four such vulnerabilities identified and fixed during the audit.

Vulnerability 1 (High) : Addresses of Rights contracts can be changed arbitrarily and repeatedly

Summary of the risk:

The smart contract system is defined by the Rights DAO, iRights and fRights. Therefore, a small change in one of the contracts could pose a threat to the overall philosophy accountable to the nature of decentralization for which the contracts were written.

Fix :

the function set_right() related to this risk was removed. And the addresses were added directly to the rightsDao constructor. This would make potential DAO models work more fluidly with systems that have the i and f rights.

Vulnerability 2 (High) : Potential denial-of-service in isApprovedForAll( )

Summary of the risk:

to give a better understanding of this risk consider the following analogy where an artist chooses to list her work of art for different rights.

Consider the following two cases:

Case 1:

The proxy registry has not registered her. In this case the default proxy would return a 0x address.

Case 2:

The artist chooses to be listed via the dao, and the dao address is added to the proxy registry. A vulnerability in the proxy registry changes the proxy address to zero address. As a result, this would prevent the artist from exercising her rights using the proxy registry.

Fix:

We included a check where if the proxy address of the artist is zero, then the function should call the default function of the corresponding NFT.

Vulnerability 3 (Informational) : Potential redundancy of exclusivity (given its equivalence to max supply = 1)

Summary of the risk:

The fRights token has properties identified by its metadata — namely version, tokenID, startTime, endTime, baseAssetAddress, baseAssetId, isExclusive, maxISupply, and circulatingISupply.

These uniquely represent an fRights and are set when the fRights token is created by the `freeze` function. Therefore, it is important to reduce any attack vectors that could corrupt the meaning of the fRights.

Fix:

Exclusivity was derived from the value of max supply provided to the freeze function, thus removing any redundancy while setting the token properties.

Vulnerability 4 (Undetermined) : Serial numbers are reused upon revocation

Summary of the risk:

Every iRights is assigned a serial number when it gets minted. When an IRights is burnt, its corresponding iRights are not updated. Therefore minting a new iRights will result in duplication of serial numbers.This can cause an issue for a renderer such as the OpenSea platform — a marketplace where NFTs are listed with traits. When these specific traits mismatch or show indistinct errors, it could be attributed to the misuse of serial numbers.

Fix:

We removed the serial numbers altogether. The side-effect of this was that in order to display NFT traits “1 or <maxSupply>/<circulatingSupply>” on OpenSea, the metadata had to query the parentID of the iRights contract.

Post deployment

After deployment to mainnet, it was observed that the smart contracts did not support NFTs with older implementation of the `safeTransferFrom` function (eg, Crypto Voxels). Based on valuable suggestions from the Quantstamp team, we deployed another version which was backward compatible. For an elegant future-proof solution to such problems, we are exploring adding upgradability to our smart contracts.

Note: This article was originally published in Lendroid’s blog site on Apr 29, 2020.