Уязвимость, позволяющая осуществить подстановку SQL-кода в GitHub Enterprise

08.01.2017 12:01

В GitHub Enterprise, варианте GitHub для предприятий, позволяющем развернуть окружение для совместной разработки внутри корпоративной сети на подконтрольном оборудовании, выявлена уязвимость, позволяющая через отправку специально оформленного запроса выполнить произвольный SQL-код на сервере.

В описании уязвимости приводится заслуживающий внимания рассказ об организации работы кода GitHub Enterprise, который во многом пересекается с кодом публичного сервиса GitHub. Пакет поставляется в виде образа виртуальной машины, доступной для бесплатного ознакомительного использования в течение 45 дней. Исходные тексты скрыты и поставляются в упакованном виде, напоминающем зашифрованный набор данных. Прозрачная распаковка в момент выполнения осуществляется при помощи библиотеки ruby_concealer.so, анализ которой показал, что метод упаковки сводится к сжатию кода при помощи Zlib::Inflate::inflate и применению операции XOR с предопределённым ключом.

После распаковки было выяснено, что большая часть кода написана на языке Ruby с использованием фреймворков Ruby on Rails и Sinatra, но также применяются компоненты на Python, Bourne Shell, C++ и Java. По мнению исследователя безопасности, код, отвечающий за работу web-сервисов, основан на реальной кодовой базе github.com, gist.github.com, render.githubusercontent.com и api.github.com. Получив доступ к коду исследователь, до этого не имевший дело c языком Ruby, потратил всего четыре дня на поиск возможных уязвимостей и выявил проблему в обработчике PreReceiveHookTarget, позволяющую осуществить подстановку SQL-кода через передачу специально оформленных данных через параметр "sort", отправив запрос к общедоступному Web API.



   $ curl -k -H 'Accept:application/vnd.github.eye-scream-preview' \
   'https://192.168.187.145/api/v3/organizations/1/pre-receive-hooks?access_token=????????&sort=id,(select+1+from+information_schema.tables+limit+1,1)'

   $ curl -k -H 'Accept:application/vnd.github.eye-scream-preview' \
   'https://192.168.187.145/api/v3/organizations/1/pre-receive-hooks?access_token=????????&sort=id,if(user()="github@localhost",sleep(5),user())'

GitHub был уведомлен о проблеме в конце декабря и устранил уязвимость в выпуске GitHub Enterprise 2.8.5. Выявившему уязвимость исследователю выплачено вознаграждение в размере 5 тысяч долларов США.

исправить +16 +/–

Лицензия: CC BY 3.0

Короткая ссылка: https://opennet.ru/45825-github

Ключевые слова: github

При перепечатке указание ссылки на opennet.ru обязательно

Обсуждение (36)

1.1, Аноним (-), 12:49, 08/01/2017 [ответить] [﹢﹢﹢] [ · · · ]	+4 +/–
Хороший способ заработать 5 тыс $ за 4 дня.

2.21, Аноним (-), 18:47, 08/01/2017 [^] [^^] [^^^] [ответить]	+/–
> исследователю выплачено вознаграждение в размере 5 тысяч долларов США А могли бы исследователю просто дать абонемент на год бесплатного Github Enterprise...

3.23, Аноненамус (?), 19:49, 08/01/2017 [^] [^^] [^^^] [ответить]	+1 +/–
Но зачем? где он его тратить будет?

4.52, Аноним (-), 18:29, 09/01/2017 [^] [^^] [^^^] [ответить]	+/–
это, видимо - отсылка к наградам некоторых компаний

1.2, deadfood (ok), 13:16, 08/01/2017 [ответить] [﹢﹢﹢] [ · · · ]	+6 +/–
>Прозрачная распаковка в момент выполнения осуществляется при помощи библиотеки ruby_concealer.so, анализ которой показал для метод упаковки сводится к сжатию кода при помощи Zlib::Inflate::inflate и применению операции XOR с предопределённым ключом. что на опеннете делает эта гнусная проприетарщина?

