slither icon indicating copy to clipboard operation
slither copied to clipboard

[Bug-Candidate]: TypeError: argument of type 'int' is not iterable

Open rappie opened this issue 3 years ago • 12 comments

Describe the issue:

Slither crashes.

Commands used:

yarn hardhat clean
slither .

Software used: Yarn Berry (PnP), with old versions of hardhat, openzeppelin, etc. Slither version 0.8.3 Python version 3.10.5 OS: Manjaro Linux (arch)

Error:

❯ yarn hardhat clean; slither .
'yarn hardhat compile --force' running
Compiled 2 Solidity files successfully

Traceback (most recent call last):
  File "/home/rappie/.local/lib/python3.10/site-packages/slither/__main__.py", line 744, in main_impl
    ) = process_all(filename, args, detector_classes, printer_classes)
  File "/home/rappie/.local/lib/python3.10/site-packages/slither/__main__.py", line 87, in process_all
    ) = process_single(compilation, args, detector_classes, printer_classes)
  File "/home/rappie/.local/lib/python3.10/site-packages/slither/__main__.py", line 70, in process_single
    slither = Slither(target, ast_format=ast, **vars(args))
  File "/home/rappie/.local/lib/python3.10/site-packages/slither/slither.py", line 95, in __init__
    parser.parse_top_level_from_loaded_json(ast, path)
  File "/home/rappie/.local/lib/python3.10/site-packages/slither/solc_parsing/slither_compilation_unit_solc.py", line 238, in parse_top_level_from_loaded_json
    _handle_import_aliases(symbol_aliases, import_directive, scope)
  File "/home/rappie/.local/lib/python3.10/site-packages/slither/solc_parsing/slither_compilation_unit_solc.py", line 50, in _handle_import_aliases
    and "name" in symbol_alias["foreign"]
TypeError: argument of type 'int' is not iterable
None
Error in .
Traceback (most recent call last):
  File "/home/rappie/.local/lib/python3.10/site-packages/slither/__main__.py", line 744, in main_impl
    ) = process_all(filename, args, detector_classes, printer_classes)
  File "/home/rappie/.local/lib/python3.10/site-packages/slither/__main__.py", line 87, in process_all
    ) = process_single(compilation, args, detector_classes, printer_classes)
  File "/home/rappie/.local/lib/python3.10/site-packages/slither/__main__.py", line 70, in process_single
    slither = Slither(target, ast_format=ast, **vars(args))
  File "/home/rappie/.local/lib/python3.10/site-packages/slither/slither.py", line 95, in __init__
    parser.parse_top_level_from_loaded_json(ast, path)
  File "/home/rappie/.local/lib/python3.10/site-packages/slither/solc_parsing/slither_compilation_unit_solc.py", line 238, in parse_top_level_from_loaded_json
    _handle_import_aliases(symbol_aliases, import_directive, scope)
  File "/home/rappie/.local/lib/python3.10/site-packages/slither/solc_parsing/slither_compilation_unit_solc.py", line 50, in _handle_import_aliases
    and "name" in symbol_alias["foreign"]
TypeError: argument of type 'int' is not iterable

Code example to reproduce the issue:

https://github.com/rappie/slither-issue1319-environment https://github.com/rappie/slither-issue1319-narrowed-down

Version:

0.8.3

Relevant log output:

No response

rappie avatar Aug 01 '22 15:08 rappie

Can you put this on github?

0xalpharush avatar Aug 01 '22 15:08 0xalpharush

Sure :)

https://github.com/rappie/slither-issue1319-environment

rappie avatar Aug 01 '22 15:08 rappie

I am having same issue. Seems like the problem is here:

https://github.com/crytic/slither/blob/ce9dbf650d7acfee51ddafd844929f4e8d345672/slither/solc_parsing/slither_compilation_unit_solc.py#L47-L53

And symbol_alias["foreign"] is 2:

[{'foreign': 2, 'local': None}]

Would it be possible to define types for some of these AST nodes, so they're not just Dicts? Would be possible to catch a lot of these errors that way.

