Description

In the TErc20 contract, the sweepToken function transfers unrelated tokens or tokens that may have accidentally been sent to the contract address, to the admin. This change was implemented in the following function:

function sweepToken(EIP20NonStandardInterface token) external override {
    require(msg.sender == admin, "TErc20::sweepToken: only admin can sweep tokens");
-   require(address(token) != underlying, "TErc20::sweepToken: can not sweep underlying token");
+   require(address(token) == underlying, "TErc20::sweepToken: can not sweep underlying token");
    uint256 balance = token.balanceOf(address(this));
    token.transfer(admin, balance);
}

This change requires that the swept token be equal to the underlying token, instead of requiring it to be the unrelated token. Since the rest of the function, including the interface and the string error message, was not changed, this appears to be an unintentional edit or an edit made to facilitate testing instead of implementing an intentional part of the design.

Impact

The swept token is no longer an unrelated token. This significantly changes the purpose of this function, because withdrawing the underlying token increases the risk that the contract becomes insolvent.

Recommendations

We recommend changing this back to properly check that the swept token is not the underlying token.

Alternatively, if the design intent is to allow the admin to freely withdraw liquidity from the contract, then we recommend adding another function to clearly document this design decision. In this case, the original sweepToken functionality is still useful, as unrelated tokens can always accidentally be transferred to the contract. Additionally, documenting the addition of this admin function communicates the design decision to the user, increasing clarity and transparency about the amount of centralization that this fork implements.

Remediation

This issue has been acknowledged by Takara Lend, and a fix was implemented in commit fa302d10↗.