> For the complete documentation index, see [llms.txt](https://ret2basic.gitbook.io/ctfnote/llms.txt). Markdown versions of documentation pages are available by appending `.md` to page URLs; this page is available as [Markdown](https://ret2basic.gitbook.io/ctfnote/web3-security-research/secureum/epoch-0/slot-7-audit-findings-101/opyn-gamma-openzeppelin.md).

# Opyn Gamma - OpenZeppelin

{% embed url="<https://blog.openzeppelin.com/opyn-gamma-protocol-audit/>" %}

## Progress

* [x] \[H-1] ETH could get trapped in the protocol
* [x] \[H-2] Protocol could become insolvent due to market’s natural movements
* [x] \[M-1] oToken can be created with a non-whitelisted collateral asset
* [x] \[M-2] ERC20 compliant assets may not be used
* [x] \[M-3] Actions are undefined at the exact time of oToken expiry
* [x] \[M-4] Use of transfer might render ETH impossible to withdraw
* [x] \[M-5] User can force the Controller contract to perform an undesired external call

## \[H-1] ETH could get trapped in the protocol

The `[Controller` contract]\(<https://github.com/opynfinance/GammaProtocol/blob/d151621b33134789b29dc78eb89dad2b557b25b9/contracts/Controller.sol#L27>) allows users to send arbitrary actions such as possible [flash loans](https://blog.openzeppelin.com/flash-loans-and-the-advent-of-episodic-finance/) through the `[_call` internal function]\(<https://github.com/opynfinance/GammaProtocol/blob/d151621b33134789b29dc78eb89dad2b557b25b9/contracts/Controller.sol#L763>).

Among other features, it allows sending ETH with the action to then perform a call to a `[CalleeInterface` type of contract]\(<https://github.com/opynfinance/GammaProtocol/blob/d151621b33134789b29dc78eb89dad2b557b25b9/contracts/interfaces/CalleeInterface.sol#L9>).

To do so, it saves the original `msg.value` sent with the `[operate` function call]\(<https://github.com/opynfinance/GammaProtocol/blob/d151621b33134789b29dc78eb89dad2b557b25b9/contracts/Controller.sol#L331>) in the `[ethLeft` variable]\(<https://github.com/opynfinance/GammaProtocol/blob/d151621b33134789b29dc78eb89dad2b557b25b9/contracts/Controller.sol#L460>) and it [updates the remaining ETH left](https://github.com/opynfinance/GammaProtocol/blob/d151621b33134789b29dc78eb89dad2b557b25b9/contracts/Controller.sol#L769) after each one of those calls to revert in case that it is not enough.

Nevertheless, if the user sends more than the necessary ETH for the batch of actions, the remaining ETH (stored in the `ethLeft` variable after the last iteration) will not be returned to the user and will be locked in the contract due to the lack of a `withdrawEth` function.

Consider either returning all the remaining ETH to the user or creating a function that allows the user to collect the remaining ETH after performing a `Call` action type, taking into account that sending ETH with a push method may trigger the fallback function on the caller’s address.

**Update:** *Fixed in* [*PR#304*](https://github.com/opynfinance/GammaProtocol/pull/304) *where the `payable` property is removed from the `operate` function. However this change also means it is impossible to do outbound calls which require ETH through the `operate` function.*

## \[C-2] Protocol could become insolvent due to market’s natural movements

**The Protocol uses 2 time-based prices to value all the actions related to assets: live and expired prices.**

The [live prices](https://github.com/opynfinance/GammaProtocol/blob/d151621b33134789b29dc78eb89dad2b557b25b9/contracts/MarginCalculator.sol#L361) are used by the `[getExcessCollateral` function]\(<https://github.com/opynfinance/GammaProtocol/blob/d151621b33134789b29dc78eb89dad2b557b25b9/contracts/MarginCalculator.sol#L74>) from the `MarginCalculator` contract to calculate the margin of a vault [before](https://github.com/opynfinance/GammaProtocol/blob/d151621b33134789b29dc78eb89dad2b557b25b9/contracts/MarginCalculator.sol#L157) the oToken expires. After the oToken expires, the [expired price](https://github.com/opynfinance/GammaProtocol/blob/d151621b33134789b29dc78eb89dad2b557b25b9/contracts/MarginCalculator.sol#L196) is then used to calculate the vault margin.

Because the [collateral asset](https://github.com/opynfinance/GammaProtocol/blob/d151621b33134789b29dc78eb89dad2b557b25b9/contracts/Otoken.sol#L29) may not be the same as the [strike asset](https://github.com/opynfinance/GammaProtocol/blob/d151621b33134789b29dc78eb89dad2b557b25b9/contracts/Otoken.sol#L26) or the [underlying asset](https://github.com/opynfinance/GammaProtocol/blob/d151621b33134789b29dc78eb89dad2b557b25b9/contracts/Otoken.sol#L23), it is not guaranteed that the collateral’s expiration price will be higher than its price in any point in the past, when the oToken has not expired yet, resulting in a possible undercollateralized situation for vaults.

Even though collateral assets must be whitelisted to be able to be used in the platform, with the caveat described in the issue ***oToken can be created with a non-whitelisted collateral asset***, assets such as cTokens from Compound that are supposed to gain value over time may suffer a drop in their values due to [market fluctuations or events as the one in early 2020](https://cointelegraph.com/news/maker-debt-crisis-post-mortem-recommends-new-safeguards), which produced a drop in value for cDai token. When the value of all collateral is worth less than the value of all borrowed assets, we say a market is insolvent. In case the platform allows the usage of non-monotonically-increasing price assets, the insolvency may be caused by a simple market price fluctuation.

Here are other examples of things that could cause a market to become insolvent:

* The price of the underlying (or borrowed) asset makes a big, quick move during a time of high network congestion.
* The price oracle temporarily goes offline during a time of high market volatility. This could result in the oracle not updating the asset prices until after the market has become insolvent.
* The admin or oracle steals enough collateral that the market becomes insolvent.
* Administrators list an ERC20 token with a later-discovered bug that allows minting of arbitrarily many tokens. This bad token is used as collateral to borrow funds that it never intends to repay.

In any case, the effects of an insolvent market could be disastrous. It may result in a “run on the bank” situation, with the last suppliers out losing their money. It is important to know that this risk does exist and it can be difficult to recover from even a small dip into insolvency.

Although the `[AddressBook` contract includes a slot for a liquidator manager]\(<https://github.com/opynfinance/GammaProtocol/blob/d151621b33134789b29dc78eb89dad2b557b25b9/contracts/AddressBook.sol#L27>), the system currently does not have any liquidation process, any vault is susceptible to becoming insolvent during the payout of the options right after expiration.

Consider adding a liquidation process to prevent insolvent vaults, carefully selecting the whitelisted assets for the protocol, adding more test units, and running a testnet version to understand how other assets may cause an undercollateralized scenario.

**Update:** *Fixed in* [*PR355*](https://github.com/opynfinance/GammaProtocol/pull/355)*, the Opyn team restricted all oTokens issued on the protocol to be collateralized with the exact payout asset depending on the oToken type(strike asset for PUT options and underlying asset for CALL options). Although this update restricted what Options the Opyn protocol can issue, it eliminated the risk of protocol become insolvent. We suggest a liquidation component to be ready in the future should the Opyn Team decide to remove PR 355 restriction, and get it thoroughly audited.*

## \[M-1] oToken can be created with a non-whitelisted collateral asset

A product consists of a set of assets and an option type. Each product has to be whitelisted by the admin using the [`whitelistProduct()` function](https://github.com/opynfinance/GammaProtocol/blob/d151621b33134789b29dc78eb89dad2b557b25b9/contracts/Whitelist.sol#L131) from the `Whitelist` contract.

Then, a user can call the [`createOtoken()` function](https://github.com/opynfinance/GammaProtocol/blob/d151621b33134789b29dc78eb89dad2b557b25b9/contracts/OtokenFactory.sol#L55) from the `OtokenFactory` with the same assets and option type, and because the product is whitelisted, the [requirement on line 70](https://github.com/opynfinance/GammaProtocol/blob/d151621b33134789b29dc78eb89dad2b557b25b9/contracts/OtokenFactory.sol#L70-L78) will succeed.

However, although the product has been whitelisted, the collateral itself may not be approved. This is because the `whitelistProduct()` function does not check against the [`isWhitelistedCollateral()` function](https://github.com/opynfinance/GammaProtocol/blob/d151621b33134789b29dc78eb89dad2b557b25b9/contracts/Whitelist.sol#L100) if that collateral is allowed in the platform or not. Therefore, the first engagement with the collateral will appear on [line 613](https://github.com/opynfinance/GammaProtocol/blob/d151621b33134789b29dc78eb89dad2b557b25b9/contracts/Controller.sol#L613) from `Controller.sol`, where the transaction will revert when the user wants to deposit some collateral in their vault.

Consider validating if the assets involved in a product have been already whitelisted before allowing the creation of oTokens.

**Update:** *Fixed in* [*PR#290*](https://github.com/opynfinance/GammaProtocol/pull/290) *where the collateral asset is required to be whitelisted during the process of whitelisting the product.*

## \[M-2] ERC20 compliant assets may not be used

A new oToken can be created by calling the [`createOtoken()` function](https://github.com/opynfinance/GammaProtocol/blob/d151621b33134789b29dc78eb89dad2b557b25b9/contracts/OtokenFactory.sol#L55) from the `OtokenFactory` contract and passing the whitelisted assets, among other parameters.

During the [initialization of the new oToken](https://github.com/opynfinance/GammaProtocol/blob/d151621b33134789b29dc78eb89dad2b557b25b9/contracts/Otoken.sol#L68), the code calls the [`_getNameAndSymbol()` function](https://github.com/opynfinance/GammaProtocol/blob/d151621b33134789b29dc78eb89dad2b557b25b9/contracts/Otoken.sol#L107) which retrieves the standardized symbol and name for that oToken.

Nevertheless, the same function [calls every single asset involved in the oToken to get their symbol](https://github.com/opynfinance/GammaProtocol/blob/d151621b33134789b29dc78eb89dad2b557b25b9/contracts/Otoken.sol#L108-L110), but because the [`symbol()` and `name()` function from the ERC20 standard are optionals](https://eips.ethereum.org/EIPS/eip-20#specification), these external calls may fail if those are not implemented and the oToken will not be created.

Similarly, the `decimals()` function is optional and ERC20 compliant assets may not include such function. Although in this scenario an oToken could be created using those assets, any action that would trigger the [`_verifyFinalState()` function](https://github.com/opynfinance/GammaProtocol/blob/d151621b33134789b29dc78eb89dad2b557b25b9/contracts/Controller.sol#L514) from the `Controller` contract will revert either in line [90](https://github.com/opynfinance/GammaProtocol/blob/d151621b33134789b29dc78eb89dad2b557b25b9/contracts/MarginCalculator.sol#L90) or [101](https://github.com/opynfinance/GammaProtocol/blob/d151621b33134789b29dc78eb89dad2b557b25b9/contracts/MarginCalculator.sol#L101) from `MarginCalculator.sol`.

Consider being as general as possible and assuming that these functions may not be implemented in the whitelisted assets.

**Update:** *The Opyn team explained their views regarding this issue in a follow-up discussion. They decided not to implement any fixes and, instead, they will rely on the admin process of whitelisting underlying ERC20 tokens that conform to the requirements of their system. In the future, if the need arises, they intend to upgrade the system to support a broader range of ERC20 tokens.*

## \[M-3] Actions are undefined at the exact time of oToken expiry

The `oToken` contract defines an [expiration time](https://github.com/opynfinance/GammaProtocol/blob/d151621b33134789b29dc78eb89dad2b557b25b9/contracts/Otoken.sol#L35) in which certain operations will no longer be active due to the expiration of the option. The option style used for the Protocol is the [european option](https://www.investopedia.com/terms/e/europeanoption.asp) in which the exercise of the option comes at maturity.

Some actions that can be performed before the expiration time are [depositing long oTokens](https://github.com/opynfinance/GammaProtocol/blob/d151621b33134789b29dc78eb89dad2b557b25b9/contracts/Controller.sol#L587) or [minting new oTokens](https://github.com/opynfinance/GammaProtocol/blob/d151621b33134789b29dc78eb89dad2b557b25b9/contracts/Controller.sol#L669); while after the expiration time it can be [redeemed](https://github.com/opynfinance/GammaProtocol/blob/d151621b33134789b29dc78eb89dad2b557b25b9/contracts/Controller.sol#L710) or [settled a vault](https://github.com/opynfinance/GammaProtocol/blob/d151621b33134789b29dc78eb89dad2b557b25b9/contracts/Controller.sol#L739).

Nevertheless, at the exact time of expiry, none of the time-based actions will succeed because that moment is not included in either of those 2 conditions and the transaction will revert.

Because it is based on a European option, actions that happen after maturity should be able to be called at the exact time of expiry. Consider including and defining the expiration time to either of the 2 conditions so transactions will not revert.

**Update:** *Fixed in* [*PR#291*](https://github.com/opynfinance/GammaProtocol/pull/291)*.*

## \[M-4] Use of transfer might render ETH impossible to withdraw

When withdrawing ETH deposits, the `[PayableProxyController` contract]\(<https://github.com/opynfinance/GammaProtocol/blob/d151621b33134789b29dc78eb89dad2b557b25b9/contracts/external/proxies/PayableProxyController.sol#L19>) uses Solidity’s [`transfer()` function](https://github.com/opynfinance/GammaProtocol/blob/d151621b33134789b29dc78eb89dad2b557b25b9/contracts/external/proxies/PayableProxyController.sol#L73). This has some notable shortcomings when the withdrawer is a smart contract, which can render ETH deposits impossible to withdraw. Specifically, the withdrawal will inevitably fail when:

* The withdrawer smart contract does not implement a payable fallback function.
* The withdrawer smart contract implements a payable fallback function which uses more than 2300 gas units.
* The withdrawer smart contract implements a payable fallback function which needs less than 2300 gas units but is called through a proxy that raises the call’s gas usage above 2300.

To prevent unexpected behavior and potential loss of funds, consider explicitly warning end-users about the mentioned shortcomings to raise awareness before they deposit Ether into the protocol. Additionally, note that the [`sendValue()` function](https://github.com/OpenZeppelin/openzeppelin-contracts/blob/v2.5.1/contracts/utils/Address.sol#L63) available in OpenZeppelin Contract’s `Address` library can be used to transfer the withdrawn Ether without being limited to 2300 gas units. Risks of reentrancy stemming from the use of this function can be mitigated by tightly following the [“Check-effects-interactions” pattern](https://solidity.readthedocs.io/en/latest/security-considerations.html#use-the-checks-effects-interactions-pattern) and using OpenZeppelin Contract’s `[ReentrancyGuard` contract]\(<https://github.com/OpenZeppelin/openzeppelin-contracts/blob/v2.5.1/contracts/utils/ReentrancyGuard.sol>). For further reference on why using Solidity’s `transfer()` is no longer recommended, refer to these articles:

* [Stop using Solidity’s transfer now](https://diligence.consensys.net/blog/2019/09/stop-using-soliditys-transfer-now/)
* [Reentrancy after Istanbul](https://blog.openzeppelin.com/reentrancy-after-istanbul/)

**Update:** *Fixed in* [*PR#305*](https://github.com/opynfinance/GammaProtocol/pull/305)*.*

## \[M-5] User can force the Controller contract to perform an undesired external call

The `[Controller` contract]\(<https://github.com/opynfinance/GammaProtocol/blob/d151621b33134789b29dc78eb89dad2b557b25b9/contracts/Controller.sol#L27>) is the users’ front gate. With it, they can interact with the protocol to either open a new vault, deposit collateral or redeem their oTokens.

Besides those actions, the contract allows the execution of a more general transaction to be used as a [flash loan](https://blog.openzeppelin.com/flash-loans-and-the-advent-of-episodic-finance/) in other projects by formatting the call with the `[CallArgs` struct format]\(<https://github.com/opynfinance/GammaProtocol/blob/d151621b33134789b29dc78eb89dad2b557b25b9/contracts/libs/Actions.sol#L137>).

By doing this, if the user submits that action in the [`operate()` function](https://github.com/opynfinance/GammaProtocol/blob/d151621b33134789b29dc78eb89dad2b557b25b9/contracts/Controller.sol#L331), the call would jump into the [`_call()` function](https://github.com/opynfinance/GammaProtocol/blob/d151621b33134789b29dc78eb89dad2b557b25b9/contracts/Controller.sol#L763) to then perform an external call to the [`callFunction()` payable function](https://github.com/opynfinance/GammaProtocol/blob/d151621b33134789b29dc78eb89dad2b557b25b9/contracts/Controller.sol#L770) in the `[callee` address]\(<https://github.com/opynfinance/GammaProtocol/blob/d151621b33134789b29dc78eb89dad2b557b25b9/contracts/libs/Actions.sol#L141>).

However, if the `callee` address is not a `[CalleeInterface` based contract]\(<https://github.com/opynfinance/GammaProtocol/blob/d151621b33134789b29dc78eb89dad2b557b25b9/contracts/interfaces/CalleeInterface.sol#L9>) but it has either a fallback or payable fallback function in it, and [it is not restricted](https://github.com/opynfinance/GammaProtocol/blob/d151621b33134789b29dc78eb89dad2b557b25b9/contracts/Controller.sol#L54) the whitelisted addresses, the call coming from the `Controller` contract will end up executing any code under the fallback function on behalf of the `Controller` contract’s address. This other address could be either an asset that may be part of the whitelisted assets in the protocol or a future contract of the project that allows the execution of sensitive actions by the `Controller` contract.

Consider preventing an external call on behalf of the `Controller` contract when the destination address is not a `CalleeInterface` type of contract and it is not a whitelisted address.


---

# Agent Instructions
This documentation is published with GitBook. GitBook is the documentation platform designed so that both humans and AI agents can read, navigate, and reason over technical content effectively. Learn more at gitbook.com.

## Querying This Documentation
If you need additional information that is not directly available in this page, you can query the documentation dynamically by asking a question.

Perform an HTTP GET request on the current page URL with the `ask` query parameter, and the optional `goal` query parameter:

```
GET https://ret2basic.gitbook.io/ctfnote/web3-security-research/secureum/epoch-0/slot-7-audit-findings-101/opyn-gamma-openzeppelin.md?ask=<question>&goal=<endgoal>
```

`ask` is the immediate question: it should be specific, self-contained, and written in natural language.
`goal` is optional and describes the broader end goal you are ultimately trying to accomplish on behalf of the user. GitBook uses it to tailor the answer towards what is most useful for that goal.

The response will contain a direct answer to the question and relevant excerpts and sources from the documentation.

Use this mechanism when the answer is not explicitly present in the current page, you need clarification or additional context, or you want to retrieve related documentation sections.