crytic / slither

Static Analyzer for Solidity and Vyper
https://blog.trailofbits.com/2018/10/19/slither-a-solidity-static-analysis-framework/
GNU Affero General Public License v3.0
5.33k stars 967 forks source link

[Bug-Candidate]: `AssertionError` in `ssa.py` #1372

Closed mds1 closed 7 months ago

mds1 commented 2 years ago

Describe the issue:

Older versions of solmate's DSTestPlus have the below block of code (it's slightly modified in newer versions, and I haven't tested those). Even though we don't use the block of code in question, it causes the error shown below.

string private checkpointLabel;
uint256 private checkpointGasLeft;

function startMeasuringGas(string memory label) internal virtual {
    checkpointLabel = label;
    checkpointGasLeft = gasleft();
}

function stopMeasuringGas() internal virtual {
    uint256 checkpointGasLeft2 = gasleft();

    string memory label = checkpointLabel;

    emit log_named_uint(string(abi.encodePacked(label, " Gas")), checkpointGasLeft - checkpointGasLeft2);
}

Logging the value of ir and variable in ssa.py's get_variable method prints the following right before the error:

ir: checkpointLabel(string) := label(string)
variable: checkpointLabel

Since we were't using the code, I was able to comment out the problematic section and use slither, though I'm not sure of other workarounds if we were using it

Code example to reproduce the issue:

https://github.com/transmissions11/solmate/blob/44a9963d4c78111f77caa0e65d677b8b46d6f2e6/src/test/utils/DSTestPlus.sol#L15-L29

Version:

0.8.3

Relevant log output:

