slither icon indicating copy to clipboard operation
slither copied to clipboard
verified publisher icon crytic

[Bug-Candidate]: Failed to convert IR to SSA with nested try/catch

Open 0xCSMNT opened this issue 7 months ago • 4 comments

Describe the issue:

I added a new feature to a function to burn tokens within a try/catch. The project is pre-release and currently private, but I can add someone from the team if you want to reproduce the error.

Code example to reproduce the issue:

if (force) {
    try getBalance(component) returns (uint256 bal) {
        if (bal > 0) {
            try sendToDeadAddress(component, bal) returns (bool) {} catch {}
        }
    } catch {}
}

Version:

0.11.3

Relevant log output:

➜  project-name git:(test-owner-attack) slither .        
'forge clean' running (wd: /Users/username/Documents/Projects/folder/project-name)
'forge config --json' running
'forge build --build-info --skip */test/** */script/** --force' running (wd: /Users/username/Documents/Projects/folder/project-name)
ERROR:SlitherSolcParsing:
Failed to convert IR to SSA for Node contract. Please open an issue https://github.com/crytic/slither/issues.
 
Traceback (most recent call last):
  File "/opt/homebrew/bin/slither", line 8, in <module>
    sys.exit(main())
             ^^^^^^
  File "/opt/homebrew/lib/python3.11/site-packages/slither/__main__.py", line 776, in main
    main_impl(all_detector_classes=detectors, all_printer_classes=printers)
  File "/opt/homebrew/lib/python3.11/site-packages/slither/__main__.py", line 882, in main_impl
    ) = process_all(filename, args, detector_classes, printer_classes)
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/lib/python3.11/site-packages/slither/__main__.py", line 107, in process_all
    ) = process_single(compilation, args, detector_classes, printer_classes)
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/lib/python3.11/site-packages/slither/__main__.py", line 80, in process_single
    slither = Slither(target, ast_format=ast, **vars(args))
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/lib/python3.11/site-packages/slither/slither.py", line 202, in __init__
    self._init_parsing_and_analyses(kwargs.get("skip_analyze", False))
  File "/opt/homebrew/lib/python3.11/site-packages/slither/slither.py", line 221, in _init_parsing_and_analyses
    raise e
  File "/opt/homebrew/lib/python3.11/site-packages/slither/slither.py", line 217, in _init_parsing_and_analyses
    parser.analyze_contracts()
  File "/opt/homebrew/lib/python3.11/site-packages/slither/solc_parsing/slither_compilation_unit_solc.py", line 593, in analyze_contracts
    self._convert_to_slithir()
  File "/opt/homebrew/lib/python3.11/site-packages/slither/solc_parsing/slither_compilation_unit_solc.py", line 835, in _convert_to_slithir
    raise e
  File "/opt/homebrew/lib/python3.11/site-packages/slither/solc_parsing/slither_compilation_unit_solc.py", line 830, in _convert_to_slithir
    contract.convert_expression_to_slithir_ssa()
  File "/opt/homebrew/lib/python3.11/site-packages/slither/core/declarations/contract.py", line 1571, in convert_expression_to_slithir_ssa
    func.generate_slithir_ssa(all_ssa_state_variables_instances)
  File "/opt/homebrew/lib/python3.11/site-packages/slither/core/declarations/function_contract.py", line 140, in generate_slithir_ssa
    add_ssa_ir(self, all_ssa_state_variables_instances)
  File "/opt/homebrew/lib/python3.11/site-packages/slither/slithir/utils/ssa.py", line 206, in add_ssa_ir
    fix_phi_rvalues_and_storage_ref(
  File "/opt/homebrew/lib/python3.11/site-packages/slither/slithir/utils/ssa.py", line 526, in fix_phi_rvalues_and_storage_ref
    fix_phi_rvalues_and_storage_ref(
  File "/opt/homebrew/lib/python3.11/site-packages/slither/slithir/utils/ssa.py", line 526, in fix_phi_rvalues_and_storage_ref
    fix_phi_rvalues_and_storage_ref(
  File "/opt/homebrew/lib/python3.11/site-packages/slither/slithir/utils/ssa.py", line 526, in fix_phi_rvalues_and_storage_ref
    fix_phi_rvalues_and_storage_ref(
  [Previous line repeated 9 more times]
  File "/opt/homebrew/lib/python3.11/site-packages/slither/slithir/utils/ssa.py", line 496, in fix_phi_rvalues_and_storage_ref
    variables = [
                ^
  File "/opt/homebrew/lib/python3.11/site-packages/slither/slithir/utils/ssa.py", line 497, in <listcomp>
    last_name(dst, ir.lvalue, init_local_variables_instances) for dst in ir.nodes
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/lib/python3.11/site-packages/slither/slithir/utils/ssa.py", line 363, in last_name
    assert candidates
AssertionError

0xCSMNT avatar May 20 '25 23:05 0xCSMNT

Hi, i cannot find a way to reproduce the issue. If you are able can you post a minimal proof of concept that triggers the issue otherwise you can add me to the project and i can take a look. Thanks.

smonicas avatar May 28 '25 08:05 smonicas

Hi @smonicas thanks for your help with this. I created a minimal PoC to recreate the issue. The try catch alone was not enough to trigger it so I pulled in the least possible logic from the actual contract to this: https://github.com/0xCSMNT/slither-issue-poc

0xCSMNT avatar May 29 '25 03:05 0xCSMNT

@0xCSMNT the issue should be fixed with https://github.com/crytic/slither/pull/2730 can you try it in your project and let me know if it works?

smonicas avatar Jun 03 '25 18:06 smonicas

Hi @smonicas

I ran the patched version and slither . succeeded. Thanks for all your help with this. Much appreciated

Output

slither-issue git:(main) ✗ slither .
'forge clean' running (wd: /Users/cormacdaly/Documents/Projects/slither-issue)
'forge config --json' running
'forge build --build-info --skip */test/** */script/** --force' running (wd: /Users/cormacdaly/Documents/Projects/slither-issue)
INFO:Detectors:
IssuePoC.removeComponent(address,bool) (src/IssuePoC.sol#53-83) ignores return value by IERC20(component).transfer(0x000000000000000000000000000000000000dEaD,balance) (src/IssuePoC.sol#63-64)
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#unchecked-transfer
INFO:Detectors:
Reentrancy in IssuePoC.removeComponent(address,bool) (src/IssuePoC.sol#53-83):
        External calls:
        - IERC20(component).transfer(0x000000000000000000000000000000000000dEaD,balance) (src/IssuePoC.sol#63-64)
        State variables written after the call(s):
        - delete componentAllocations[component] (src/IssuePoC.sol#79)
        IssuePoC.componentAllocations (src/IssuePoC.sol#12) can be used in cross function reentrancies:
        - IssuePoC.addComponent(address) (src/IssuePoC.sol#45-49)
        - IssuePoC.removeComponent(address,bool) (src/IssuePoC.sol#53-83)
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#reentrancy-vulnerabilities-1
INFO:Detectors:
Reentrancy in IssuePoC.removeComponent(address,bool) (src/IssuePoC.sol#53-83):
        External calls:
        - IERC20(component).transfer(0x000000000000000000000000000000000000dEaD,balance) (src/IssuePoC.sol#63-64)
        State variables written after the call(s):
        - components[i] = components[length - 1] (src/IssuePoC.sol#75)
        - components.pop() (src/IssuePoC.sol#78)
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#reentrancy-vulnerabilities-2
INFO:Detectors:
2 different versions of Solidity are used:
        - Version constraint ^0.8.20 is used by:
                -^0.8.20 (lib/openzeppelin-contracts/contracts/token/ERC20/IERC20.sol#4)
        - Version constraint ^0.8.0 is used by:
                -^0.8.0 (src/IssuePoC.sol#2)
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#different-pragma-directives-are-used
INFO:Detectors:
IssuePoC.removeComponent(address,bool) (src/IssuePoC.sol#53-83) has costly operations inside a loop:
        - components.pop() (src/IssuePoC.sol#78)
IssuePoC.removeComponent(address,bool) (src/IssuePoC.sol#53-83) has costly operations inside a loop:
        - delete componentAllocations[component] (src/IssuePoC.sol#79)
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#costly-operations-inside-a-loop
INFO:Detectors:
Version constraint ^0.8.20 contains known severe issues (https://solidity.readthedocs.io/en/latest/bugs.html)
        - VerbatimInvalidDeduplication
        - FullInlinerNonExpressionSplitArgumentEvaluationOrder
        - MissingSideEffectsOnSelectorAccess.
It is used by:
        - ^0.8.20 (lib/openzeppelin-contracts/contracts/token/ERC20/IERC20.sol#4)
Version constraint ^0.8.0 contains known severe issues (https://solidity.readthedocs.io/en/latest/bugs.html)
        - FullInlinerNonExpressionSplitArgumentEvaluationOrder
        - MissingSideEffectsOnSelectorAccess
        - AbiReencodingHeadOverflowWithStaticArrayCleanup
        - DirtyBytesArrayToStorage
        - DataLocationChangeInInternalOverride
        - NestedCalldataArrayAbiReencodingSizeValidation
        - SignedImmutables
        - ABIDecodeTwoDimensionalArrayMemory
        - KeccakCaching.
It is used by:
        - ^0.8.0 (src/IssuePoC.sol#2)
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#incorrect-versions-of-solidity
INFO:Detectors:
IssuePoC.rebalanceWindow (src/IssuePoC.sol#16) should be constant 
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#state-variables-that-could-be-declared-constant
INFO:Detectors:
IssuePoC.lastRebalance (src/IssuePoC.sol#14) should be immutable 
IssuePoC.owner (src/IssuePoC.sol#8) should be immutable 
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#state-variables-that-could-be-declared-immutable
INFO:Slither:. analyzed (2 contracts with 100 detectors), 11 result(s) found

0xCSMNT avatar Jun 05 '25 17:06 0xCSMNT