Does anyone have a link to the json AST schema? I can't seem to find it anywhere, just typescript libs like this: https://github.com/OpenZeppelin/solidity-ast .

KholdStare avatar Aug 27 '22 04:08 KholdStare

Any progress?

I have tried narrowing it down as much as possible. It looks it has something to do with the compiler version.

With 0.5.11 it fails, with 0.8.12 it works.

You can test this easily in the repo below by editing hardhat.config.js. I've also cleaned up the code as much as possible. It's just 2 interfaces now.

https://github.com/rappie/slither-issue1319-narrowed-down

rappie avatar Aug 30 '22 12:08 rappie

By any chance, you tested using slither and crytic-compile from their respective master branches?

gustavo-grieco avatar Aug 30 '22 12:08 gustavo-grieco

Some problem.

What I did:

  • First uninstall with with pip uninstall <package>
  • Clone repo
  • Run sudo python setup.py install

I did this for both slither and crytic-compile.

❯ crytic-compile --version
0.2.3
❯ slither --version
0.8.3

Here's the error again, with the new line numbers:

❯ slither .
'npx hardhat compile --force' running
Compiled 2 Solidity files successfully

contracts/IERC20.sol:1:1: Warning: Source file does not specify required compiler version! Consider adding "pragma solidity ^0.5.11;"
interface IERC20 {}
^ (Relevant source part starts here and spans across multiple lines).

contracts/InitializableERC20Detailed.sol:1:1: Warning: Source file does not specify required compiler version! Consider adding "pragma solidity ^0.5.11;"
import {IERC20} from "./IERC20.sol";
^ (Relevant source part starts here and spans across multiple lines).


Traceback (most recent call last):
  File "/usr/lib/python3.10/site-packages/slither_analyzer-0.8.3-py3.10.egg/slither/__main__.py", line 793, in main_impl
    ) = process_all(filename, args, detector_classes, printer_classes)
  File "/usr/lib/python3.10/site-packages/slither_analyzer-0.8.3-py3.10.egg/slither/__main__.py", line 97, in process_all
    ) = process_single(compilation, args, detector_classes, printer_classes)
  File "/usr/lib/python3.10/site-packages/slither_analyzer-0.8.3-py3.10.egg/slither/__main__.py", line 75, in process_single
    slither = Slither(target, ast_format=ast, **vars(args))
  File "/usr/lib/python3.10/site-packages/slither_analyzer-0.8.3-py3.10.egg/slither/slither.py", line 100, in __init__
    parser.parse_top_level_from_loaded_json(ast, path)
  File "/usr/lib/python3.10/site-packages/slither_analyzer-0.8.3-py3.10.egg/slither/solc_parsing/slither_compilation_unit_solc.py", line 238, in parse_top_level_from_loaded_json
    _handle_import_aliases(symbol_aliases, import_directive, scope)
  File "/usr/lib/python3.10/site-packages/slither_analyzer-0.8.3-py3.10.egg/slither/solc_parsing/slither_compilation_unit_solc.py", line 50, in _handle_import_aliases
    and "name" in symbol_alias["foreign"]
TypeError: argument of type 'int' is not iterable
None
Error in .
Traceback (most recent call last):
  File "/usr/lib/python3.10/site-packages/slither_analyzer-0.8.3-py3.10.egg/slither/__main__.py", line 793, in main_impl
    ) = process_all(filename, args, detector_classes, printer_classes)
  File "/usr/lib/python3.10/site-packages/slither_analyzer-0.8.3-py3.10.egg/slither/__main__.py", line 97, in process_all
    ) = process_single(compilation, args, detector_classes, printer_classes)
  File "/usr/lib/python3.10/site-packages/slither_analyzer-0.8.3-py3.10.egg/slither/__main__.py", line 75, in process_single
    slither = Slither(target, ast_format=ast, **vars(args))
  File "/usr/lib/python3.10/site-packages/slither_analyzer-0.8.3-py3.10.egg/slither/slither.py", line 100, in __init__
    parser.parse_top_level_from_loaded_json(ast, path)
  File "/usr/lib/python3.10/site-packages/slither_analyzer-0.8.3-py3.10.egg/slither/solc_parsing/slither_compilation_unit_solc.py", line 238, in parse_top_level_from_loaded_json
    _handle_import_aliases(symbol_aliases, import_directive, scope)
  File "/usr/lib/python3.10/site-packages/slither_analyzer-0.8.3-py3.10.egg/slither/solc_parsing/slither_compilation_unit_solc.py", line 50, in _handle_import_aliases
    and "name" in symbol_alias["foreign"]
