Reflexer security record

The `DepositRAI.vy` and `StableSwapRAI.vy` files use an old version of the Vyper compiler which is known to contain high severity vulnerabilities such as: 1. Memory corruption when returning a literal struct with a private call inside of it [GHSA-xv8x-pr4h-73jv](https://github.com/vyperlang/vyper/security/advisories/GHSA-xv8x-pr4h-73jv). 2. Storage variables overwritten by re-entrancy locks [GHSA-7f92-rr6w-cq64](https://github.com/vyperlang/vyper/security/advisories/GHSA-7f92-rr6w-cq64).

The [requirements](https://gitcoin.co/issue/reflexer-labs/curve-contract/1/100026434) state that: > In order to fetch RAI's redemption price, one has to first update it and then retrieve it. This is why the [redemption price getter](https://github.com/reflexer-labs/geb/blob/ef7c7f20d047dd0fe8b7dc4cb24f4d5f1c7a04dd/src/single/OracleRelayer.sol#L265) is not a view function. To make it easier to fetch the redemption price, you can use [this contract](https://github.com/reflexer-labs/geb-redemption-price-snap/blob/master/src/RedemptionPriceSnap.sol) that stores a slightly older version of the price or you can update and fetch the latest price from [here](https://github.com/reflexer-labs/geb/blob/ef7c7f20d047dd0fe8b7dc4cb24f4d5f1c7a04dd/src/single/OracleRelayer.sol#L265). However, using a slightly older version is a sub-optimal solution that might enable arbitrage opportunities depending on how outdated the prices is.

The `README.md` and `pooldata.json` files contain address placeholders for the RAI Curve pool contracts. Users should be able to check the correct address where these contracts were deployed, in order to avoid using the wrong contracts (by mistake or social engineering attacks).

The `Curve` interface uses the `BASE_N_COINS` constant with functions `calc_token_amount` (L20) and `add_liquidity` (L26) in `StableSwapRAI.vy`. The same issue occurs in `MetaRAI.vy` on different line numbers. However, the specs show https://curve.readthedocs.io/exchange-deposits.html?highlight=N_COINS#id14 show that `N_COINS` constant is used. Moreover the actual implementation of those two functions also employ the `N_COINS` constant internally as a loop bound. This discrepancy may lead to issues since the values of these 2 constants are different for the RAI pool.

User inputs should always be checked to be in the expected bounds, especially if the inputs come from untrusted users. However, trusted user inputs should also be checked to avoid incidents caused by human error. The following functions do not have proper input validation in place: 1. `_A` , `_fee` and `_admin_fee` in __init__() of `StableSwapRAI.vy` are not validated. Values bigger than `MAX_A `, `MAX_ADMIN_FEE` or `MAX_FEE` can be set here. 2. `_owner`, ` _pool_token`, `_base_pool` in `__init__()` of `StableSwapRAI.vy` are not validated. Providing incorrect values may result in a later discovery that the smart contract is not working as intended. 3. `_token` in `__init__()` of `DepositRAI.vy` is not validated. Providing incorrect values may result in a later discovery that the smart contract is not working as intended. 4. On L430, L644, L716, L844, L1051 in `MetaRAI.vy` there is no check that `_receiver` is different from the zero address.

The redemption price data is provided by the external RAI `RedemptionPriceSnap` oracle and is queried in the following functions of `MetaRAI.vy` and `StableSwapRAI.vy`: - `get_virtual_price()`(, through `_xp()` in the case of `StableSwapRAI.vy`) - `calc_token_amount()` - `add_liquidity()` - `get_dy()` - `get_dy_underlying()` - `exchange()` - `exchange_underlying()` - `remove_liquidity_imbalance()` - `calc_withdraw_one_coin()`, through `_calc_withdraw_one_coin()` - `remove_liquidity_one_coin()`, through `_calc_withdraw_one_coin()` As this oracle influences the amount of tokens one may get when interacting with the pool its reliance is a potential risk factor if the oracle is attacked.

Smart contracts will often have `owner` variables to designate the person with special privileges to make modifications to the smart contract. In the case of the RAI pool contracts the `Factory(self.factory).admin()` and `self.owner` have privileged roles that may pose a risk to end-users.

In L178 (MetaRAI) and L88 (DepositRAI), the function `ERC20(coin).approve` is called with the maximum value, which is considered security anti-pattern and could lead to end-user wallet funds being drained if the contract is exploited.

Variable shadowing occurs when a local or `calldata` variable uses the same name as a storage variable. This could lead to ambiguities regarding which variable is actually used. The following instances have been observed: 1. The local variable `base_pool` inside `__init__` shadows the storage variable `base_pool`. 2. The local variable `base_pool` inside `remove_liquidity_imbalance` shadows the storage variable `base_pool`. 3. The local variable `base_coins` inside `remove_liquidity_imbalance` shadows the storage variable `base_coins`.

The function `restartAuction()` works even when the contract is not enabled (unlike other methods). Also, it can be called by anyone (no authorization checks).