v0.6.0

.github/workflows/benchmark.yml

@@ -0,0 +1,102 @@
+name: Run Benchmark
+
+on:
+    pull_request:
+        branches:
+            - 'master'
+            - 'develop'
+jobs:
+    RunBenchmarkMainnet: 
+        runs-on: ubuntu-latest
+        steps:
+            - name: Checkout repository
+              uses: actions/checkout@v4
+              with:
+                submodules: 'true'
+            - name: Set up Python
+              uses: actions/setup-python@v4
+              with:
+                python-version: '3.x'
+            - name: Set up Node
+              uses: actions/setup-node@v4
+              with:
+                node-version: '18.x'
+            - name: Update pip
+              run: python -m pip install --upgrade pip
+            - name: Install solc-select
+              run: python -m pip install solc-select
+            - name: Install Slither
+              run: python -m pip install slither-analyzer
+            - name: Install Setuptools
+              run: python -m pip install setuptools
+            - name: Install Slitherin
+              run: python setup.py develop
+            - name: Configure
+              run: |
+                cd slitherin-benchmark/
+                mv example.config.py config.py
+            - name: Install node dependencies
+              run: npm ci
+            - name: Install benchmark requirements
+              run: |
+                cd slitherin-benchmark/
+                python -m pip install -r requirements.txt
+            - name: Run Benchmark
+              run: |
+                cd slitherin-benchmark/
+                python runner.py -i contracts/mainnet -o mainnet.csv --limit 7000
+            - name: 'Upload Artifact'
+              uses: actions/upload-artifact@v3
+              with:
+                name: mainnet
+                path: slitherin-benchmark/mainnet.csv
+    RunBenchmarkOZ: 
+        runs-on: ubuntu-latest
+        steps:
+            - name: Checkout repository
+              uses: actions/checkout@v4
+              with:
+                submodules: 'true'
+            - name: Set up Python
+              uses: actions/setup-python@v4
+              with:
+                python-version: '3.x'
+            - name: Set up Node
+              uses: actions/setup-node@v4
+              with:
+                node-version: '18.x'
+            - name: Update pip
+              run: python -m pip install --upgrade pip
+            - name: Install solc-select
+              run: python -m pip install solc-select
+            - name: Install Slither
+              run: python -m pip install slither-analyzer
+            - name: Install Setuptools
+              run: python -m pip install setuptools
+            - name: Install Slitherin
+              run: python setup.py develop
+            - name: Configure
+              run: |
+                cd slitherin-benchmark/
+                mv example.config.py config.py
+            - name: Install node dependencies
+              run: npm ci
+            - name: Install benchmark requirements
+              run: |
+                cd slitherin-benchmark/
+                python -m pip install -r requirements.txt
+            - name: Run Benchmark
+              run: |
+                cd slitherin-benchmark/
+                python runner.py -i contracts/openzeppelin -o oz.csv -eo oz_extra.csv
+            - name: 'Upload Artifact'
+              uses: actions/upload-artifact@v3
+              with:
+                name: oz
+                path: slitherin-benchmark/oz.csv
+            - name: 'Upload Artifact'
+              uses: actions/upload-artifact@v3
+              with:
+                name: oz_extra
+                path: slitherin-benchmark/oz_extra.csv
+

.gitmodules

@@ -0,0 +1,3 @@
+[submodule "slitherin-benchmark"]
+	path = slitherin-benchmark
+	url = https://github.com/pessimistic-io/slitherin-benchmark.git

README.md

@@ -64,6 +64,10 @@ slitherin . --slither
 ```bash
 slitherin . --separated
 ```