TypeError: argument of type 'int' is not iterable

rappie avatar Aug 30 '22 13:08 rappie

By any chance, you tested using slither and crytic-compile from their respective master branches?

The code I highlighted earlier is from the master branch. _handle_import_aliases does not work with some ASTs it looks like. There really need to be precise types for the AST in the code. I've run into several bugs that could have been avoided if the types were not just Dict.

Most likely different compiler versions output a different json AST, and slither doesn't support them.

KholdStare avatar Aug 30 '22 16:08 KholdStare

@KholdStare Do you have any suggestions for a workaround or quickfix I can apply locally?

rappie avatar Sep 02 '22 19:09 rappie

Can you use solc 0.8.12? https://github.com/crytic/slither/issues/1319#issuecomment-1231620830

0xalpharush avatar Sep 02 '22 19:09 0xalpharush

I'm not sure, it is not my own codebase. I'm trying to reproduce a known vulnerabilty and "re-find" it with echidna. I'm not sure if updating to 0.8 changes any behaviours of the system.

I might give it a try next week, but something like a small patch to slither would be preferable.

rappie avatar Sep 02 '22 19:09 rappie

@rappie as a temporary fix can you change in slitherbug import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; to import "@openzeppelin/contracts/token/ERC20/IERC20.sol"; it should work with 0.5.11

smonicas avatar Sep 02 '22 19:09 smonicas

Awesome! This indeed works.

Thanks :smile:

rappie avatar Sep 02 '22 21:09 rappie

@smonicas @0xalpharush Hi, any acknowledgement about what I pointed out above as the bug? https://github.com/crytic/slither/issues/1319#issuecomment-1229122518

I think a lot of issues would be fixed if there were proper types for the AST. We currently cannot use Slither because of these kinds of issues - we don't want to be bending over backwards changing our code, just so that slither doesn't crash on it. I and my team can help out making fixes, but not if our comments are basically ignored.

KholdStare avatar Oct 05 '22 20:10 KholdStare

I have labeled this issue as a bug. Yes, having a typed AST would be nice; however, it is not our highest priority. We do our best to keep up with breaking changes in Solidity while maintaining backwards compatibility. There was an effort to separate the legacy and compact parsers, but it was not completed (https://github.com/crytic/slither/pull/627). I will try to fix this specific issue soon.

0xalpharush avatar Oct 05 '22 21:10 0xalpharush

@0xalpharush Thanks for the reply. I think Slither's core strength is in the detectors etc. Right now it is hindered by the parsing stage which is just a necessary step to get to the detectors. Without the parsing stage, it renders the rest of it useless, so paradoxically IMHO it should be of high priority to do parsing properly.

Taking a step back, I think there should be standalone library specifically handling AST parsing and having a single representation for all versions (not necessarily maintained by you, just in general). Slither could then use it and not have to worry about parsing. Does such a project exist in python land? It exists in typescript land: https://github.com/OpenZeppelin/solidity-ast . Any insights into who can start such a project?

KholdStare avatar Oct 06 '22 02:10 KholdStare

Hello, are there any updates on this issue, this bug also blocks us, we can't run CI solidity slighter audit jobs anymore.

slither . --truffle-build-directory build/ results with :

> Compiled successfully using:
   - solc: 0.5.16+commit.9c3226ce.Emscripten.clang

Traceback (most recent call last):
  File "/home/amine/.local/lib/python3.9/site-packages/slither/__main__.py", line 826, in main_impl
    ) = process_all(filename, args, detector_classes, printer_classes)
  File "/home/amine/.local/lib/python3.9/site-packages/slither/__main__.py", line 97, in process_all
    ) = process_single(compilation, args, detector_classes, printer_classes)
  File "/home/amine/.local/lib/python3.9/site-packages/slither/__main__.py", line 75, in process_single
    slither = Slither(target, ast_format=ast, **vars(args))
  File "/home/amine/.local/lib/python3.9/site-packages/slither/slither.py", line 102, in __init__
    parser.parse_top_level_from_loaded_json(ast, path)
  File "/home/amine/.local/lib/python3.9/site-packages/slither/solc_parsing/slither_compilation_unit_solc.py", line 238, in parse_top_level_from_loaded_json
    _handle_import_aliases(symbol_aliases, import_directive, scope)
  File "/home/amine/.local/lib/python3.9/site-packages/slither/solc_parsing/slither_compilation_unit_solc.py", line 50, in _handle_import_aliases
    and "name" in symbol_alias["foreign"]
