hats-finance / Tapioca-0xe0b920d38a0900af3bab7ff0ca0af554129f54ad

4 stars 4 forks source link

Ownable.sol constructor param not supplied #1

Open hats-bug-reporter[bot] opened 6 months ago

hats-bug-reporter[bot] commented 6 months ago

Github username: @adeolu98 Twitter username: adeoluwami__ Submission hash (on-chain): 0x7480ed9e2f4d21bc43dd98ea3489507e454bc6b93ebc6acdd91d8214dfac2066 Severity: medium

Description:

Description

ownable.sol is used in Vesting.sol as seen here, ownable has a constructor and needs the constructor param to be supplied but vesting.sol contract doesnt supply this constructor param.

Attachments

  1. Proof of Concept (PoC) File

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/a241f099054953be8e30bbca5f47c9a79ed24c69/contracts/access/Ownable.sol#L38C1-L44C1

    constructor(address initialOwner) {
        if (initialOwner == address(0)) {
            revert OwnableInvalidOwner(address(0));
        }
        _transferOwnership(initialOwner);
    }

above we can see the constructor of ownable.sol and its parameter. Up next i will show vestitng.sol 's constructor

https://github.com/Tapioca-DAO/tap-token/blob/baddafc15616a5674e73c2f5a920b92bf9b21888/contracts/tokens/Vesting.sol#L84C1-L91C6

    constructor(uint256 _cliff, uint256 _duration, address _owner) {
        if (_duration == 0) revert VestingDurationNotValid();

        cliff = _cliff;
        duration = _duration;

        _transferOwnership(_owner);
    }

above in vesting.sol's constructor there is no definition of the param needed by Ownable which is a base contract here. This will mean Vesting.sol is not deployable as it doesnt supply the parameter and it is not an abstract contract.

Recommeneded Mitigation

add the ownable constructor to the Vesting contract constructor.

    constructor(uint256 _cliff, uint256 _duration, address _owner) Ownable(_owner) {
        if (_duration == 0) revert VestingDurationNotValid();

        cliff = _cliff;
        duration = _duration;

        _transferOwnership(_owner);
    }
0xRektora commented 6 months ago

This is wrong, there's no constructor arg on Ownable.