---
# System prepended metadata

title: Gelato Aave Security Assessment (DONE)

---

# Gelato Aave Security Assessment (DONE)

<!-- ## Overview -->



<!-- ## Privileged Roles -->


# Findings

## Incorrect calculation of amtOfColToWithdraw and amtToFlashBorrow

<!-- Severity: `Medium` -->

https://github.com/gelatodigital/gelato-aave/blob/4a6d42fc61e21ca1e0850d6d088763e2cd08d12d/contracts/functions/FProtectionResolver.sol#L18-L66


```solidity=
function _amountToPaybackAndWithdraw(
    bytes32 _id,
    ProtectionDataCompute memory _protectionDataCompute
) view returns (RepayAndWithdrawResult memory) {
    // Liquidation is a 4 digit values lower than 10000.
    uint256 intermediateValue =
        _wdiv(
            (
                (
                    (_wmul(
                        _protectionDataCompute.wantedHealthFactor,
                        _protectionDataCompute.totalBorrowsETH
                    ) -
                        (
                            _qmul(
                                _protectionDataCompute.totalCollateralETH,
                                _protectionDataCompute
                                    .currentLiquidationThreshold
                            )
                        ))
                )
            ),
            _qmul(
                _protectionDataCompute.wantedHealthFactor,
                (TEN_THOUSAND_BPS - _protectionDataCompute.flashloanPremium)
            ) -
                _qmul(
                    _protectionDataCompute.colLiquidationThreshold,
                    (TEN_THOUSAND_BPS +
                        _protectionDataCompute.protectionActionFee)
                ) *
                1e14
        );

    uint256 colTokenDecimals =
        ERC20(_protectionDataCompute.colToken).decimals();
    uint256 debtTokenDecimals =
        ERC20(_protectionDataCompute.debtToken).decimals();

    return
        RepayAndWithdrawResult(
            _id,
            ((_tokenToTokenPrecision(intermediateValue, 18, colTokenDecimals) *
                10**colTokenDecimals) / _protectionDataCompute.colPrice),
            ((_tokenToTokenPrecision(intermediateValue, 18, debtTokenDecimals) *
                10**debtTokenDecimals) / _protectionDataCompute.debtPrice),
            "OK"
        );
}
```

### Issue Description

$amtOfColToWithdraw = amtToFlashBorrow \times (1 + flashloanPremiumRate)$

$wantedHealthFactor = {totalCollateralETH \times currentLiquidationThreshold - amtOfColToWithdraw \times (1 + protectionActionFeeRate) \times colLiquidationThreshold \over totalBorrowsETH - amtToFlashBorrow}$

$\iff$

$amtOfColToWithdraw = { wantedHealthFactor \times totalBorrowsETH - totalCollateralETH \times currentLiquidationThreshold \over {wantedHealthFactor \over 1 + flashloanPremiumRate} - (1 + protectionActionFeeRate) \times colLiquidationThreshold}$

$amtToFlashBorrow = {amtOfColToWithdraw \over 1 + flashloanPremiumRate}$


### Recommendation

-   change
    ```solidity=
    _qmul(
        _protectionDataCompute.wantedHealthFactor,
        (TEN_THOUSAND_BPS - _protectionDataCompute.flashloanPremium)
    )
    ```
    to
    
    ```solidity=
    _qdiv(
        _protectionDataCompute.wantedHealthFactor,
        (TEN_THOUSAND_BPS + _protectionDataCompute.flashloanPremium)
    )
    ```
-   change
    ```solidity=
    return
        RepayAndWithdrawResult(
            _id,
            ((_tokenToTokenPrecision(intermediateValue, 18, colTokenDecimals) *
                10**colTokenDecimals) / _protectionDataCompute.colPrice),
            ((_tokenToTokenPrecision(intermediateValue, 18, debtTokenDecimals) *
                10**debtTokenDecimals) / _protectionDataCompute.debtPrice),
            "OK"
        );
    ```
    to
    ```solidity=
    return
        RepayAndWithdrawResult(
            _id,
            ((_tokenToTokenPrecision(intermediateValue, 18, colTokenDecimals) *
                10**colTokenDecimals) / _protectionDataCompute.colPrice),
            ((_tokenToTokenPrecision(_qdiv(intermediateValue, TEN_THOUSAND_BPS + _protectionDataCompute.flashloanPremium), 18, debtTokenDecimals) *
                10**debtTokenDecimals) / _protectionDataCompute.debtPrice),
            "OK"
        );
    ```

<!-- ### Resolution
 -->

---

## Miscalculation in _tokenToTokenPrecision when _oldPrecision <= _newPrecision

<!-- Severity: `Medium` -->

https://github.com/gelatodigital/gelato-aave/blob/4a6d42fc61e21ca1e0850d6d088763e2cd08d12d/contracts/functions/FProtectionResolver.sol#L122-L131


