A Practical Guide to Smart Contract Security Tools. Part 2: Slither

Warning

This article is NOT a rating of automated analyzers. I use them for my own contracts: deliberately add pseudo-errors and study the responses. It is not a “better-or-worse” type of research, such tasks require a blind review of a large number of contracts and actual results won’t be very precise, given the nature of this kind of software. A small mistake in a particular contract may knock out a large piece of analyzer logic, whereas a simple heuristic feature, such as finding a typical bug that competitors simply forgot to add, can level up the analyzer. Also, contract compiling errors may affect the outcome. All reviewed software is quite fresh and is constantly under development, thus serious errors should not be considered unsolvable. In this article we don’t aim to help a reader decide which analyzer is better but demonstrate different code analysis methods in practice and how to choose the right ones. We recommend applying several tools at once, choosing the most suitable one for the audited contract. The article is related to current versions of software, at the moment when you read this article many things can be change

Analyzer: Slither

Description: Open-source static analysis framework for Solidity

Github: https://github.com/trailofbits/slither

Slither is a static code analysis framework written in Python. It can track variables, function calls and detect the following list of vulnerabilities: https://github.com/trailofbits/slither#detectors. Every vulnerability contains a link with a short description, so if you are a Solidity newbie, you’d better get familiar with all of them.

Slither can function as a Python module and provide a developer with a customizable audit interface. You can check out a nice and simple example of Slither functionality here:

https://github.com/trailofbits/slither/blob/master/examples/scripts/functions_writing.py

We will come back to analysis scripts at the end of the article. Now let’s launch Slither:

git clone https://github.com/trailofbits/slither.git
cd slither
docker build -t slither .

and try to analyze our contract.

Enter the constructor-eth-booking/ catalog and try launching Slither from Docker:

docker run -v $(pwd)/contracts:/slither/contracts slither:latest slither contracts/flattened.sol

