TokenMigrator Static Analysis Findings

TMR-01S: Potentially Unsafe Approve Invocations

Type	Severity	Location
Standard Conformity		TokenMigrator.sol:L76, L77

Description:

The linked approve instructions may yield a bool variable that currently remains unsanitized. As per the EIP-20 standard, callers MUST NOT assume that false is never returned.

Impact:

A potential failure case for the approvals will require a redeployment of the protocol to ensure that it has sufficient allowance to interact with the revenue-locked BTRFLY as well as the staking implementation's unstake function.

Example:

Copy
76xBtrfly.approve(staking_, type(uint256).max);
77btrflyV2.approve(rlBtrfly_, type(uint256).max);

Recommendation:

We advise the SafeTransferLib implementation already utilized by the codebase to be imported here and its safeApprove function to be utilized. To note, the Solmate implementation does not suffer from the same flaws as OpenZeppelin's and as such is safe to use in this scenario as it solely evaluates the returned bool.

Alleviation:

The correct safeApprove implementation from the Solmate library is now utilized in the codebase as advised.

TMR-02S: Improper Invocations of EIP-20 transferFrom

Type	Severity	Location
Standard Conformity		TokenMigrator.sol:L90, L106, L121

Description:

The linked statements do not properly validate the returned bool of the EIP-20 standard transferFrom function. As the standard dictates, callers must not assume that false is never returned.

Impact:

If the code mandates that the returned bool is true, this will cause incompatibility with tokens such as USDT / Tether as no such bool is returned to be evaluated causing the check to fail at all times. On the other hand, if the token utilized can return a false value under certain conditions but the code does not validate it, the contract itself can be compromised as having received / sent funds that it never did.

Example:

Copy
90wxBtrfly.transferFrom(msg.sender, address(this), amount);

Recommendation:

Since not all standardized tokens are EIP-20 compliant (such as Tether / USDT), we advise a safe wrapper library to be utilized instead such as SafeERC20 by OpenZeppelin to opportunistically validate the returned bool only if it exists in each instance.

Alleviation:

All relevant transferFrom invocations have been properly replaced by their OpenZeppelin SafeERC20 counterparts alleviating this exhibit.