Что по аудиту? Sablier - результаты

Только вчера вечером изучал предварительные результаты конкурса и сегодня уже в комментах увидел вопрос о том, приняли ли мои находки.

Нет, оба репорта оказались не валидными. И раз уж я пошел делиться с вами своим процессом аудита, то будет правильно рассказать вам об "ожиданиях на бум" и столкновением с реальностью.

Всего сейчас в конкурсе три подтвержденных находки уровня Low.

Я отправил два репорта, вместо трех, как хотел изначально. И знаете, какой был третий репорт? Да, как раз на расхождение со стандартом ERC4906. Почему же я не отправил его?

После изучения протокола я увидел, что Sablier превосходно подготовились к аудиту и не оставили никаких более-менее открытых возможностей для атаки. Контракт отличается высоким уровнем безопасности и продуманности. И такая проблема, как расхождение со стандартом, скорее всего, была обусловлена выбором самих разработчиков. И что ее посчитают максимум Info, и все равно сделают не валидной.

По своему первому репорту, я понадеялся, что "прокатит". Да, видел в репорте от LightChaser есть пункт на отсутствие проверки аргументов на нулевой адрес. При этом там были явно указаны некоторые функции, где такой проверки не было. Я же обратил внимание на ситуацию, которая не была описана в отчете. Судьи посчитали, что это "проблема одной сути" (что по факту так и есть) и поставили invalid.

Со вторым репортом я сам ошибся и только вчера понял, что если бы уделил чуть больше времени на валидацию находки, то ничего бы в итоге не отправил.

Когда я просматривал контракты, то обратил внимание, что в одной из функций сначала было деление (descale), а потом умножение (scale). Решил, что это обычная математическая проблема и не проверил, на что она повлияет. В итоге получилось, что я был отчасти прав и это часть уже была указана в одном из предыдущих репортов.

В завершение могу сказать, что упустил две проблемы:

1. Пользователи могут избежать комиссии, делая депозит мелких сумм.
2. Оракул возвращает одинаковые значения для разных по сути Flow.

И если про первый баг, я даже не задумывался, то со вторым интереснее...

Я писал, что понял протокол где-то на 90%. И функция, в которой был найден 2 баг, была как раз в этих 10%. Я тогда (да и сейчас), не особо понял, зачем вообще нужна эта функция. К тому же она не использовалась ни в каких других в протоколе. Грубо говоря, тогда я решил просто "забить" на нее и фокусировал внимание на основной части протокола.

Думаю, стоит вынести пару уроков из этого конкурса:

1. Подавайте все, что вам кажется будет валидным. Не стоит делать за судей работу и решать уровни багов. Нашли несоответствие стандарту - подавайте. Даже если эту находку не примут, вы ничего не потеряете.

2. Проверяйте лучше свои находки. Если вы нашли какую-то проблему в контракте, то потратьте больше времени на подтверждение своей находки. Не стоит полагаться, что если подобное было раньше, то 100% это засчитают и сейчас. Проверяйте! А лучше напишите тест, если время вам позволяет.

3. Старайтесь понять протокол на 100%. Если вы планируете стать аудитором на все рабочее время, то для хороших находок вам нужно понимать весь протокол целиком. Даже из этого конкурса можно вынести то, что в нишевой функции, которая нигде больше не используется, может быть проблема.

Вскоре заканчивается еще один маленький конкурс на Codehawks и после него я также поделюсь своими заметками и наблюдениями!

Всем приятной недели и легкого обучения!

#audit