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);
}
}
}