+* Run Arbitrum-specific Slitherin detectors:
+```bash
+slitherin . --arbitrum
+```
 > Keep in mind that Slitherin-cli supports all Slither run options.
 ### Slither
 Slitherin detectors are included into original Slither after the installation. You can use Slither [as usual](https://github.com/crytic/slither#usage).

docs/arb_block_number_timestamp.md

@@ -0,0 +1,19 @@
+# Arbitrum block.number/block.timestamp
+
+## Configuration
+
+- Check: `pess-arb-solidity-version`
+- Severity: `Low`
+- Confidence: `High`
+
+## Description
+
+`block.number` and `block.timestamp` behave different, than how they behave on Ethereum. For details: [arbitrum docs](https://docs.arbitrum.io/for-devs/concepts/differences-between-arbitrum-ethereum/block-numbers-and-time)
+
+## Vulnerable Scenario
+
+[test scenarios](../tests/arbitrum_block_number_timestamp_test.sol)
+
+## Recommendation
+
+Verify, that contract's logic does not break because of this difference

docs/arb_difficulty_randao.md

@@ -0,0 +1,15 @@
+# Arbitrum prevRandao/difficulty
+
+## Configuration
+* Check: `pess-arb-prevrandao-difficulty`
+* Severity: `Medium`
+* Confidence: `High`
+
+## Description
+The detector sees if an Arbitrum contract contains usages of prevRandao/difficulty block context variables or Yul funciton.
+
+## Vulnerable Scenario
+[test scenarios](../tests/arbitrum_prevrandao_difficulty_test.sol)
+
+## Recommendation
+Consider removing usage of prevRandao/difficulty inside Arbitrum contracts as they will constantly return `1` in Arbitrum.

docs/arb_solidity_version.md

@@ -0,0 +1,19 @@
+# Arbitrum solidity version
+
+## Configuration
+
+- Check: `pess-arb-solidity-version`
+- Severity: `High`
+- Confidence: `Low`
+
+## Description
+
+Checks that sol version >= 0.8.20 is not used inside an Arbitrum contract. Solidity in these versions will utilize PUSH0 opcode, which is not supported on Arbitrum.
+
+## Vulnerable Scenario
+
+[test scenarios](../tests/arbitrum_prevrandao_solidity_version_test.sol)
+
+## Recommendation
+
+Either, use versions `0.8.19`` and below, or EVM versions below shanghai

docs/ecrecover.md

@@ -12,7 +12,8 @@
 
 ### Potential Improvement
 
-As for now, the detector might not work on asm level.
+- As for now, the detector might not work on asm level.
+- `ecrecover_negative5` in [test scenarios](../tests/ecrecover.sol) should be found since the result of the check is unused. However, currently such scenarious are not covered.
 
 ## Vulnerable Scenario
 

docs/potential_arith_overflow.md

@@ -0,0 +1,21 @@
+# Potential arithmetic overflow
+
+## Configuration
+* Check: `pess-potential-arithmetic-overflow`
+* Severity: `Medium`
+* Confidence: `Medium`
+
+## Description
+The detector sees if there are assignments/returns that calculate some arithmetic expressions and if some intermediate calculations
+contain a type that is lower than the expected result. Such behavior may lead to unexpected overflow/underflow, e.g., trying to assign the multiplication of two `uint48` variables to `uint256` would look like `uint48 * uint48` and it may overflow (however, the final type would fit such multiplication).
+
+### Potential Improvement
+- Handle return statements that return tuples of arithmetic expressions;
+- Handle signed integer underflow for subtraction operation e.g. `int256 = int64 - int64` should produce error (`int64` should be cast to `int256`).
+- Improve the output of the detector (unroll complex expressions in something readable, not just `...`)
+
+## Vulnerable Scenario
+[test scenario](../tests/potential_arith_overflow.sol)
+
+## Recommendation
+Use explicit type casting in sub expressions when the assigment to a larger type is performed.

docs/strange_setter.md

@@ -6,15 +6,15 @@
 * Confidence: `Medium`
 
 ## Description
-The detector sees if a contract contains a setter (also constructor) that does not change contract storage variables.
-Setter functions MUST change the values of storage variables.
-Setter functions that do not modify storage variables may lead to contract misfunctions.
+The detector sees if a contract contains a setter (also constructor) that does not change contract storage variables or does not perform external calls using provided arguments.
+Setter functions MUST change the values of storage variables or perform external calls using provided parameters.
+Setter functions that do not use provided variables may lead to contract misfunctions.
 
 ### Potential Improvement
-Remove highlights of mappings.
+Detect shadowing before storage update/external call
 
 ## Vulnerable Scenario
 [test scenario](../tests/strange_setter_test.sol)
 
 ## Recommendation
-Make sure that setter functions modify the states of storage variables.
+Make sure that setter functions modify the states of storage variables or performs external call using provided arguments.

setup.py

@@ -23,7 +23,7 @@
     packages=find_packages(),
     license="AGPL-3.0",
     python_requires=">=3.8",
-    install_requires=["slither-analyzer>=0.9.3"],
+    install_requires=["slither-analyzer>=0.10.0"],
     extras_requires={
         "dev": ["twine>=4.0.2"],
     },

slitherin/__init__.py

@@ -23,9 +23,22 @@
 from slitherin.detectors.ecrecover import Ecrecover
 from slitherin.detectors.public_vs_external import PublicVsExternal
 from slitherin.detectors.aave.flashloan_callback import AAVEFlashloanCallbackDetector
+from slitherin.detectors.arbitrum.arbitrum_prevrandao_difficulty import (
+    ArbitrumPrevrandaoDifficulty,
+)
+from slitherin.detectors.arbitrum.solidity_version import ArbitrumSolidityVersion
+from slitherin.detectors.arbitrum.block_number_timestamp import (
+    ArbitrumBlockNumberTimestamp,
+)
+from slitherin.detectors.potential_arith_overflow import PotentialArithmOverflow
 
+artbitrum_detectors = [
+    ArbitrumPrevrandaoDifficulty,
+    ArbitrumSolidityVersion,
+    ArbitrumBlockNumberTimestamp,
+]
 
-plugin_detectors = [
+plugin_detectors = artbitrum_detectors + [
     DoubleEntryTokenPossiblity,
     UnprotectedSetter,
     NftApproveWarning,
@@ -49,6 +62,7 @@
     Ecrecover,
     PublicVsExternal,
     AAVEFlashloanCallbackDetector,
+    PotentialArithmOverflow
 ]
 plugin_printers = []
 

slitherin/cli.py

@@ -5,17 +5,22 @@
 import os
 import shutil
 import pty
+import sys
 from pathlib import Path
 import slitherin
 from pkg_resources import iter_entry_points
 
-SLITHERIN_VERSION = "0.5.0"
+from .consts import *
 
 
 def slitherin_detectors_list_as_arguments() -> str:
     return ",".join([detector.ARGUMENT for detector in slitherin.plugin_detectors])
 
 
+def arbitrum_detectors_list_as_arguments() -> str:
+    return ",".join([detector.ARGUMENT for detector in slitherin.artbitrum_detectors])
+
+
 logging.basicConfig()
 LOGGER = logging.getLogger("slitherinLogger")
 LOGGER.setLevel(logging.INFO)
@@ -47,7 +52,34 @@ def run(
         subprocess_cwd,
     )
 
-    pty.spawn(cmd, read)  # this allows to print continuously and with colors
+    master, slave = pty.openpty()
+
+    try:
+        process = subprocess.Popen(cmd, stdout=slave, stderr=slave, text=False)
+        os.close(slave)
+
+        while True:
+            try:
+                output = os.read(master, 1024)
+            except OSError as e:
+                if e.errno == 5:  # Errno 5 corresponds to Input/output error
+                    break
+                else:
+                    raise
+            if not output:
+                break
+            decoded_output = output.decode(sys.stdout.encoding, errors="replace")
+            print(decoded_output, end="")
+
+        return_code = process.wait()
+        if return_code != 0:
+            raise Exception(
+                f"Errored out with code: {return_code}, while running slither"
+            )
+
+    except Exception as e:
+        print(f"Failed to run slither: {str(e)}")
+        raise e
 
 
 def handle_list() -> None:
@@ -61,7 +93,11 @@ def handle_list() -> None:
 
 def handle_parser(args: argparse.Namespace, slither_args) -> None:
     slitherin_detectors = slitherin_detectors_list_as_arguments()
-    slither_with_args = ["slither"] + slither_args
+    slither_with_args = [
+        "slither",
+        "--fail-none",
+    ] + slither_args  # using fail-none flag, so slither will return 0 even though there are findings
+
     if args.pess:
         run(
             slither_with_args + ["--detect", slitherin_detectors],
@@ -79,6 +115,11 @@ def handle_parser(args: argparse.Namespace, slither_args) -> None:
         run(
             slither_with_args + ["--ignore-compile", "--detect", slitherin_detectors],
         )
+    elif args.arbitrum:
+        os.environ["SLITHERIN_ARBITRUM"] = "True"
+        run(slither_with_args + ["--detect", arbitrum_detectors_list_as_arguments()])
+        del os.environ["SLITHERIN_ARBITRUM"]
+
     else:
         run(slither_with_args)
 
@@ -120,6 +161,12 @@ def generate_argument_parser() -> argparse.ArgumentParser:
         action="store_true",
         help="Run slither detectors, then slitherin",
     )
+
+    parser.add_argument(
+        "--arbitrum",
+        action="store_true",
+        help="Run arbitrum detectors",
+    )
     return parser
 
 

slitherin/consts.py

@@ -0,0 +1,2 @@
+ARBITRUM_KEY = "SLITHERIN_ARBITRUM"
+SLITHERIN_VERSION = "0.6.0"