code-423n4 / 2022-03-maple-findings

0 stars 0 forks source link

QA Report #21

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

QA Report

Non-Critical Findings

Redundant type cast to address

Description

Variable asset is defined as address public override asset, the type casting to address is redundant.

Findings

RevenueDistributionToken.sol#L162
RevenueDistributionToken.sol#L181

Recommended mitigation steps

Remove redundant type cast.

From:

require(ERC20Helper.transfer(address(asset), receiver_, assets_), "RDT:B:TRANSFER");

To:

require(ERC20Helper.transfer(asset, receiver_, assets_), "RDT:B:TRANSFER");

Open TODOs in code

Description

Open TODOs can hint at programming or architectural errors that still need to be fixed.

Findings

RevenueDistributionToken.sol#L78
RevenueDistributionToken.sol#L276

Recommended mitigation steps

Implement open TODOs and remove comments.

xMPL.performMigration() is safe to be called by everyone

Description

xMPL.performMigration() is currently only allowed to be called by the contract owner, but as there are no funds at risk and no downsides to having everyone (public) call the function, the modifier onlyOwner can be removed.

It even states so in the README:

3. After the time delay, anyone can call `performMigration`, which executes the migration with the parameters set 10 days prior.

Findings

xMPL.performMigration()

Recommended mitigation steps

Remove modifier onlyOwner.

Low Risk

None found.

lucas-manuel commented 2 years ago
  1. Valid, will remove, informational
  2. Duplicate
  3. We want to perform the migration ourselves through governance. Even though this is possible, we want to keep it this way.