sherlock-audit / 2022-10-illuminate-judging

3 stars 0 forks source link

HonorLt - scheduleWithdrawal is useless #226

Closed sherlock-admin closed 2 years ago

sherlock-admin commented 2 years ago

HonorLt

medium

scheduleWithdrawal is useless

Summary

scheduleWithdrawal is not effective because an admin can approve any address anyway.

Vulnerability Detail

scheduleWithdrawal is intended to give users some time to react and avoid a possible rug pull, but in reality, it does not make sense if we have this:

    function approve(address[] calldata u, address[] calldata a)
        external
        authorized(admin)
        returns (bool)
    {
        for (uint256 i; i != u.length; ) {
            IERC20 uToken = IERC20(u[i]);
            if (address(0) != (address(uToken))) {
                Safe.approve(uToken, a[i], type(uint256).max);
            }
            unchecked {
                ++i;
            }
        }
        return true;
    }

An admin can just approve any address to withdraw any token making all the protections pointless.

Impact

I know admin over privilege is considered out of scope for the current iteration of the protocol but I see this case as a special one because here the protocol was explicitly intended to protect against this attack but left a gap to overcome this restriction.

Code Snippet

https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/Lender.sol#L782-L796

https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/Lender.sol#L141-L165

Tool used

Manual Review

Recommendation

Consider if you really need scheduled withdrawals.