crytic / crytic-compile

Abstraction layer for smart contract build systems
GNU Affero General Public License v3.0
154 stars 83 forks source link

[Bug-Candidate]: Unable to use foundry out of the box with relative imports in lib #270

Closed Elyx0 closed 1 year ago

Elyx0 commented 2 years ago

Describe the issue:

Slither(foundry) chokes on libs/ with relative import inside. Command: slither .

Using:

Example slitherable minimal repo: https://github.com/Elyx0/foundry-slither-oz-choke/blob/main/src/Contract.sol#L6 See file comments on how to make unslitherable by uncommenting 2 lines of Contract.sol

Built with:

forge init
forge install openzeppelin/openzeppelin-contracts
forge remappings > remappings.txt

Error log:

'forge build --extra-output abi --extra-output userdoc --extra-output devdoc --extra-output evm.methodIdentifiers --force' running
Compiling 13 files with 0.8.13
Solc 0.8.13 finished in 1.17s
Compiler run successful (with warnings)
warning[2072]: Warning: Unused local variable.
  --> src/Contract.sol:13:9:
   |
13 |         IERC20 token = IERC20(_token); // Uncomment me as well if you uncomment above
   |         ^^^^^^^^^^^^

Traceback (most recent call last):
  File "/opt/homebrew/lib/python3.9/site-packages/slither/__main__.py", line 744, in main_impl
    ) = process_all(filename, args, detector_classes, printer_classes)
  File "/opt/homebrew/lib/python3.9/site-packages/slither/__main__.py", line 87, in process_all
    ) = process_single(compilation, args, detector_classes, printer_classes)
  File "/opt/homebrew/lib/python3.9/site-packages/slither/__main__.py", line 70, in process_single
    slither = Slither(target, ast_format=ast, **vars(args))
  File "/opt/homebrew/lib/python3.9/site-packages/slither/slither.py", line 95, in __init__
    parser.parse_top_level_from_loaded_json(ast, path)
  File "/opt/homebrew/lib/python3.9/site-packages/slither/solc_parsing/slither_compilation_unit_solc.py", line 256, in parse_top_level_from_loaded_json
    get_imported_scope = self.compilation_unit.get_scope(import_directive.filename)
  File "/opt/homebrew/lib/python3.9/site-packages/slither/core/compilation_unit.py", line 231, in get_scope
    filename = self._crytic_compile_compilation_unit.crytic_compile.filename_lookup(
  File "/opt/homebrew/lib/python3.9/site-packages/crytic_compile/crytic_compile.py", line 199, in filename_lookup
    raise ValueError(
ValueError: lib/openzeppelin-contracts/contracts/interfaces/IERC20.sol does not exist in ['/Users/me/slithfoundry/lib/forge-std/src/console2.sol', '/Users/me/slithfoundry/lib/forge-std/src/console2.sol', ...ETC ALL .SOL FILES]
None
Error in .

forge test passes correctly while in the "unslitherable" state. No issues.

Potentially related: https://github.com/crytic/slither/issues/1214 I grepped the ast and they seem to be there.

grep -nr "ast" ./out
./out/IERC20.sol/IERC20.json:234:  "ast": {
./out/Vm.sol/Vm.json:61:      "name": "broadcast",
./out/Vm.sol/Vm.json:74:      "name": "broadcast",
./out/Vm.sol/Vm.json:870:      "name": "startBroadcast",
./out/Vm.sol/Vm.json:883:      "name": "startBroadcast",
./out/Vm.sol/Vm.json:921:      "name": "stopBroadcast",
./out/Vm.sol/Vm.json:984:    "broadcast()": "afc98040",
./out/Vm.sol/Vm.json:985:    "broadcast(address)": "e6962cdb",
./out/Vm.sol/Vm.json:1027:    "startBroadcast()": "7fb5297f",
./out/Vm.sol/Vm.json:1028:    "startBroadcast(address)": "7fec2a8d",
./out/Vm.sol/Vm.json:1031:    "stopBroadcast()": "76eadd36",
./out/Vm.sol/Vm.json:1044:  "ast": {
./out/Vm.sol/Vm.json:5344:            "name": "broadcast",
./out/Vm.sol/Vm.json:5371:            "name": "broadcast",
./out/Vm.sol/Vm.json:5427:            "name": "startBroadcast",
./out/Vm.sol/Vm.json:5454:            "name": "startBroadcast",
./out/Vm.sol/Vm.json:5510:            "name": "stopBroadcast",
./out/Ownable.sol/Ownable.json:94:  "ast": {
./out/Address.sol/Address.json:23:  "ast": {
./out/Address.sol/Address.json:1301:              "text": " @dev Same as {xref-Address-functionCall-address-bytes-}[`functionCall`],\n but also transferring `value` wei to `target`.\n Requirements:\n - the calling contract must have an ETH balance of at least `value`.\n - the called Solidity function must be `payable`.\n _Available since v3.1._"
./out/console2.sol/console2.json:22:  "ast": {
./out/test.sol/stdStorage.json:99:  "ast": {
./out/test.sol/stdMath.json:22:  "ast": {
./out/test.sol/Test.json:698:  "ast": {
./out/test.sol/stdError.json:164:  "ast": {
./out/test.sol/DSTest.json:320:  "ast": {
./out/Context.sol/Context.json:23:  "ast": {
./out/Script.sol/Script.json:38:  "ast": {
./out/console.sol/console.json:22:  "ast": {
./out/Contract.sol/Greeter.json:106:  "ast": {
./out/Contract.t.sol/ContractTest.json:714:  "ast": {

out/IERC20.sol/IERC20.json only present path is

  {
  "ast": {
    "absolutePath": "lib/openzeppelin-contracts/contracts/token/ERC20/IERC20.sol",
    "id": 20898,
    "exportedSymbols": {
      "IERC20": [
        20897
      ]
    },
    "nodeType": "SourceUnit",
    "src": "106:2661:8",
    "nodes": [] // a lot
}

out/Contract.sol/Greeter.json has

      {
        "id": 21219,
        "nodeType": "ImportDirective",
        "src": "185:64:11",
        "absolutePath": "lib/openzeppelin-contracts/contracts/interfaces/IERC20.sol",
        "file": "openzeppelin-contracts/contracts/interfaces/IERC20.sol",
        "nameLocation": "-1:-1:-1",
        "scope": 21247,
        "sourceUnit": 20820,
        "symbolAliases": [],
        "unitAlias": ""
      }    

Additional Notes:

In the slitherable state (no IERC20 uncommented):

slither src/Contract.sol while trying to target a specific file chokes as well:

crytic_compile.platform.exceptions.InvalidCompilation: Invalid solc compilation Error: Source "openzeppelin-contracts/contracts/utils/Address.sol" not found: File not found. Searched the following locations: "".
 --> src/Contract.sol:4:1:
  |
4 | import "openzeppelin-contracts/contracts/utils/Address.sol";
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Error: Source "openzeppelin-contracts/contracts/access/Ownable.sol" not found: File not found. Searched the following locations: "".
 --> src/Contract.sol:5:1:
  |
5 | import "openzeppelin-contracts/contracts/access/Ownable.sol";
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Likewise for slither src/Contract.sol --compile-force-framework foundry

  File "/opt/homebrew/Cellar/python@3.9/3.9.12_1/Frameworks/Python.framework/Versions/3.9/lib/python3.9/subprocess.py", line 951, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
  File "/opt/homebrew/Cellar/python@3.9/3.9.12_1/Frameworks/Python.framework/Versions/3.9/lib/python3.9/subprocess.py", line 1821, in _execute_child
    raise child_exception_type(errno_num, err_msg, err_filename)
NotADirectoryError: [Errno 20] Not a directory: 'src/Contract.sol'
None
Error in src/Contract.sol

Out of ideas for now. Good luck!

Code example to reproduce the issue:

https://github.com/Elyx0/foundry-slither-oz-choke/blob/main/src/Contract.sol#L6

Version:

0.8.3

Relevant log output:

No response

0xalpharush commented 2 years ago

Whenever slither is used on contracts that have dependencies, it should be ran using slither . in the parent directory that contains the compilation artifacts. For reference, Foundry puts the artifacts in out/ so the parent directory contains the following: (I ran tree -L 1)

.
├── cache
├── foundry.toml
├── lib
├── out
├── remappings.txt
├── src
└── test

To ignore the test contracts and imported libraries, you can use the filter-paths flag and regex: slither . --filter-paths "lib|test"

EDIT: Nvm, I see what you mean. I suspect it has something to do with how crytic-compile or Foundry resolves this import. I got slither to work using the following patch to the codebase:

diff --git a/src/Contract.sol b/src/Contract.sol
index aca5b25..2013e60 100644
--- a/src/Contract.sol
+++ b/src/Contract.sol
@@ -3,14 +3,14 @@ pragma solidity ^0.8.0;
 import "openzeppelin-contracts/contracts/utils/Address.sol";
 import "openzeppelin-contracts/contracts/access/Ownable.sol";
-// import "openzeppelin-contracts/contracts/interfaces/IERC20.sol"; // Uncomment me to crash `slither .`
+import "openzeppelin-contracts/contracts/token/ERC20/IERC20.sol"; // Uncomment me to crash `slither .`

 contract Greeter is Ownable {
     string private greeting;
     using Address for address;
     constructor(string memory _greeting, address _token) {
         greeting = _greeting;
-       // IERC20 token = IERC20(_token); // Uncomment me as well if you uncomment above
+       IERC20 token = IERC20(_token); // Uncomment me as well if you uncomment above

Sidenote: @montyly I've never seen slither run this slowly on a small codebase. It looks like it's the similar variables detector

   Ordered by: cumulative time

   ncalls  tottime  percall  cumtime  percall filename:lineno(function)
        1    0.000    0.000   86.581   86.581 /Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/site-packages/slither/__main__.py:75(process_all)
        1    0.000    0.000   83.149   83.149 /Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/site-packages/slither/__main__.py:57(process_single)
        1    0.000    0.000   77.118   77.118 /Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/site-packages/slither/__main__.py:100(_process)
        1    0.000    0.000   77.114   77.114 /Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/site-packages/slither/slither.py:195(run_detectors)
        1    0.000    0.000   77.114   77.114 /Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/site-packages/slither/slither.py:201(<listcomp>)
      107    0.001    0.000   77.114    0.721 /Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/site-packages/slither/detectors/abstract_detector.py:148(detect)
        1    0.000    0.000   73.002   73.002 /Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/site-packages/slither/detectors/variables/similar_variables.py:70(_detect)
       14    4.989    0.356   73.002    5.214 /Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/site-packages/slither/detectors/variables/similar_variables.py:46(detect_sim)
  2648530    5.115    0.000   64.842    0.000 /Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/site-packages/slither/detectors/variables/similar_variables.py:27(similar)
  2507192    4.001    0.000   46.018    0.000 /Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/difflib.py:597(ratio)
  2507192   13.011    0.000   38.108    0.000 /Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/difflib.py:421(get_matching_blocks)
  4990563   14.629    0.000   19.207    0.000 /Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/difflib.py:305(find_longest_match)
  2507192    1.801    0.000   12.777    0.000 /Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/difflib.py:120(__init__)
  2507192    1.591    0.000   10.976    0.000 /Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/difflib.py:184(set_seqs)
  2507192    1.591    0.000    8.517    0.000 /Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/difflib.py:222(set_seq2)
  2507192    5.470    0.000    6.926    0.000 /Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/difflib.py:266(__chain_b)
pedrommaiaa commented 2 years ago

Having the same problem with the Solmate library.

crytic_compile.platform.exceptions.InvalidCompilation: Invalid solc compilation Error: Source "solmate/utils/FixedPointMathLib.sol" not found: File not found.
 --> src/Contract.sol:5:1:
  |
5 | import {FixedPointMathLib} from "solmate/utils/FixedPointMathLib.sol";
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

forge test works normally.

montyly commented 2 years ago

Thanks @Elyx0 for the minimal testcase, it was really useful.

I think that this is due to https://github.com/foundry-rs/foundry/issues/1488.

I can see in the compilation artefacts that both

Are included when https://github.com/Elyx0/foundry-slither-oz-choke is compiled. However I have no idea why forge includes lib/openzeppelin-contracts/contracts/token/ERC20/IERC20.sol.

Unfortunately, as this is a due to a bug in foundry, I am not sure that there is a lot we can do.

douglasqian commented 2 years ago

@montyly I seem to running into a similar issue with foundry NotADirectoryError

slither ./src/SpiralsCeloVault.sol --compile-force-framework foundry
target ./src/SpiralsCeloVault.sol
'forge build --extra-output abi --extra-output userdoc --extra-output devdoc --extra-output evm.methodIdentifiers --force' running
Traceback (most recent call last):
  File "/usr/local/lib/python3.9/site-packages/slither/__main__.py", line 745, in main_impl
    ) = process_all(filename, args, detector_classes, printer_classes)
  File "/usr/local/lib/python3.9/site-packages/slither/__main__.py", line 77, in process_all
    compilations = compile_all(target, **vars(args))
  File "/usr/local/lib/python3.9/site-packages/crytic_compile/crytic_compile.py", line 637, in compile_all
    compilations.append(CryticCompile(target, **kwargs))
  File "/usr/local/lib/python3.9/site-packages/crytic_compile/crytic_compile.py", line 117, in __init__
    self._compile(**kwargs)
  File "/usr/local/lib/python3.9/site-packages/crytic_compile/crytic_compile.py", line 548, in _compile
    self._platform.compile(self, **kwargs)
  File "/usr/local/lib/python3.9/site-packages/crytic_compile/platform/foundry.py", line 78, in compile
    with subprocess.Popen(
  File "/usr/local/Cellar/python@3.9/3.9.12/Frameworks/Python.framework/Versions/3.9/lib/python3.9/subprocess.py", line 951, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
  File "/usr/local/Cellar/python@3.9/3.9.12/Frameworks/Python.framework/Versions/3.9/lib/python3.9/subprocess.py", line 1821, in _execute_child
    raise child_exception_type(errno_num, err_msg, err_filename)
NotADirectoryError: [Errno 20] Not a directory: './src/SpiralsCeloVault.sol'

What's interesting is that the subprocess command forge build --extra-output abi --extra-output userdoc --extra-output devdoc --extra-output evm.methodIdentifiers --force works independently. npx hardhat compile also works, and I found that a good temporary workaround is to simply run slither ./src.

Saw that a PR was merged to resolve https://github.com/foundry-rs/foundry/issues/1488, but maybe it's because forge hasn't included this in a new release yet

adamxyzxyz commented 1 year ago

is this resolved?

0xalpharush commented 1 year ago

Slither 0.9.2 works on the OP's repo. Note, running slither on directories/contracts with dependencies may not work because often a build system like Foundry is needed. I would run slither . in the root directory of Foundry projects i.e. the same directory the foundry.toml is in. If anyone has other issues or questions, please open a new issue.