Часть нити удалена модератором

3.9, VANANIMAS (?), 17:34, 08/01/2017 [^] [^^] [^^^] [ответить]	+/–
Ноет.

2.36, Аноним (-), 05:43, 09/01/2017 [^] [^^] [^^^] [ответить]	+/–
> что на опеннете делает эта гнусная проприетарщина? Кормит троллей лулзами разве что. Для чего-то такого проприетарь и нужна.

1.3, IB (?), 14:13, 08/01/2017 [ответить] [﹢﹢﹢] [ · · · ]	+/–
Вашу ж мать, когда "ынтырпрайз" научится использовать prepared statements

2.4, Ordu (ok), 14:54, 08/01/2017 [^] [^^] [^^^] [ответить]	–3 +/–
Это ruby и ActiveRecord. Там это может быть не столь элементарно, чем в случае, когда SQL-запрос собирается вручную при помощи конкатенации строк, типа как в пхп. В том смысле, что ActiveRecord позволяет подставлять SQL-код иногда, для всяких разных целей. И мало просто использовать prepared statements, надо знать где находятся грабли, чтобы на них не наступить. Это, к слову, о безопасности языка или, как в данном случае, библиотеки: было бы полезнее урезать функциональность ActiveRecord, вынудив программиста использовать sql вручную, когда ему не хватает возможностей ActiveRecoed -- было бы больше шансов на то, что программист не наступит на эти грабли. Но -- нет, -- хочется напихать в ActiveRecord максимум функциональности, даже когда выбранные абстракции уже не позволяют это сделать, не подкладывая программисту граблей. Может быть и можно выработать какие-то гайдлайны для программистов, чтобы даже не зная где лежат грабли, можно было бы успешно через них перешагивать -- я не настолько хорошо знаю рельсы, чтобы говорить. Но во всяком случае, тут всё чуть сложнее, чем безрукий "ынтырпрайз": да, кодеру не хватило квалификации, но это задача библиотек и языка делать так, чтобы если кодеру не хватает квалификации, то он бы не мог написать код, не позволяя ему создавать г-нокод, который вроде как даже работает, но требует внимательного чтения и вникания в детали, чтобы найти в нём косяк.

3.11, Аноним (-), 17:46, 08/01/2017 [^] [^^] [^^^] [ответить]	+3 +/–
Проблема не в ActiveRecord, а в том, что входные данные никто не проверяет как следует. Что до ActiveRecord, то библиотека, видимо, лишь позволяет в качестве order_by использовать всё, что можно написать в SQL-запросе через терминал.

4.17, Алексей Морозов (ok), 18:37, 08/01/2017 [^] [^^] [^^^] [ответить]	+3 +/–
Оставлять проверку входных данных на прикладного программиста — изощренный способ сделать себе больно в неожиданный момент. Кто-нибудь обязательно облажается, не подумает обо всех corner cases, а ревьюверы кода не заметят, будут заняты другими делами и вообще, режим "давай-давай-быстрее!" никто не отменял. SQL-стейтменты должны формироваться автоматом. Простой, хотя и не гибкий и не всегда применимый способ — prepared statements. Более сложный — DSL, который позволит создавать запросы из кубиков и переиспользовать какие-то стандартные заготовки в разных местах, без опасности того, что злые люди пропихнут injection. Из публично доступных библиотек, реализующих такой DSL, мне известен http://www.querydsl.com/ , хотя мы внутри пользуемся некоторым собственным велосипедом для C++ и Java.

5.30, АнониМ (ok), 23:21, 08/01/2017 [^] [^^] [^^^] [ответить]	–1 +/–
от prepared statements может просесть производительность. Тот же оракл может очень хорошо оптимизнуть запрос, ежель имеет конкретные значения, а не ps. Так что я б поостерёгся от утверждений "SQL-стейтменты должны формироваться автоматом".

