the function `isValidInputRoutes` handling an empty `inputRouters_` array.

[hats-bug-reporter[bot]]

Github username: -- Twitter username: -- Submission hash (on-chain): 0x3ae97bfa83154a3f4d14ad19d2d2efb615525a2f580b1fc77f8b172b20e43374 Severity: low

Description: Description

1. Function Overview:
   • isValidInputRoutes: This function iterates through each element of the inputRouters_ array, checking if the 'from' address of each route is allowed by consulting a mapping (isAllowedTokenInInputRoutes). If any 'from' address is not allowed, the function returns false. If it completes the for-loop without finding any non-allowed token, it returns true.

    function isValidInputRoutes(IRouterV2.route[] calldata inputRouters_) external view returns (bool) {
        for (uint256 i; i < inputRouters_.length; ) {
            if (!isAllowedTokenInInputRoutes[inputRouters_[i].from]) {
                return false;
            }
            unchecked {
                i++;
            }
        }
        return true;
    }

2. For-loop Execution with Empty Array:

   • When inputRouters_ is empty (inputRouters_.length == 0), the initialization of the for-loop uint256 i; i < inputRouters_.length; sets i to 0.
   • The loop condition i < inputRouters_.length checks if 0 < 0, which is false when the array is empty.
   • This results in the for-loop never executing its body, and no checks on route validity are performed.

3. Effect of Empty Array on Function Return:

   • Since the for-loop does not run due to the array being empty, the function, by default, moves to its final statement which is return true;.
   • The return of true in this scenario may be misleading as there were actually no route objects in the array to validate, so the "valid" status merely reflects the absence of invalid routes rather than the presence of exclusively valid routes.

Attack Scenario

1. Empty Array Exploit:

   • Assume a contract or an external caller calls the function isValidInputRoutes with an empty array (inputRouters_).
   • Because the for-loop initialization (uint256 i; i < inputRouters_.length;) doesn't inherently restrict execution based on the array containing elements, it directly initializes i to 0 and the condition is checked. With an empty array (inputRouters_.length being 0), the condition i < inputRouters_.length fails.
   • This causes the loop not to execute and no check is made on any routes at all.
   • Consequently, the function simply progresses to return true at the end, because the for-loop body, which includes the validation check, never runs.

2. Outcome of Exploit:

   • The function isValidInputRoutes erroneously returns true for an empty array, indicating that no routes are invalid when, in truth, no validation was ever performed.
   • This false positive can mislead other functions or operations within the contract or ecosystem that rely on this function for input validation of routing paths assuming that all given routes meets the required criteria where actually no route was ever checked.

Attachments

1. Proof of Concept (PoC) File

2. Revised Code File (Optional)

[0xmahdirostami]

Good catch, however the function buybackTokenByV2 checks for the length of the input routes, and by default, the zero length array wouldn't even be passed to the isValidInputRoutes function