Skip to content

Commit

Permalink
Merge pull request #114 from pessimistic-io/add-arb-prevrandao-diffic…
Browse files Browse the repository at this point in the history
…ulty-detector

Arbitrum detectors
  • Loading branch information
ndkirillov authored Jan 22, 2024
2 parents 3184552 + ae09548 commit 7151098
Show file tree
Hide file tree
Showing 14 changed files with 477 additions and 3 deletions.
19 changes: 19 additions & 0 deletions docs/arb_block_number_timestamp.md
Original file line number Diff line number Diff line change
@@ -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
15 changes: 15 additions & 0 deletions docs/arb_difficulty_randao.md
Original file line number Diff line number Diff line change
@@ -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.
19 changes: 19 additions & 0 deletions docs/arb_solidity_version.md
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"],
},
Expand Down
14 changes: 13 additions & 1 deletion slitherin/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,21 @@
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,
)

artbitrum_detectors = [
ArbitrumPrevrandaoDifficulty,
ArbitrumSolidityVersion,
ArbitrumBlockNumberTimestamp,
]

plugin_detectors = [
plugin_detectors = artbitrum_detectors + [
DoubleEntryTokenPossiblity,
UnprotectedSetter,
NftApproveWarning,
Expand Down
17 changes: 16 additions & 1 deletion slitherin/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,17 @@
import slitherin
from pkg_resources import iter_entry_points

SLITHERIN_VERSION = "0.5.1"
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)
Expand Down Expand Up @@ -111,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)

Expand Down Expand Up @@ -152,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


Expand Down
2 changes: 2 additions & 0 deletions slitherin/consts.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
ARBITRUM_KEY = "SLITHERIN_ARBITRUM"
SLITHERIN_VERSION = "0.5.1"
81 changes: 81 additions & 0 deletions slitherin/detectors/arbitrum/arbitrum_prevrandao_difficulty.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
import os
from typing import List
from slither.utils.output import Output
from slither.core.cfg.node import Node
from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification
from slither.core.declarations import Function
from slither.slithir.operations.event_call import EventCall
from slither.analyses.data_dependency.data_dependency import is_dependent
from slither.slithir.operations import SolidityCall
from slither.core.declarations.solidity_variables import (
SolidityVariableComposed,
SolidityFunction,
)

from ...consts import ARBITRUM_KEY


class ArbitrumPrevrandaoDifficulty(AbstractDetector):
"""
Sees if `prevRandao` or `difficulty` is used inside an Arbitrum contract (as they return constant `1` in Arbitrum)
"""

ARGUMENT = "pess-arb-prevrandao-difficulty" # slither will launch the detector with slither.py --detect mydetector
HELP = (
"PrevRandao or difficulty is used in contract that will be deployed to Arbitrum"
)
IMPACT = DetectorClassification.MEDIUM
CONFIDENCE = DetectorClassification.HIGH

WIKI = "https://github.com/pessimistic-io/slitherin/blob/master/docs/arb_difficulty_randao.md"
WIKI_TITLE = "Usage of prevRandao/difficulty inside the Arbitrum contract"
WIKI_DESCRIPTION = "Arbitrum prevRandao/difficulty"
WIKI_EXPLOIT_SCENARIO = "N/A"
WIKI_RECOMMENDATION = (
"Do not use prevRandao/difficulty inside the code of an Arbitrum contract"
)

def _find_randao_or_difficulty(self, f: Function) -> List[Node]:
ret = set()
for node in f.nodes:
for var in node.variables_read:
if is_dependent(
var, SolidityVariableComposed("block.prevrandao"), node
) or is_dependent(
var, SolidityVariableComposed("block.difficulty"), node
):
ret.add(node)
for ir in node.irs:
if (
isinstance(ir, SolidityCall)
and ir.function == SolidityFunction("prevrandao()")
or isinstance(ir, SolidityCall)
and ir.function == SolidityFunction("difficulty()")
):
ret.add(node)
return list(ret)

def _detect(self) -> List[Output]:
"""Main function"""
results = []

if os.getenv(ARBITRUM_KEY) is None:
return results

for contract in self.compilation_unit.contracts_derived:
for f in contract.functions_and_modifiers:
nodes = self._find_randao_or_difficulty(f)
if nodes:
info = [
f,
" function uses prevRandao/difficulty inside the code of the Arbitrum contract\n",
"\tDangerous usages:\n",
]

nodes.sort(key=lambda x: x.node_id)

for node in nodes:
info += ["\t- ", node, "\n"]

results.append(self.generate_result(info))
return results
81 changes: 81 additions & 0 deletions slitherin/detectors/arbitrum/block_number_timestamp.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
import os
from typing import List
from slither.utils.output import Output
from slither.core.cfg.node import Node
from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification
from slither.core.declarations import Function
from slither.slithir.operations.event_call import EventCall
from slither.analyses.data_dependency.data_dependency import is_dependent
from slither.slithir.operations import SolidityCall
from slither.core.declarations.solidity_variables import (
SolidityVariableComposed,
SolidityFunction,
)