6.34, Анином (?), 03:18, 09/01/2017 [^] [^^] [^^^] [ответить]

–2 +/–

>Тот же оракл может очень хорошо оптимизнуть запрос, ежель имеет конкретные значения

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

То есть вместо запроса
select * from tbl where fld=:val
который один раз распарсится и далее оракл берет уже расперсеный запрос с вместе с планом из кеша,
писать запросы так
select * from tbl where fld=123
то для разных значений fld запрос для оракла будет буде считаться другим, а в силу большого количества возможных значений fld, старые варианты будут вытесняться из кеша. Как следствие, фактически каждый запрос будет заново парситься и заново строиться план запроса.
Хуже того, нормальные пацаны, которые грамотно пишут запросы, тоже пострадают, так как кеш запросов у оракла общий. Такой альтернативно одареный кодер сломает работу на сервере всем.

Подозреваю, что это майэскуэльщики перелезли в оракл, т.к. у майскуэля работа организована по другому.

6.35, . (?), 04:18, 09/01/2017 [^] [^^] [^^^] [ответить]	–2 +/–
>Тот же оракл может очень хорошо оптимизнуть запрос, ежель имеет конкретные значения, а не ps. Ага, расскажи о вкусе ананасов тем кто их и в самом деле ел :-)))) В общем - в сад!

6.51, anonymous (??), 17:44, 09/01/2017 [^] [^^] [^^^] [ответить]	+/–
> от prepared statements может просесть производительность. Тот же оракл может очень хорошо > оптимизнуть запрос, ежель имеет конкретные значения, а не ps. Так что > я б поостерёгся от утверждений "SQL-стейтменты должны формироваться автоматом". Отпимизация случайного возникшего хорошего случая (не забываем, что речь идет о параметрах, формируемых из пользовательсокого ввода) не стоит возникающих из-за нее хлопот. Отпимизировать имеет смысл плохие и средние случаи

5.58, xz (??), 12:52, 12/01/2017 [^] [^^] [^^^] [ответить]	+/–
> SQL-стейтменты должны формироваться автоматом. Простой, хотя и не гибкий и не всегда > применимый способ — prepared statements. А какая связь между автоматическим созданием SQL-стейтмента и prepared?

4.33, Ordu (ok), 02:22, 09/01/2017 [^] [^^] [^^^] [ответить]	+/–
Конечно же. Сколь бы дурацкий инструмент мы не использовали, если не справляется программист, то он и виноват. Продолжай думать так и дальше. Это несомненно позволит тебе не занимаясь тщательным выбором инструмента поднять свой скилл хождения по граблям до невообразимых высот. Только, тебе не боязно, что другие тем временем найдут более безопасные инструменты и потратят то же самое время на развитие более конструктивных навыков, таким образом подняв свою продуктивность до тех самых высот, на которых будет твоё умение не наступать на разложенные инструментами грабли?

5.40, Аноним (-), 11:23, 09/01/2017 [^] [^^] [^^^] [ответить]	–1 +/–
клоун: Пока запрос пишется в текстовом виде, будут существовать ошибки подстановки в него "инъекций". Другой инструмент? Сколько тебе лет что ты веришь в священные граали, решающие все проблемы лишь фактом своего существования? Защита? Это как в Java, когда к любой функции нужно дописать "throw IOException\|BakaMegaException\|NeZnayuChtuException", а затем заимпортить всю эту дребедень лишь чтобы компилятор от тебя отвязался? Мда... Защита во все дыры. Вместо критической ошибки выполнения в одном месте получают логическую ошибку в другом. По твоему это лучше?

6.43, Аноним (-), 13:27, 09/01/2017 [^] [^^] [^^^] [ответить]	+/–
Однако ж попробуй заинъектить что-нибудь в простую key-value базу, где ключ и значение произвольные бинарные значения. Если специальной трактовки символов нет то и инъекция не получится.

