Анализатор: Slither
Описание: Open-source static analysis framework for Solidity
githib: https://github.com/trailofbits/slither
Это статический анализатор кода, написанный на python. Он умеет следить за переменными, вызовами, и детектирует вот такой список уязвимостей: https://github.com/trailofbits/slither#detectors. У каждой уязвимости есть ссылка с описанием, и, если вы новичок в Solidity, вам имеет смысл ознакомиться со всеми.
Slither может работать, как модуль python и предоставлять программисту интерфес, для аудита по собственному плану. Простой и показательный пример того, что умеет делать slither можно увидеть тут: https://github.com/trailofbits/slither/blob/master/examples/scripts/functions_writing.py
Мы еще вернемся к сценариям анализа в конце статьи, а пока запустим Slither:
git clone https://github.com/trailofbits/slither.git
cd slither
docker build -t slither .и попробуем проанализировать наш контракт.
Заходим в каталог с constructor-eth-booking и пытаемся запустить slither из докера:
$ docker run -v $(pwd)/contracts:/slither/contracts slither contracts/flattened.solполучаем ошибку “Source file requires different compiler version”, и теперь нам надо засунуть версию solc=0.4.20 в докер slither-а. Для этого правим Dockerfile самого Slither, как было указано в п.2 во введении, т.е. где нибудь в конце Dockerfile (https://github.com/trailofbits/slither/blob/master/Dockerfile) добавляем строку:
COPY --from=ethereum/solc:0.4.20 /usr/bin/solc /usr/bin, пересобираем образ, запускаем, ура, все компилируется.
Видим вывод, warning-и про разные “pragma” и про неверные названия переменных типа Parameter '_price' of Booking.Booking (flattened.sol#73) is not in mixedCase. Анализаторы выдают много предупреждений, но мы ищем реальные баги, и не будем обращать внимание на мелочь. Отфильтруем все сообщения про mixedCase, сейчас не до стиля:
$ docker run -v $(pwd)/contracts:/slither/contracts slither contracts/flattened.sol 2>&1 | fgrep -v 'mixedCase'Истинные программисты пропускают все зелененькое, смотрят все красненькое, и вот что помимо false-positives нашел в контракте Slither:
Booking.refundWithoutCancellationFee (flattened.sol#243-250) sends eth to arbirary 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 arbirary 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теперь смотрим что в контракте не так с этим функциями:
/************************** PRIVATE **********************/
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);
}при этом, например, функция refundWithoutCancellationFee() вызывается вот так:
function rejectPayment() external onlyOwner onlyState(State.PAID) {
refundWithoutCancellationFee();
}
function refund() external onlyClient onlyState(State.PAID) {
refundWithoutCancellationFee();
}
Хм, формально ошибок нет: вызовы защищены всякими onlyOwner, но Slither ругается, что, мол, внутри refundWithoutCancellationFee() без всяких проверок шлется эфир. И он прав, сама функция действительно не имеет почти никаких ограничений. Пускай она private, и вызывается из wrapper-ов “rejectPayment()” “refund()” c нужными ограничениями, но в таком виде, если дорабатывать контракт — велик риск забыть про ограничения и воткнуть вызов refundWithoutCancellationFee() в какое нибудь другое место, доступное атакующему. Так, что, пусть формально уязвимости нет, информация оказалась полезной — это как минимум “warning” level, если по заданию планируется развивать код контракта дальше. В данном случае, две функции от разных участников используют один код, и, такое решение было принято для экономии газа — контракт является одноразовым, и стоимость его выкладки является важным фактором.
Я перепроверил, не просто ли так Slither ругается на любую отправку эфира, и перенес тело функции прямо в вышеуказанные “rejectPayment()” и “refund()”, предупреждение исчезло, т.е. Slither понял, что теперь эфир не высылается без проверок адресов. Отличное начало!
Теперь проверим как Slither следит за инициализацией переменных, для этого закомментируем две инициализации:
- m_fileHash = _fileHash;
+ // m_fileHash = _fileHash;
- m_price = _price;
+ // m_price = _price;первая — не сильно важная, в плане дыр, кроме траты ресурсов, ибо m_fileHash не используется нигде, он просто сохраняется в блокчейне при создании контракта. А вот m_price используется, и Slither правильно ругается на то, что m_price нигде не инициализируется, хотя и используется:
Booking.m_price (flattened.sol#128) is never initialized. It is used in:
- fallback (flattened.sol#144-156)Ну это простой трюк, как и ожидалось, всё сработало нормально.
Теперь внесём в контракт столь полюбившуюся всем reentrancу: будем изменять state контракта после внешнего вызова. Внесем такие изменения:
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);
}Пришлось заменить transfer на call, т.к. на вариант с transfer Slither не ругается из за того, что transfer отправляет вызов с минимумом газа, и обратный вызов невозможен (хотя при переходе на форк Constantinople в Ethereum, была изменена цена газа, и это заново “включило” reentrancy атаку с использованием transfer (https://github.com/ChainSecurity/constantinople-reentrancy).
Результат поиска reentrancy:
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)Прекрасно, как минимум изменять state variables после внешних вызовов он не даст, и это очень хорошо.
Если двигаться по списку, то остальные уязвимости в списке либо представляют собой просто поиск конкретных методов в коде, либо известные паттерны, которые при наличии доступа к разметке кода, выполненной python-ом, конечно же работают, и достаточно надежно. Т.е. well-knows паттерны Slither не пропустит.
Теперь, я внесу изменения, которые отлично показывают специфику работы статических анализаторов:
- client.transfer(address(this).balance
+ for (uint i=0; i < 1; i++) {
+ client.transfer(address(this).balance - 999999999999999999);
+ } и результат:
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Цикл выполняется единожды, и является вырожденным — поэтому выданное предупреждение — false positive, а отсутствие предупреждения об опасной арифметике — false negative. Анализ типов, результатов операций, подсчет вызовов — задачи не для статических анализаторов. Поэтому чётко понимайте, какие ошибки найдет Slither, а какие необходимо искать с помощью других инструментов.
Мы обещали упомянуть о возможности написания собственных сценариев для тестирования, и вывода всякой интересной информации о контракте с помощью ключа --print. С этой точки зрения, Slither представляет собой отличный инcтрумент для CI. Разработчики большой системы контрактов знают названия критических для безопасности переменных: балансов, размеров комиссий, флагов, и могут написать сценарий тестирования, который будет блокировать любые изменения в коде, которые, к примеру, перезаписывают важную переменную, или меняют state переменные после внешнего вызова, и его отлично предсказуемый анализ является отличным инструментом для использования в hook-ах.
Задача Slither — избавить вас от глупых багов, найти хорошо знакомые опасные паттерны и предупредить разработчика. В этом варианте он хорош и как инструмент начинающего разработчика на Solidity, сразу подсказывая как правильно писать код на Solidity.
Итоги
В моем личном тестировании я бы поставил Slither четверку, за универсальность, простоту и удобство использования, а также за простые и понятные сценарии тестирования и приспособленность к CI.
Slither уверенно нашел реальный WARNING, связанный с использованием функции, отправляющей эфир, обнаружил все внесенные баги. Он не справился лишь с динамическим анализом, который формально и не должен делать, иначе придется пожертвовать универсальностью, предсказуемостью и простотой использования.
В следующей статье мы займемся анализатором Mythril, а вот все оглавление статей, которые готовы или планируются к написанию:
Часть 1. Введение. Компиляция, flattening, версии Solidity
Часть 2. Slither (эта статья)
Часть 3. Mythril (в процессе написания)
Часть 4. Manticore (в процессе написания)
Часть 5. Echidna (в процессе написания)