Summary :
The DatasetAccessToken
contract has a vulnerability in its constructor where it assumes that the datasetAccessRegistry
is not null without checking.
A require
statement can be added to the constructor to ensure that the datasetAccessRegistry
is not null.
The require
statement checks if the address of the datasetAccessRegistry
is not equal to the zero address.
By adding this require
statement, the contract ensures that the datasetAccessRegistry
is not null before using it, preventing unexpected behavior or errors.
Vulnerability Details : The DatasetAccessToken contract has a vulnerability in its constructor where it assumes that the datasetAccessRegistry is not null without checking. This can lead to unexpected behavior or errors if a null value is passed to the constructor.
To exploit this vulnerability, an attacker would need to pass a null value to the constructor of the DatasetAccessToken contract. This could be done by creating a new instance of the contract and passing a null value as the datasetAccessRegistry parameter.
Impact : The impact of this vulnerability is medium, as it can lead to unexpected behavior or errors if a null value is passed to the constructor. However, it is not a critical vulnerability as it does not allow for unauthorized access or control of the contract.
Proof of Concept Code : Here's a proof of concept code that demonstrates the vulnerability in the DatasetAccessToken contract..
pragma solidity ^0.8.0;
import {ERC721} from "@openzeppelin/contracts/token/ERC721/ERC721.sol";
import {ERC721Burnable} from "@openzeppelin/contracts/token/ERC721/extensions/ERC721Burnable.sol";
contract DatasetAccessToken is ERC721, ERC721Burnable {
constructor(DatasetAccessRegistry datasetAccessRegistry_, uint256 knowledgeId_, address owner_, uint256 supply_)
ERC721("Dataset Access Token", "DAT")
{
datasetAccessRegistry = datasetAccessRegistry_;
knowledgeId = knowledgeId_;
KnowledgeRegistry knowledgeRegistry = datasetAccessRegistry.knowledgeRegistry();
require(knowledgeRegistry.isRegistered(knowledgeId_), "Knowledge not registered");
for (uint256 i = 0; i < supply_; i++) {
ERC721._mint(owner_, i);
}
}
}
contract Attacker {
DatasetAccessToken public datasetAccessToken;
constructor(DatasetAccessToken _datasetAccessToken) public {
datasetAccessToken = _datasetAccessToken;
}
function attack() public {
DatasetAccessToken newContract = new DatasetAccessToken(address(0), 0, msg.sender, 1);
newContract.constructor(address(0), 0, msg.sender, 1);
}
}
In this proof of concept code, we have two contracts: DatasetAccessToken
and Attacker
. The DatasetAccessToken
contract has a vulnerability in its constructor where it assumes that the datasetAccessRegistry
is not null without checking. The Attacker
contract creates a new instance of the DatasetAccessToken
contract with a null datasetAccessRegistry
and calls the constructor of the new contract.
To deploy and run this proof of concept code, you can use a tool like Truffle or Remix. Here are the steps:
Deploy the DatasetAccessToken
contract to the Ethereum blockchain.
Deploy the Attacker
contract to the Ethereum blockchain.
Call the attack
function on the Attacker
contract.
Observe the behavior of the contracts and verify that the vulnerability is exploited.
Tools Used VS Code
Recommendations : To fix this issue, a require
statement can be added to the constructor to ensure that the datasetAccessRegistry
is not null. The require
statement checks if the address of the datasetAccessRegistry
is not equal to the zero address (which represents a null value in Solidity).
Here's the updated constructor code with the added require
statement:
constructor(DatasetAccessRegistry datasetAccessRegistry_, uint256 knowledgeId_, address owner_, uint256 supply_)
ERC721("Dataset Access Token", "DAT")
{
require(address(datasetAccessRegistry_) != address(0), "DatasetAccessRegistry cannot be null");
datasetAccessRegistry = datasetAccessRegistry_;
knowledgeId = knowledgeId_;
KnowledgeRegistry knowledgeRegistry = datasetAccessRegistry.knowledgeRegistry();
require(knowledgeRegistry.isRegistered(knowledgeId_), "Knowledge not registered");
for (uint256 i = 0; i < supply_; i++) {
ERC721._mint(owner_, i);
}
}
}