```solidity=
function _tokenToTokenPrecision(
    uint256 _amount,
    uint256 _oldPrecision,
    uint256 _newPrecision
) pure returns (uint256) {
    return
        _oldPrecision > _newPrecision
            ? _amount / (10**(_oldPrecision - _newPrecision))
            : _amount * (10**(_oldPrecision - _newPrecision));
}
```

### Issue Description

When `_oldPrecision <= _newPrecision`, it should return `_amount * (10**(_newPrecision - _oldPrecision))`



### Recommendation

change
```solidity=
_amount * (10**(_oldPrecision - _newPrecision))
```
to
    
```solidity=
_amount * (10**(_newPrecision - _oldPrecision))
```



---


## Unused code

<!-- Severity: `Informational` -->

### Issue Description

https://github.com/gelatodigital/gelato-aave/blob/master/contracts/services/aave/resolver/ProtectionResolver.sol#L131-L141

https://github.com/gelatodigital/gelato-aave/blob/master/contracts/functions/FProtectionResolver.sol#L83-L118

The variable `colInETH` and `debtInETH` in `ProtectionResolver.sol#protectionDataCompute()` and the function `_getColInETH` and `_getDebtInEth` in `FProtectionResolver.sol` are unused. They may be leftover from a previous version.

### Recommendation

Consider removing unused variables and functions.



---


## Misleading names

<!-- Severity: `Informational` -->

### Issue Description

- https://github.com/gelatodigital/gelato-aave/blob/master/contracts/functions/FProtectionAction.sol#L107

    The second parameter of `_withdrawCollateral()` named `aaveService` is actually not the aaveService contract.
    
    **Recommendation**: Consider changing the name to `recipient` or `to`.

- https://github.com/gelatodigital/gelato-aave/blob/master/contracts/functions/FProtectionAction.sol#L144

    The function `FProtectionAction.sol#_isPositionSafe` and `FProtectionAction.sol#_isPositionUnSafe` suggests that it returns a bool that indicates whether the position is safe/unsafe, while it actually requires the position to be safe/unsafe.
    
    https://github.com/gelatodigital/gelato-aave/blob/master/contracts/functions/FProtectionResolver.sol#L139-L146

    Using `_isPositionSafe` and `_isPositionUnSafe` is also likely to be confused with `FProtectionResolver.sol#_isPositionUnsafe`.
    
    **Recommendation**: Consider changing the name from `_isPositionUnSafe` to `_requirePositionUnSafe`, and `_isPositionSafe` to `_requirePositionSafe`.


- https://github.com/gelatodigital/gelato-aave/blob/e9cd8e37aea7117273decc379175cfe923f780a6/contracts/services/aave/resolver/ProtectionResolver.sol#L129-L132
    
    The name `protectionActionFee` and `flashloanPremium` might be misinterpreted as the amount of the fees.

    **Recommendation**: Consider changing the name from `protectionActionFee` to `protectionActionFeeBps`, and `flashloanPremium` to `flashloanPremiumBps`.


---

## Unnecessary imports

<!-- Severity: `Informational` -->

### Issue Description

- https://github.com/gelatodigital/gelato-aave/blob/master/contracts/services/aave/FlashLoanReceiverBase.sol#L17

    Unnecessary import of `SafeMath` library in `FlashLoanReceiverBase.sol`.

- https://github.com/gelatodigital/gelato-aave/blob/master/contracts/structs/SProtection.sol#L4

    `import {Path} from "../structs/SParaswap.sol"`

- https://github.com/gelatodigital/gelato-aave/blob/master/contracts/functions/FProtectionAction.sol#L10

    `import {_rdiv} from "../vendor/DSMath.sol"`

- https://github.com/gelatodigital/gelato-aave/blob/7bc98ff445dc700e158a56832e35a677e2313fc2/contracts/services/aave/resolver/ProtectionResolver.sol#L12

    Unnecessary import of `IPriceOracle`, `ProtectionDataCompute` and `IERC20`.

### Recommendation

Remove unnecessary imports.


---

## Wrong name or value of `isPositionSafe`

<!-- Severity: `Informational` -->

### Issue Description

https://github.com/gelatodigital/gelato-aave/blob/master/contracts/services/aave/resolver/ProtectionResolver.sol#L188-L191

```solidity=
result.isPositionSafe = _isPositionUnsafe(
    _canExecData.user,
    _canExecData.minimumHF
);
```

### Recommendation

Consider changing to:

```solidity=
result.isPositionSafe = !_isPositionUnsafe(
    _canExecData.user,
    _canExecData.minimumHF
);
```

or

```solidity=
result.isPositionUnsafe = _isPositionUnsafe(
    _canExecData.user,
    _canExecData.minimumHF
);
```



---

## Unnecessary ternary statements

### Issue Description

https://github.com/gelatodigital/gelato-aave/blob/master/contracts/functions/FProtectionResolver.sol#L94-L114

