BwareTokenVault Manual Review Findings

BTV-01M: Exorbitantly High Gas Cost

Description:

The lockVault function can become in-executable in case the total investorCount becomes sufficiently high.

Example:

Copy
595for (uint256 i = 0; i < investorCount; i++) {
596    address _investor = investorAddress[i];
597
598    allocations[_investor] = investorAllocation[_investor];
599    walletGroup[_investor] = _investors_;
600
601    emit Allocated(_investor, investorAllocation[_investor]);
602}

Recommendation:

We advise the function to potentially halt mid-execution and continue where it left off in the investorCount iteration to ensure that it will be possible to execute it at all times since investors cannot be removed, they can only be set to 0.

Alleviation:

The development team has acknowledged this exhibit but decided to not apply its remediation in the current version of the codebase citing time constraints.

BTV-02M: Inexistent Input Sanitization

Description:

The _allocation variable is not sanitized, meaning that the investor allocations do not necessarily add up to the 15_000_000 mentioned by L484 in the token allocation declarations.

Example:

Copy
538function registerInvestor(address _investor, uint256 _allocation) external onlyOwner notLocked returns (bool) {
539    require(_investor != address(0), "Empty-address investor provided");
540    require(_allocation > 0, "Investor allocation is zero.");
541
542    if (investorAllocation[_investor] == 0)
543        investorAddress[investorCount++] = _investor;
544
545    investorAllocation[_investor] = _allocation * (10 ** 18);
546    return true;
547}

Recommendation:

We advise a separate variable to be introduced that tracks the total investor allocations and makes sure that they do not exceed 15 million as it would cause a failure in redemptions and cannot be adjusted post-lock.

Alleviation:

A require check was introduced in the lockVault function ensuring that the allocation is sane and as such, this exhibit has been dealt with since the owner will not be able to properly lock an invalid configuration.

BTV-03M: Pull-Over-Push Pattern

Description:

The _transferOwnership function which is invoked by its public non-prefixed counterpart overwrites the current _owner with the newOwner without validating that the latter is able to actuate transactions on the blockchain.

Example:

Copy
437function _transferOwnership(address newOwner) private {
438    require(newOwner != address(0), "Ownable: new owner is the zero address");
439    emit OwnershipTransferred(_owner, newOwner);
440    _owner = newOwner;
441}

Recommendation:

We advise the pull-over-push pattern to be applied whereby a new owner is first proposed and consequently needs to accept ownership via a dedicated function ensuring that they are able to transact on the blockchain.

Alleviation:

The development team has acknowledged this exhibit but decided to not apply its remediation in the current version of the codebase citing time constraints.