It prints the “Source file requires different compiler version” error (because our contract requires solc=0.4.20), and we have to put version “solc=0.4.20” into Slither Docker. To do it, let’s modify Slither Dockerfile as indicated in item 2 of Part 1 of the Guide, i.e. add the following line somewhere at the bottom of the Dockerfile (https://github.com/trailofbits/slither/blob/master/Dockerfile):

COPY --from=ethereum/solc:0.4.20 /usr/bin/solc /usr/bin

recompile the image, run it, and ta-da — success!

We see multiple warnings about “pragma” and invalid variable name warnings like

Parameter ‘_price’ of Booking.Booking (flattened.sol#73) is not in mixedCase

Analyzers generate numerous warnings about code style but we are looking for serious bugs while ignoring minor flaws. Style aside, let’s filter all “mixedCase” messages:

docker run -v $(pwd)/contracts:/slither/contracts slither:latest slither contracts/flattened.sol 2>&1 | fgrep -v 'mixedCase'

Let’s skip the green lines and concentrate on everything in red :) Here’s what interesting Slither detected besides false-positives:

Booking.refundWithoutCancellationFee (flattened.sol#243–250) sends eth to arbitrary user
Dangerous calls:
- client.transfer(address(this).balance) (flattened.sol#249)
Reference: https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#functions-that-send-ether-to-arbitrary-destinations
INFO:Detectors:
Booking.refundWithCancellationFee (flattened.sol#252–259) sends eth to arbitrary user
Dangerous calls:
- owner.transfer(m_cancellationFee) (flattened.sol#257)
- client.transfer(address(this).balance) (flattened.sol#258)
Reference: https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description#functions-that-send-ether-to-arbitrary-destinations

Now let’s check what’s wrong with the contract functions:

function refundWithoutCancellationFee() private {
    address client = m_client;
    m_client = address(0);
    changeState(State.OFFER);
    client.transfer(address(this).balance);
}
function refundWithCancellationFee() private {
    address client = m_client;
    m_client = address(0);
    changeState(State.CANCELED);
    owner.transfer(m_cancellationFee);
    client.transfer(address(this).balance);
}

The function refundWithoutCancellationFee(), for instance, is called this way:

function rejectPayment() external onlyOwner onlyState(State.PAID) {
    refundWithoutCancellationFee();
}
function refund() external onlyClient onlyState(State.PAID) {
    refundWithoutCancellationFee();
}

In fact, there are no errors: calls are protected by onlyOwner and other modifiers, but Slither says that Ether is sent in refundWithoutCancellationFee() without checks. Is it right or not?

The function refundWithoutCancellationFee() itself has no limitations, no modifiers, and no branches inside. Although it is private and called from “rejectPayment()” and “refund()” wrappers with adequate limitations, there is still a risk to forget about limitations and expose the refundWithoutCancellationFee() call to attackers while developing a contract. So, while technically there is no vulnerability, the message was useful. At least, it is a “warning” risk level in case the contract code is supposed to be developed further. In this case, two functions from different parties use the same code and such decision was born from the need to save gas: for a single-use contract, the deploy fee is an important criterion.

I double-checked whether Slither is unhappy about any Ether sending or not, and moved the function body directly into the above “rejectPayment()” and “refund()”. No warning — and now Slither can tell Ether is not sent without address verifications. Good start!

Now let’s comment two variable initializations to see how Slither will handle them:

- m_fileHash = _fileHash;
+ // m_fileHash = _fileHash;
- m_price = _price;
+ // m_price = _price;

The first init is not very important in terms of vulnerability risks as m_fileHash is only used to be stored in blockchain after contract creation. On the contrary, m_price is quite common and Slither is unhappy it is used but not initialized:

Booking.m_price (flattened.sol#128) is never initialized. It is used in:
- fallback (flattened.sol#144–156)

This is a simple trick, everything worked out fine as expected.

Now we will add a reentrancу vulnerability to the contract and change its state after an external call. The changes are as follows:

function refundWithoutCancellationFee() private {
address client = m_client;
-     m_client = address(0);
-     changeState(State.OFFER);
-     client.transfer(address(this).balance);
+     client.call.value(address(this).balance)();
+     m_client = address(0);
+     changeState(State.OFFER);
}

I was forced to replace the ‘transfer’ command with ‘call’. Slither appears to pass reentrancy with ‘transfer()’ as valid, as it makes a call with minimum (2300) amount of gas, making а call from the called contact impossible. But, in case of migration to Constantinople Ethereum fork, the gas price gets changed re-introducing the risk of reentrancy attack which can use ‘transfer’ (https://github.com/ChainSecurity/constantinople-reentrancy). Be careful.

Reentrancy search results are as follows:

Reentrancy in Booking.refundWithoutCancellationFee (flattened.sol#243–253):
External calls:
- client.call.value(address(this).balance)() (flattened.sol#245)
State variables written after the call(s):
- m_client (flattened.sol#246)

That is great! At least, Slither won’t let us change state variables after external calls.

If we proceed with other vulnerabilities from the Slither’s list, we will either search for specific methods in the codebase or typical patterns that perform well and reliably if given access to Python code markup, i.e. Slither won’t miss well-known patterns.

Next step — I will add specially crafted pattern, idiotic in real life, but perfectly demonstrate static sources analyzers operation:

- client.transfer(address(this).balance
+ for (uint i=0; i < 1; i++) {
+     client.transfer(address(this).balance - 999999999999999999);
+ }

Here’s the result:

Booking.refundWithoutCancellationFee has external calls inside a loop:
- client.transfer(address(this).balance — 999999999999999999) (flattened.sol#252)
Reference: https://github.com/trailofbits/slither/wiki/Vulnerabilities-Description/_edit#calls-inside-a-loop

As the cycle is single and degenerate, the warning is false-positive whereas the absence of a warning about “dangerous” arithmetic is false-negative. Static analyzers are not used to perform type analysis, estimate operation results or calls. Therefore, we should clearly differentiate between the errors specific for static analysis and those identified using dynamic analysis.

I promised to tell about writing individual testing scripts and interesting contract details output using the “ — print” key. From this point of view, Slither is an excellent tool for CI. Developers of large contract systems know the names of variables that are crucial for safety: balances, commission sizes, flags, and can write a testing script that will automatically block any code modifications involving essential variables and changes in state variables after an external call. Slither is an excellent tool for using in hooks because of high predictability of results. Slither helps to get rid of silly bugs, identify well-known dangerous patterns and warn a developer. It is also a good tool for Solidity newbies who want to write code correctly from the start.

Results

Slither is a versatile and flexible tool, having powerful, simple and easy-to-follow analysis scripts, written in Python, with excellent CI compatibility.

Slither found a serious WARNING relating to Ether sending function, and detected all added pseudo-bugs. It failed only at dynamic analysis — the one it’s not supposed to do by design. Otherwise, it would lose its main advantages — predictability, usability and simplicity.

Next time we will discuss Mythril analyzer. Other articles to be published or in production are as follows:

Part 1. Introduction. Compilation, Flattening, Solidity Versions

Part 2. Slither (this article)

Part 3. Mythril

In case you have a specific question regarding the use of Slither for your project, feel free to contact us onour website or via Telegram.