Missing params 'startMeasuringGas(string)'
Missing params 'startMeasuringGas(string)'
Missing params 'startMeasuringGas(string)'
Missing params 'startMeasuringGas(string)'
Traceback (most recent call last):
  File "/usr/local/lib/python3.9/site-packages/slither/__main__.py", line 744, in main_impl
    ) = process_all(filename, args, detector_classes, printer_classes)
  File "/usr/local/lib/python3.9/site-packages/slither/__main__.py", line 87, in process_all
    ) = process_single(compilation, args, detector_classes, printer_classes)
  File "/usr/local/lib/python3.9/site-packages/slither/__main__.py", line 70, in process_single
    slither = Slither(target, ast_format=ast, **vars(args))
  File "/usr/local/lib/python3.9/site-packages/slither/slither.py", line 123, in __init__
    parser.analyze_contracts()
  File "/usr/local/lib/python3.9/site-packages/slither/solc_parsing/slither_compilation_unit_solc.py", line 496, in analyze_contracts
    self._convert_to_slithir()
  File "/usr/local/lib/python3.9/site-packages/slither/solc_parsing/slither_compilation_unit_solc.py", line 693, in _convert_to_slithir
    contract.convert_expression_to_slithir_ssa()
  File "/usr/local/lib/python3.9/site-packages/slither/core/declarations/contract.py", line 1301, in convert_expression_to_slithir_ssa
    func.generate_slithir_ssa(all_ssa_state_variables_instances)
  File "/usr/local/lib/python3.9/site-packages/slither/core/declarations/function_contract.py", line 110, in generate_slithir_ssa
    add_ssa_ir(self, all_ssa_state_variables_instances)
  File "/usr/local/lib/python3.9/site-packages/slither/slithir/utils/ssa.py", line 182, in add_ssa_ir
    generate_ssa_irs(
  File "/usr/local/lib/python3.9/site-packages/slither/slithir/utils/ssa.py", line 297, in generate_ssa_irs
    generate_ssa_irs(
  File "/usr/local/lib/python3.9/site-packages/slither/slithir/utils/ssa.py", line 241, in generate_ssa_irs
    new_ir = copy_ir(
  File "/usr/local/lib/python3.9/site-packages/slither/slithir/utils/ssa.py", line 670, in copy_ir
    lvalue = get_variable(ir, lambda x: x.lvalue, *instances)
  File "/usr/local/lib/python3.9/site-packages/slither/slithir/utils/ssa.py", line 630, in get_variable
    variable = get(variable, *instances)
  File "/usr/local/lib/python3.9/site-packages/slither/slithir/utils/ssa.py", line 609, in get
    assert isinstance(
AssertionError
None
Error in .
Traceback (most recent call last):
  File "/usr/local/lib/python3.9/site-packages/slither/__main__.py", line 744, in main_impl
    ) = process_all(filename, args, detector_classes, printer_classes)
  File "/usr/local/lib/python3.9/site-packages/slither/__main__.py", line 87, in process_all
    ) = process_single(compilation, args, detector_classes, printer_classes)
  File "/usr/local/lib/python3.9/site-packages/slither/__main__.py", line 70, in process_single
    slither = Slither(target, ast_format=ast, **vars(args))
  File "/usr/local/lib/python3.9/site-packages/slither/slither.py", line 123, in __init__
    parser.analyze_contracts()
  File "/usr/local/lib/python3.9/site-packages/slither/solc_parsing/slither_compilation_unit_solc.py", line 496, in analyze_contracts
    self._convert_to_slithir()
  File "/usr/local/lib/python3.9/site-packages/slither/solc_parsing/slither_compilation_unit_solc.py", line 693, in _convert_to_slithir
    contract.convert_expression_to_slithir_ssa()
  File "/usr/local/lib/python3.9/site-packages/slither/core/declarations/contract.py", line 1301, in convert_expression_to_slithir_ssa
    func.generate_slithir_ssa(all_ssa_state_variables_instances)
  File "/usr/local/lib/python3.9/site-packages/slither/core/declarations/function_contract.py", line 110, in generate_slithir_ssa
    add_ssa_ir(self, all_ssa_state_variables_instances)
  File "/usr/local/lib/python3.9/site-packages/slither/slithir/utils/ssa.py", line 182, in add_ssa_ir
    generate_ssa_irs(
  File "/usr/local/lib/python3.9/site-packages/slither/slithir/utils/ssa.py", line 297, in generate_ssa_irs
    generate_ssa_irs(
  File "/usr/local/lib/python3.9/site-packages/slither/slithir/utils/ssa.py", line 241, in generate_ssa_irs
    new_ir = copy_ir(
  File "/usr/local/lib/python3.9/site-packages/slither/slithir/utils/ssa.py", line 670, in copy_ir
    lvalue = get_variable(ir, lambda x: x.lvalue, *instances)
  File "/usr/local/lib/python3.9/site-packages/slither/slithir/utils/ssa.py", line 630, in get_variable
    variable = get(variable, *instances)
  File "/usr/local/lib/python3.9/site-packages/slither/slithir/utils/ssa.py", line 609, in get
    assert isinstance(
AssertionError
0xalpharush commented 1 year ago

I get the following message when running with disallow-partial:

Traceback (most recent call last):
  File "/Users/alpharush/tob/slither/slither/solc_parsing/declarations/contract.py", line 553, in _analyze_params_elements
    if accessible_elements[element.full_name] != all_elements[element.canonical_name]:
KeyError: 'startMeasuringGas(string)'
0xalpharush commented 1 year ago

I think this import alias with collision is the issue as if I rename the contract it works (there's a separate issue for ternaries) https://github.com/endaoment/endaoment-contracts-v2/blob/main/src/test/utils/DSTestPlus.sol#L4-L10

0xalpharush commented 1 year ago

https://github.com/crytic/slither/blob/b96beeaa5742d5d7a862e964bc72b3cab67623f4/slither/solc_parsing/slither_compilation_unit_solc.py#L407-L409 This code here doesn't account for contract name collisions and returns the same contract. This causes a contract to be set as inheriting from itself. This becomes clear if you add an assertion that none of the ancestors contract id's match the contract https://github.com/crytic/slither/blob/b96beeaa5742d5d7a862e964bc72b3cab67623f4/slither/core/declarations/contract.py#L660-L662

0xalpharush commented 7 months ago

closed by https://github.com/crytic/slither/pull/2403