```solidity=
function _isPositionUnsafe(address _user, uint256 _minimumHF)
    view
    returns (bool)
{
    (, , , , , uint256 currenthealthFactor) =
        ILendingPool(LENDINGPOOL).getUserAccountData(_user);
    return currenthealthFactor < _minimumHF ? true : false;
}

function _isAllowed(
    address _aToken,
    address _user,
    address _spender,
    uint256 _allowedAmt
) view returns (bool) {
    return
        IERC20(_aToken).balanceOf(_user) >= _allowedAmt &&
            IERC20(_aToken).allowance(_user, _spender) >= _allowedAmt
            ? true
            : false;
}
```

The ternary statements in `FProtectionResolver.sol#_isPositionUnsafe()` and `FProtectionResolver.sol#_isAllowed()` are unnecessary. They may be leftover from a previous version.

### Recommendation

Consider removing the unnecessary ternary statements.

---

## Leftover debtTokens in edge cases are not returned

### Issue Description

https://github.com/gelatodigital/gelato-aave/blob/master/contracts/services/aave/actions/ProtectionAction.sol#L195-L201

The current implementation of `ProtectionAction.sol#_executeOperation()` won't return the leftover debtTokens in edge cases of the `debtRepaid` being larger than the debt amount.

As an example, consider the scenario of:

Current debt amount: 10,000 USDC
Calculated amtOfDebtToRepay : 9,999 USDC

After the task is generated and submitted, but before executed, the user repaid the debt for 2 USDC.

So, by the time we execute the task, the `amtOfDebtToRepay` will actually be larger than the debt amount (9,998 USDC). As a result, 1 USDC will be left in the contract.

### Recommendation

Consider checking for leftover debtTokens after `_paybackToLendingPool` and send them back to the user.

---

## Using all the wallet balance can cause Denial of Service (DoS)

### Issue Description

https://github.com/gelatodigital/gelato-aave/blob/f58b6b042090582edec59c2b65f1497a5f8901c7/contracts/services/aave/actions/ProtectionAction.sol#L200

https://github.com/gelatodigital/gelato-aave/blob/7bc98ff445dc700e158a56832e35a677e2313fc2/contracts/functions/FProtectionAction.sol#L242

`ProtectionAction.sol#_executeOperation()` and `FProtectionAction.sol#_swap()` is using all the wallet balance to swap and repay debt. 

An attacker may send a certain amount of colToken/debtToken to the `ProtectionAction` contract which leads to over repay of the debt so that the result Health Factor can be higher than the upper limit and cause the transaction to be reverted.

### Recommendation

Consider using `_flashloanData.amounts[0]` to swap, and `receivedAmt` to repay debt.


---

## ProtectionAction.sol allows for arbitrary code execution

### Issue Description

https://github.com/gelatodigital/gelato-aave/blob/f58b6b042090582edec59c2b65f1497a5f8901c7/contracts/functions/FProtectionAction.sol#L181

The Gelato protocol's design allows the executors to craft calldata and make call to `ProtectionAction.sol` on behalf of the GELATO address, and the on-chain requirements only enforced the post protection health factor.

A compromised or malicious executor can take advantage of this and stealing user's funds.

As an example, consider the scenario of:

A user with 200 ETH worth of collateral and 50 ETH worth of debt, and the weighted average liquidation threshold is 50%, then the current Health Factor will be:

(200 * 0.5) / 50 = 200%

If the target Health Factor of the protection task is 300%.

A malicious executor can craft a transaction that do the following:

1. Withdraw collateral and repay 49.9 ETH worth of debt;
2. Withdraw 149.5 ETH worth of collateral and transfer to the executor's address;

The final Health Factor will be:

(200 - 49.9 - 149.5) * 0.5 / (50 - 49.9) = 300%

Which will pass the onchain check.

### Recommendation

Create an isolated swap contract and heighten the on-chain requirements.

### Resolution

This issue has been addressed with commit [7bc98ff445dc700e158a56832e35a677e2313fc2](https://github.com/gelatodigital/gelato-aave/commit/7bc98ff445dc700e158a56832e35a677e2313fc2) by adding an isolated swap contract and on-chain verification of `amtToFlashBorrow` and `amtOfDebtToRepay`.

<!--

## 待定或待写

-   把闪电贷改为借 colToken
-   任意代码执行 at https://github.com/gelatodigital/gelato-aave/blob/e9cd8e37aea7117273decc379175cfe923f780a6/contracts/functions/FProtectionAction.sol#L167-L177

-   unused import
    -   unused import of IPriceOracle, ProtectionDataCompute and IERC20 in [ProtectionResolver.sol](https://github.com/gelatodigital/gelato-aave/blob/7bc98ff445dc700e158a56832e35a677e2313fc2/contracts/services/aave/resolver/ProtectionResolver.sol#L12)
-->