rule to detect curve readonly reentrancy (#49)

* rule to detect curve readonly reentrancy

* checkReentrancy pattern + README

README.md

@@ -116,6 +116,7 @@ accessible-selfdestruct | Generic | Contract can be destructed by anyone in $FUN
 no-slippage-check| Generic|  No slippage check in a Uniswap v2/v3 trade
 balancer-readonly-reentrancy-getrate | Balancer | getRate() call on a Balancer pool is not protected from the read-only reentrancy.
 balancer-readonly-reentrancy-getpooltokens | Balancer | getPoolTokens() call on a Balancer pool is not protected from the read-only reentrancy.
+curve-readonly-reentrancy | Curve | get_virtual_price() call on a Curve pool is not protected from the read-only reentrancy.
 
 ## Gas Optimization Rules
 

solidity/security/curve-readonly-reentrancy.sol

@@ -0,0 +1,65 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.0;
+
+contract CurveBuggyContract {
+    function _getPrice(IPriceOracle _priceOracle, ILPCurve _lpCurve)
+        internal
+        virtual
+        returns (uint256)
+    {
+        ICurvePool _pool = ICurvePool(_lpCurve.minter());
+        IiToken _coin = IiToken(_pool.coins(0));
+        if (_coin == ETH) _coin = iETH;
+
+        ICurvePoolOwner(getCurvePoolOwner()).withdraw_admin_fees(_pool);
+        return
+            _priceOracle.getUnderlyingPrice(_coin).mul(
+                // ok: curve-readonly-reentrancy
+                _pool.get_virtual_price()
+            ) / 10**uint256(doubleDecimals).sub(uint256(_coin.decimals()));
+    }
+
+    function __setPoolInfo2(address _pool, address _invariantProxyAsset, bool _reentrantVirtualPrice) private {
+        uint256 lastValidatedVirtualPrice;
+        if (_reentrantVirtualPrice) {
+            // Validate the virtual price by calling a non-reentrant pool function
+            // ruleid: curve-readonly-reentrancy
+            lastValidatedVirtualPrice = ICurveLiquidityPool(_pool).get_virtual_price();
+
+            emit ValidatedVirtualPriceForPoolUpdated(_pool, lastValidatedVirtualPrice);
+        }
+
+        poolToPoolInfo[_pool] = PoolInfo({
+            invariantProxyAsset: _invariantProxyAsset,
+            invariantProxyAssetDecimals: ERC20(_invariantProxyAsset).decimals(),
+            lastValidatedVirtualPrice: uint88(lastValidatedVirtualPrice)
+        });
+
+        emit InvariantProxyAssetForPoolSet(_pool, _invariantProxyAsset);
+    }
+
+    function __setPoolInfo(address _pool, address _invariantProxyAsset, bool _reentrantVirtualPrice) private {
+        uint256 lastValidatedVirtualPrice;
+        if (_reentrantVirtualPrice) {
+            // Validate the virtual price by calling a non-reentrant pool function
+            __makeNonReentrantPoolCall(_pool);
+            // ok: curve-readonly-reentrancy
+            lastValidatedVirtualPrice = ICurveLiquidityPool(_pool).get_virtual_price();
+
+            emit ValidatedVirtualPriceForPoolUpdated(_pool, lastValidatedVirtualPrice);
+        }
+
+        poolToPoolInfo[_pool] = PoolInfo({
+            invariantProxyAsset: _invariantProxyAsset,
+            invariantProxyAssetDecimals: ERC20(_invariantProxyAsset).decimals(),
+            lastValidatedVirtualPrice: uint88(lastValidatedVirtualPrice)
+        });
+
+        emit InvariantProxyAssetForPoolSet(_pool, _invariantProxyAsset);
+    }
+
+    /// @dev Helper to call a known non-reenterable pool function
+    function __makeNonReentrantPoolCall(address _pool) private {
+        ICurvePoolOwner(getCurvePoolOwner()).withdraw_admin_fees(_pool);
+    }
+}

solidity/security/curve-readonly-reentrancy.yaml

@@ -0,0 +1,65 @@
+rules:
+  - id: curve-readonly-reentrancy
+    message: $POOL.get_virtual_price() call on a Curve pool is not protected from the read-only reentrancy.
+    metadata:
+      references:
+        - https://chainsecurity.com/heartbreaks-curve-lp-oracles/
+        - https://chainsecurity.com/curve-lp-oracle-manipulation-post-mortem/
+      category: security
+      tags:
+        - curve
+        - reentrancy
+    patterns:
+      - pattern: |
+          $POOL.get_virtual_price()
+      - pattern-not-inside: |
+          function $F(...) {
+            ...
+            $VAR.withdraw_admin_fees(...);
+            ...
+          }
+      - pattern-not-inside: |
+          function $F(...) {
+            ...
+            $VAR.withdraw_admin_fees(...);
+            ...
+          }
+      - pattern-not-inside: |
+          contract $C {
+            ...
+            function $CHECKFUNC(...) {
+              ...
+              $VAR.withdraw_admin_fees(...);
+              ...
+            }
+            ...
+            function $F(...) {
+              ...
+              $CHECKFUNC(...);
+              ...
+              $POOL.get_virtual_price();
+              ...
+            }
+            ...
+          }
+      - pattern-not-inside: |
+          contract $C {
+            ...
+            function $CHECKFUNC(...) {
+              ...
+              $VAR.withdraw_admin_fees(...);
+              ...
+            }
+            ...
+            function $F(...) {
+              ...
+              $POOL.get_virtual_price();
+              ...
+              $CHECKFUNC(...);
+              ...
+            }
+            ...
+          }
+    languages:
+      - solidity
+    severity: ERROR