Ethena Labs - Walter's results

EthenaMinting.sol

1. Lack of signer!=address(0) check

no check on whatever ECDSA.recover(...) will return address(0) ----> in line 341 should be checked like so: require(signer!=address(0),"invalid signer"); line: https://github.com/code-423n4/2023-10-ethena/blob/ee67d9b542642c9757a6b826c82d0cae60256509/contracts/EthenaMinting.sol#L341 2) #### Unnecessary control of order type in verifyRoute function

In the Solidity code for the verifyRoute function, there exists an unnecessary conditional check of orderType == OrderType.REDEEM. This condition is redundant and can be safely removed, as the verifyRoute function is only called within the mint function.

In the mint function, there is already a check on the order type, and therefore, rechecking it within the verifyRoute function is unnecessary. This additional check doesn't provide any extra value or security; instead, it introduces code redundancy and reduces code readability.

Moreover, in the redeem function, there's a check only on the order type without any analysis of the route, which further underscores the redundancy of the orderType check in verifyRoute.

Since the verifyRoute function is exclusively used within the mint function, and the redeem function solely checks the order type without route analysis, removing this redundant condition in verifyRoute will help streamline the code and improve its maintainability without impacting the program's functionality.

resulting in this updated code:

function verifyRoute(Route calldata route) public view override returns (bool) {
    uint256 totalRatio = 0;
    if (route.addresses.length != route.ratios.length) {
      return false;
    }
    if (route.addresses.length == 0) {
      return false;
    }
    for (uint256 i = 0; i < route.addresses.length; ++i) {
      if (!_custodianAddresses.contains(route.addresses[i]) || route.addresses[i] == address(0) || route.ratios[i] == 0)
      {
        return false;
      }
      totalRatio += route.ratios[i];
    }
    if (totalRatio != 10_000) {
      return false;
    }
    return true;
  }

then needs to be updated the call of this function in such this way: if (!verifyRoute(route)) revert InvalidRoute(); 3) #### Unexpected revert when withdrawing currently if the user needs to withdraw his funds and the amount to be withdrawn is larger than the address balance,the contract will revert,this in my opinion is a problem because will block the user funds in the contract itself,to fix this problem,the contract could send all its balance to the user even if it doesnt reach the amount required from the user affected code:

if (address(this).balance < amount) revert InvalidAmount();

could be resolved in this way:

if (address(this).balance < amount) {
(bool success,) = (beneficiary).call{value: address(this).balance}("");
if (!success) revert TransferFailed();
}

on the other hand, when withdrawing tokens that are not native, we have a similar problem that can be solved in a similar way, in fact the check on the balance of the token does not even take place,so we could re-write all the _transferToBeneficiary function in this way:

  function _transferToBeneficiary(address beneficiary, address asset, uint256 amount) internal {
    if (asset == NATIVE_TOKEN) {
        if (address(this).balance < amount) {
            (bool success,) = (beneficiary).call{value: address(this).balance}("");
            if (!success) revert TransferFailed();
        }else{
            (bool success,) = (beneficiary).call{value: amount}("");
            if (!success) revert TransferFailed();
        }
    } else {
      if (!_supportedAssets.contains(asset)) revert UnsupportedAsset();
      if (IERC20(asset).balanceOf(address(this)) < amount) {{
        IERC20(asset).safeTransfer(beneficiary, IERC20(asset).balanceOf(address(this)));
      }
      IERC20(asset).safeTransfer(beneficiary, amount);
      }
    }
  }

USDeSilo.sol

1. should remove immutable on STAKING_VAULT and USDE variables,making the contract upgradeable to future updates and insert all the set functions for each one of those variable