Pull to refresh
8K+
7
fimskiy@fimskiy

User

55
Rating
Send message

Сделал демо с branch protection: https://github.com/fimskiy/evil-merge-demo/pull/9

Сценарий:

  1. Branch protection включён — require 1 approving review

  2. Второй аккаунт (reviewer) одобрил PR, видел только "superuser"

  3. Мейнтейнер смержил локально, добавил "hacked" token в merge-коммит

  4. Запушил в main через admin bypass

Ключевой момент: в момент апрува merge-коммита ещё не существует — он создаётся только при нажатии “Merge”. Ревьюер физически не может его видеть, потому что его ещё нет. После merge ссылка на коммит 92a08fd появляется внизу PR — но туда нужно специально идти и знать что искать.

Да, если после merge специально кликнуть на hash коммита — evil code виден. Атака не про техническую невозможность обнаружения, а про то, что очень часто никто этого не делает в рутинной работе. Именно так это работало 3,5 месяца в реальном проекте с CI, code review и несколькими разработчиками.

Нет, в этом примере evil merge сделан локально через git commit --amend — это более простой способ для демонстрации.
Через GitHub UI сценарий немного другой: при наличии конфликта GitHub предлагает редактировать файл прямо в браузере, и в этот момент можно добавить extra code. Конечный эффект тот же — в merge-коммит попадает код, которого не было в PR.

Проблема именно с merge-коммитами, потому что в GitHub UI можно редактировать конфликты прямо в браузере, добавляя extra code. При rebase + fast-forward конфликты разрешаются локально, и такой фокус не пройдёт.

В статье я допустил неточную формулировку про скролл. Суть проблемы не в этом. Дело в том, что GitHub не показывает вообще строку в PR-diff. Это не про длинные строки, а про то, что изменения, добавленные в самом merge-коммите (поверх результата слияния), не отображаются в PR.
GitHub показывает diff: feature branch vs main.
GitHub не показывает: что было добавлено в merge-коммите после слияния.

Спасибо за уточнение, я обновил формулировку в статье.

Вот полный пример: https://github.com/fimskiy/evil-merge-demo
В PR виден только "superuser", но в итоговом файле в main есть оба токена — "hacked" и "superuser". Можешь сам посмотреть merge-коммит и убедиться.

Да, это усложняет атаку в GitHub, но полностью проблему не убирает. При достаточных правах правила можно обойти. И это не защищает от уже существующих изменений в истории.

Rebase + fast-forward полностью закрывает эту атаку — merge-коммитов нет, вектора нет. Чистая история как бонус. Если команда может себе это позволить, это лучшее решение. Проблема в том, что большинство крупных проектов используют merge-коммиты — GitHub создаёт их по умолчанию, и далеко не все меняют эту настройку. Для них и нужен инструментальный контроль.

Именно так. Классический сценарий: ревьюер одобрил PR, попросил только разрешить конфликты — и после этого повторного ревью не требуется. Атакующий в момент разрешения конфликта добавляет произвольный код в merge-коммит, и этот код уходит в main без чьего-либо взгляда.

▎ Что интересно — решение, которое GitHub упомянул как возможное направление (“may make this functionality more strict”), как раз и означало бы: после изменений в merge-коммите аппрув сбрасывается и требуется повторное ревью. Пока этого нет,
единственная защита — инструментальная проверка самого merge-коммита.

При разрешении конфликта все изменения видны в diff PR’а — ревьюер может их проверить. Evil merge — это изменения, которых нет ни в одной из веток-родителей и которые не являются конфликтом. Они добавляются напрямую в merge-коммит после того, как git уже сделал слияние. GitHub в таком случае показывает в PR только diff относительно base branch — и эти изменения в нём не видны совсем.

Выглядит, как фантастика, но легко воспроизводится

Information

Rating
169-th
Registered
Activity

Specialization

Фулстек разработчик