TypeError: argument of type 'int' is not iterable
Error in .
Traceback (most recent call last):
  File "/home/amine/.local/lib/python3.9/site-packages/slither/__main__.py", line 826, in main_impl
    ) = process_all(filename, args, detector_classes, printer_classes)
  File "/home/amine/.local/lib/python3.9/site-packages/slither/__main__.py", line 97, in process_all
    ) = process_single(compilation, args, detector_classes, printer_classes)
  File "/home/amine/.local/lib/python3.9/site-packages/slither/__main__.py", line 75, in process_single
    slither = Slither(target, ast_format=ast, **vars(args))
  File "/home/amine/.local/lib/python3.9/site-packages/slither/slither.py", line 102, in __init__
    parser.parse_top_level_from_loaded_json(ast, path)
  File "/home/amine/.local/lib/python3.9/site-packages/slither/solc_parsing/slither_compilation_unit_solc.py", line 238, in parse_top_level_from_loaded_json
    _handle_import_aliases(symbol_aliases, import_directive, scope)
  File "/home/amine/.local/lib/python3.9/site-packages/slither/solc_parsing/slither_compilation_unit_solc.py", line 50, in _handle_import_aliases
    and "name" in symbol_alias["foreign"]
TypeError: argument of type 'int' is not iterable

Aminechakr avatar Oct 22 '22 11:10 Aminechakr

Based off of this issue, the AST for solc 0.5.12 is completely missing the foreign info. I don't see a workaround other than upgrading the a version of solc that contains the bug fix.

0xalpharush avatar Dec 12 '22 19:12 0xalpharush

Thanks for pinpointing it to the solc bug. Looks like the earliest solc version in which it was fixed is 0.6.0, so unfortunately upgrading solc might require some changes in the solidity code base. The following patch applied to slither seems to work around the solc bug:

diff --git a/slither/solc_parsing/slither_compilation_unit_solc.py b/slither/solc_parsing/slither_compilation_unit_solc.py
index d12bda1b..67614e84 100644
--- a/slither/solc_parsing/slither_compilation_unit_solc.py
+++ b/slither/solc_parsing/slither_compilation_unit_solc.py
@@ -47,6 +47,7 @@ def _handle_import_aliases(
     for symbol_alias in symbol_aliases:
         if (
             "foreign" in symbol_alias
+            and not isinstance(symbol_alias["foreign"], int)
             and "name" in symbol_alias["foreign"]
             and "local" in symbol_alias
         ):

If my reading is correct, this should do the right thing as long as the problematic imports are of the format

import {Foo} from "bar.sol";

and not

import {Foo as AnotherName} from "bar.sol";

(Of course, I'm not suggesting this should be applied to slither globally, since that would probably break the latter kind of imports.)

SheldonHolmgren avatar Jan 01 '23 12:01 SheldonHolmgren

We add a better error message and support import {Foo} from "bar.sol"; style imports for these older solc versions

0xalpharush avatar Jan 11 '23 17:01 0xalpharush