[Bug-Candidate]: Ternary operation not handled

[james-toussaint]

Describe the issue:

Slither in Docker fails to analyze project when following line is introduced in contract

import {MessageHashUtils} from "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol"

Error is Ternary operation not handled string(<class 'slither.core.expressions.elementary_type_name_expression.ElementaryTypeNameExpression'>)

Code example to reproduce the issue:

• To reproduce

1. Clone https://github.com/jeremyjams/hardhat-slither
2. Run slither analysis with docker

docker run --rm --entrypoint /bin/bash -v $(pwd):/share trailofbits/eth-security-toolbox -c 'cd /share && slither .'

• Code causing the issue

// SPDX-License-Identifier: Apache-2.0
pragma solidity ^0.8.20;

// Slither analysis works only if the following line is removed
import {MessageHashUtils} from "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol";

contract Lock {
    uint public unlockTime;
    address payable public owner;

    event Withdrawal(uint amount, uint when);

    constructor(uint _unlockTime) payable {
        require(
            block.timestamp < _unlockTime,
            "Unlock time should be in the future"
        );

        unlockTime = _unlockTime;
        owner = payable(msg.sender);
    }

    function withdraw() public {
        // Uncomment this line, and the import of "hardhat/console.sol", to print a log in your terminal
        // console.log("Unlock time is %o and block timestamp is %o", unlockTime, block.timestamp);

        require(block.timestamp >= unlockTime, "You can't withdraw yet");
        require(msg.sender == owner, "You aren't the owner");

        emit Withdrawal(address(this).balance, block.timestamp);

        owner.transfer(address(this).balance);
    }
}

Version:

0.8.3

Relevant log output:

- With `MessageHashUtils` import

Traceback (most recent call last):
  File "/home/ethsec/.local/lib/python3.6/site-packages/slither/__main__.py", line 744, in main_impl
    ) = process_all(filename, args, detector_classes, printer_classes)
  File "/home/ethsec/.local/lib/python3.6/site-packages/slither/__main__.py", line 87, in process_all
    ) = process_single(compilation, args, detector_classes, printer_classes)
  File "/home/ethsec/.local/lib/python3.6/site-packages/slither/__main__.py", line 70, in process_single
    slither = Slither(target, ast_format=ast, **vars(args))
  File "/home/ethsec/.local/lib/python3.6/site-packages/slither/slither.py", line 118, in __init__
    parser.parse_contracts()
  File "/home/ethsec/.local/lib/python3.6/site-packages/slither/solc_parsing/slither_compilation_unit_solc.py", line 489, in parse_contracts
    self._analyze_third_part(contracts_to_be_analyzed, libraries)
  File "/home/ethsec/.local/lib/python3.6/site-packages/slither/solc_parsing/slither_compilation_unit_solc.py", line 596, in _analyze_third_part
    self._analyze_variables_modifiers_functions(contract)
  File "/home/ethsec/.local/lib/python3.6/site-packages/slither/solc_parsing/slither_compilation_unit_solc.py", line 669, in _analyze_variables_modifiers_functions
    contract.analyze_content_functions()
  File "/home/ethsec/.local/lib/python3.6/site-packages/slither/solc_parsing/declarations/contract.py", line 404, in analyze_content_functions
    function_parser.analyze_content()
  File "/home/ethsec/.local/lib/python3.6/site-packages/slither/solc_parsing/declarations/function.py", line 310, in analyze_content
    self._filter_ternary()
  File "/home/ethsec/.local/lib/python3.6/site-packages/slither/solc_parsing/declarations/function.py", line 1346, in _filter_ternary
    st = SplitTernaryExpression(node.expression)
  File "/home/ethsec/.local/lib/python3.6/site-packages/slither/utils/expression_manipulations.py", line 53, in __init__
    self.copy_expression(expression, self.true_expression, self.false_expression)
  File "/home/ethsec/.local/lib/python3.6/site-packages/slither/utils/expression_manipulations.py", line 120, in copy_expression
    self.copy_expression(next_expr, true_expression.called, false_expression.called)
  File "/home/ethsec/.local/lib/python3.6/site-packages/slither/utils/expression_manipulations.py", line 92, in copy_expression
    next_expr, true_expression.expression, false_expression.expression
  File "/home/ethsec/.local/lib/python3.6/site-packages/slither/utils/expression_manipulations.py", line 145, in copy_expression
    f"Ternary operation not handled {expression}({type(expression)})"
slither.exceptions.SlitherException: Ternary operation not handled string(<class 'slither.core.expressions.elementary_type_name_expression.ElementaryTypeNameExpression'>)
Error:
Ternary operation not handled string(<class 'slither.core.expressions.elementary_type_name_expression.ElementaryTypeNameExpression'>)
Please report an issue to https://github.com/crytic/slither/issues

• Without MessageHashUtils import

Lock.constructor(uint256) (contracts/Lock.sol#12-20) uses timestamp for comparisons
        Dangerous comparisons:
        - require(bool,string)(block.timestamp < _unlockTime,Unlock time should be in the future) (contracts/Lock.sol#13-16)
Lock.withdraw() (contracts/Lock.sol#22-32) uses timestamp for comparisons
        Dangerous comparisons:
        - require(bool,string)(block.timestamp >= unlockTime,You can't withdraw yet) (contracts/Lock.sol#26)
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#block-timestamp

Pragma version^0.8.20 (contracts/Lock.sol#2) necessitates a version too recent to be trusted. Consider deploying with 0.6.12/0.7.6/0.8.7
solc-0.8.20 is not recommended for deployment
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#incorrect-versions-of-solidity

withdraw() should be declared external:
        - Lock.withdraw() (contracts/Lock.sol#22-32)
Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#public-function-that-could-be-declared-external
. analyzed (1 contracts with 78 detectors), 5 result(s) found

[0xalpharush]

Please upgrade your slither version with python3 -m pip install slither-analyzer --upgrade. I believe this has been fixed but I'd appreciate if you'd let us know otherwise :)

[james-toussaint]

Thank you for your reply @0xalpharush :slightly_smiling_face:

Could you explain:

1. Why the following and recommended https://github.com/crytic/slither#using-docker trailofbits/eth-security-toolbox Docker image hasn't been updated for 2 years (https://hub.docker.com/r/trailofbits/eth-security-toolbox/tags)?
2. Where is pushed the "other" image related to https://github.com/crytic/slither/blob/master/Dockerfile?
3. Why latest push of the 0.10.0 has been canceled?

From an external point of view, the combination of all these points looks weird .. :thinking:

Thanks for your reply :)

[elopez]

Hi @jeremyjams!

eth-security-toolbox hasn't seen any updates in a while, and the automated build is likely broken. The last build published contains out of date and deprecated tools, and lacks new ones like medusa. I'm actually working now to refresh the image, you should see a new version coming out soon :) The eth-security-toolbox is supposed to be more "user-friendly" (e.g. by including node, solc-select, shell completions, documentation, etc) and better suited for interactive use -- to the cost of a larger image vs the slither-only image.

The slither-only Dockerfile you see in this repository is pushed to the GitHub Container Registry corresponding to this repository, you can access it on the "Packages" section in the repository homepage, on the right sidebar:

I don't know why GitHub shows the status as cancelled; it's probably showing the status of the branch push, but the tag push ran correctly as you can see on the runs list for the Docker workflow.

[james-toussaint]

Thank you @elopez for the explanation and pointers :pray:

And good luck for the future refreshing of the toolbox :muscle:!