7.44, Аноним (-), 13:44, 09/01/2017 [^] [^^] [^^^] [ответить]	–1 +/–
клоун если программа работает с внешними данными, то можно подобрать такой их н... большой текст свёрнут, показать

8.54, Аноним (-), 20:03, 09/01/2017 [^] [^^] [^^^] [ответить]	+/–
Только если автор - дол и не понял что извне таки натурально могут приехать Л... большой текст свёрнут, показать

6.45, Ordu (ok), 14:02, 09/01/2017 [^] [^^] [^^^] [ответить]

+1 +/–

> Пока запрос пишется в текстовом виде, будут существовать ошибки подстановки в него "инъекций".

Бла-бла-бла... Ещё какой всемирный закон ты открыл?

> Сколько тебе лет что ты веришь в священные граали, решающие все проблемы лишь фактом своего существования?

Сколько _тебе_ лет, что ты никак не можешь излечиться от юношеского максимализма? Если священного грааля не существует, значит не надо, создавая API, думать о том, как бы поменьше граблей туда напихать?

7.46, Аноним (-), 14:18, 09/01/2017 [^] [^^] [^^^] [ответить]	–1 +/–
клоун: Ты в суть проблемы то вник? Они неверно экранировали входные данные. Какие ещё грабли? Грабли - это о другом. Напр в php есть уже 3 функции экранирования (экранирование при выводе в HTML, экранирование для SQL, улучшенное экранирование для SQL). Последнее - как раз те самые грабли. "Руководство по собиранию парашюта. Версия вторая. Исправленная". Мало того что они накосячили при первой реализации экранирования, так они ещё оставили обе функции. Более того: они использовали пересекающуюся терминологию (слово "экранирование" в обоих случаях) чтобы накосячил прикладной программист. Вот это грабли как они есть.

8.50, Ordu (ok), 17:40, 09/01/2017 [^] [^^] [^^^] [ответить]	+/–
Ты с ActiveRecord знаком, деточка ActiveRecord занимается сборкой текстового sq... большой текст свёрнут, показать

3.12, Kodir (ok), 17:48, 08/01/2017 [^] [^^] [^^^] [ответить]	–7 +/–
AR вообще тут не причём! Какой бы ни был ORM, программист просто не имеет права брать "абстрактный шмот символов" и тут же совать на исполнение! Правильно люди сказали - обязан быть хардкоженый SQL оператор + параметр, принимающий строку извне. Эксплойт лишний раз доказывает, насколько раздута слава git/github безо всякого анализа качества - а хомячки и рады прыгать "это написал наш великий Линус!". Mercurial - наше всё. :)

4.13, Аноним (-), 17:55, 08/01/2017 [^] [^^] [^^^] [ответить]	+6 +/–
> Уязвимость, позволяющая осуществить подстановку SQL-кода в GitHub Enterprise > Эксплойт лишний раз доказывает, насколько раздута слава git > GitHub Enterprise > git > GitHub Enterprise > git > GitHub Enterprise > это написал наш великий Линус! > GitHub Enterprise > это написал наш великий Линус! Слишком толсто, попробуйте потоньше.

4.37, Аноним (-), 05:45, 09/01/2017 [^] [^^] [^^^] [ответить]	+1 +/–
> Mercurial - наше всё. :) Угу, ваше. "Ну тогда и т...ся сам с собой!" (c) анекдот.

2.14, Crazy Alex (ok), 17:57, 08/01/2017 [^] [^^] [^^^] [ответить]	–6 +/–
Гитхаб с руби - это так себе энтерпрайз Со джавой спрингом я подобных уязвимост... большой текст свёрнут, показать

3.18, Лютый жабист__ (?), 18:41, 08/01/2017 [^] [^^] [^^^] [ответить]	–5 +/–
Prepared statements на любом языке это кал. Приходится писать их 100500 однотипных. Вообще sql устарел, вот в монге всё гуд и без ps.