from ...consts import ARBITRUM_KEY


class ArbitrumBlockNumberTimestamp(AbstractDetector):
"""
Sees if `block.number` or `block.timtestamp` is used inside an Arbitrum contract
"""

ARGUMENT = "pess-arb-block-number-timestamp" # slither will launch the detector with slither.py --detect mydetector
HELP = "`block.number` or `block.timtestamp` is used inside an Arbitrum contract"
IMPACT = DetectorClassification.LOW
CONFIDENCE = DetectorClassification.HIGH

WIKI = "https://github.com/pessimistic-io/slitherin/blob/master/docs/arb_block_number_timestamp.md"
WIKI_TITLE = (
"Usage of `block.number` or `block.timtestamp` inside the Arbitrum contract"
)
WIKI_DESCRIPTION = "# Arbitrum block.number/block.timestamp"
WIKI_EXPLOIT_SCENARIO = "N/A"
WIKI_RECOMMENDATION = "Look at docs for details"

def _find_randao_or_difficulty(self, f: Function) -> List[Node]:
ret = set()
for node in f.nodes:
for var in node.variables_read:
if is_dependent(
var, SolidityVariableComposed("block.number"), node
) or is_dependent(
var, SolidityVariableComposed("block.timestamp"), node
):
ret.add(node)
for ir in node.irs:
if (
isinstance(ir, SolidityCall)
and ir.function == SolidityFunction("number()")
or isinstance(ir, SolidityCall)
and ir.function == SolidityFunction("timestamp()")
):
ret.add(node)
return list(ret)

def _detect(self) -> List[Output]:
"""Main function"""
results = []

if os.getenv(ARBITRUM_KEY) is None:
return results

for contract in self.compilation_unit.contracts_derived:
for f in contract.functions_and_modifiers:
nodes = self._find_randao_or_difficulty(f)
if nodes:
info = [
f,
" function uses block.number/block.timestamp inside the code of the Arbitrum contract\n"
"They behave different than on Ethereum, for details: (https://docs.arbitrum.io/for-devs/concepts/differences-between-arbitrum-ethereum/block-numbers-and-time)\n",
"Verify, that contract's logic does not break because of these differences\n"
"\t Usages:\n",
]

nodes.sort(key=lambda x: x.node_id)

for node in nodes:
info += ["\t- ", node, "\n"]

results.append(self.generate_result(info))
return results
61 changes: 61 additions & 0 deletions slitherin/detectors/arbitrum/solidity_version.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import os
from typing import List
from slither.utils.output import Output
from slither.core.cfg.node import Node
from slither.detectors.abstract_detector import (
AbstractDetector,
DetectorClassification,
make_solc_versions,
)
from slither.core.declarations import Function
from slither.slithir.operations.event_call import EventCall
from slither.analyses.data_dependency.data_dependency import is_dependent
from slither.slithir.operations import SolidityCall
from slither.core.declarations.solidity_variables import (
SolidityVariableComposed,
SolidityFunction,
)

from ...consts import ARBITRUM_KEY


class ArbitrumSolidityVersion(AbstractDetector):
"""
Checks that sol version >= 0.8.20 is not used inside an Arbitrum contract
"""

ARGUMENT = "pess-arb-solidity-version" # slither will launch the detector with slither.py --detect mydetector
HELP = "sol version >= 0.8.20 is used in contract that will be deployed to Arbitrum (Potential usage of PUSH0, which is not supported)"
IMPACT = DetectorClassification.HIGH
CONFIDENCE = DetectorClassification.LOW

WIKI = "https://github.com/pessimistic-io/slitherin/blob/master/docs/arb_difficulty_randao.md"
WIKI_TITLE = "Arbitrum solidity version"
WIKI_DESCRIPTION = "Potential usage of PUSH0 opcode"
WIKI_EXPLOIT_SCENARIO = "N/A"
WIKI_RECOMMENDATION = (
"Either, use versions 0.8.19 and below, or EVM versions below shanghai"
)

def _detect(self) -> List[Output]:
"""Main function"""
results = []

if os.getenv(ARBITRUM_KEY) is None:
return results

if (
self.compilation_unit.is_solidity
and self.compilation_unit.solc_version in make_solc_versions(8, 20, 99)
):
results.append(
self.generate_result(
[
"Potential usage of solidity version >= 0.8.20 detected. Solidity in these versions will utilize PUSH0 opcode, ",
"which is not supported on Arbitrum. ",
"Either, use versions 0.8.19 and below, or EVM versions below shanghai",
]
)
)

return results
Loading

0 comments on commit 7151098

Please sign in to comment.