3.19, Алексей Морозов (ok), 18:41, 08/01/2017 [^] [^^] [^^^] [ответить]

+/–

> По идее, давно пора добавить ещё уровень - где динамическим было бы
> всё, от имён таблиц и полей до ключей запроса.

Известный мне open source пример — querydsl: http://www.querydsl.com/static/querydsl/latest/reference/html/ch02s03.html Однако, я не уверен, что querydsl равномощен SQL'ю, т.к. мы пользуемся самописным велосипедом, который позволяет сгенерировать любой требуемый SQL, с расширениями типа postgis'а.

4.31, Crazy Alex (ok), 23:48, 08/01/2017 [^] [^^] [^^^] [ответить]	+/–
Это вообще не то. Во-первых, я говорил о том, что подобная штука должна быть чем-то стандартным и разбираться в базе. А здесь - опять просто надстройка, генерирующая SQL. Во-вторых, она не решает те проблемы, из-за которых обычно начинают руками генерировать SQL. Например, "сунуть в SELECT список полей из конфига".

5.49, Алексей Морозов (ok), 16:28, 09/01/2017 [^] [^^] [^^^] [ответить]

–1 +/–

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

Реальность, данная нам в ощущениях, явственно говорит, что в существующие SQL RDBMS запросы всовываются в текстовом виде. Да, я знаю, что у многих из таких RDBMS есть альтернативные способы формирования запросов, более структурированные. Однако, речь идет именно об SQL, он ровно такой, какой есть.

> Во-вторых, она не решает те проблемы, из-за которых обычно начинают руками генерировать SQL

Ещё как решает. Если в объекте, отвечающем за таблицу, есть фиксированный набор методов/пропертей, описывающих манипуляции с полями, то и в формируемый SQL-запрос можно будет затолкать ровно эти поля.

> Например, "сунуть в SELECT список полей из конфига".

В этом случае прикладной программист будет, по первости чертыхаясь и жалуясь на непроизводительную трату времени, писать явный разбор входных данных и явный маппинг токенов в разобранном тексте на вызовы DSL'я.

Собственно, прямо буквально за соседним столом такой человек сидит. Но, нужно отметить, после новостей об очередной SQL-инъекции жалоб "а нафига здесь париться, если можно просто склеить несколько строк" становится существенно меньше :)

2.42, angra (ok), 13:04, 09/01/2017 [^] [^^] [^^^] [ответить]	+/–
> Вашу ж мать, когда "ынтырпрайз" научится использовать prepared statements Когда уже невежды узнают про ограниченность возможностей prepared statements и перестанут вещать про них во всех новостях про SQL injection? В данном случае никакой prepared statement помочь в принципе не мог.

2.47, mimocrocodile (?), 15:12, 09/01/2017 [^] [^^] [^^^] [ответить]	+1 +/–
И тут ты такой приводишь пример использования prepared statements с задаваемой пользователем сортировкой

1.32, Вы забыли заполнить поле Name (?), 01:55, 09/01/2017 [ответить] [﹢﹢﹢] [ · · · ]

–1 +/–

> анализ которой показал, что метод упаковки сводится к сжатию кода при помощи Zlib::Inflate::inflate и применению операции XOR с предопределённым ключом.
> после распаковки было выяснено, что большая часть кода написана на языке Ruby

Я правильно понял, что выявивший уязвимость определил ключ и получил код?

> По мнению исследователя безопасности, код, отвечающий за работу web-сервисов, основан на реальной кодовой базе github.com, gist.github.com, render.githubusercontent.com и api.github.com.

Как он это понял, если код перечисленных сервисов закрыт?

2.39, Аноним (-), 05:49, 09/01/2017 [^] [^^] [^^^] [ответить]	+/–
> Как он это понял, если код перечисленных сервисов закрыт? Правильно, определять надо путем drop table students без лишних церемоний, это научит вебмакак валидировать ввод :)

игнорирование участников | лог модерирования

Добавить комментарий

Текст: