The OpenNET Project / Index page

[ новости /+++ | форум | wiki | теги | ]

Уязвимость, позволяющая осуществить подстановку 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 тысяч долларов США.

  1. Главная ссылка к новости (http://blog.orange.tw/2017/01/...)
  2. OpenNews: Выявлена порция уязвимых SSH-ключей доступа к GitHub
  3. OpenNews: Возможность доступа к приватным репозиториям GitHub при помощи серии незначительных уязвимостей в OAuth
  4. OpenNews: Через уязвимость в GitHub от имени Линуса Торвальдса создан фиктивный репозиторий linux-ng
  5. OpenNews: В GitHub устранена уязвимость, допускающая внедрение кода в любой репозиторий
  6. OpenNews: Половина кода на GitHub поставляется без указания лицензии и небезопасна для интеграции в открытые проекты
Лицензия: CC-BY
Тип: Проблемы безопасности
Короткая ссылка: https://opennet.ru/45825-github
Ключевые слова: github
При перепечатке указание ссылки на opennet.ru обязательно
Обсуждение (36) Ajax | 1 уровень | Линейный | +/- | Раскрыть всё | RSS
  • 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 без лишних церемоний, это научит вебмакак валидировать ввод :)

     

     Добавить комментарий
    Имя:
    E-Mail:
    Текст:



    Спонсоры:
    Inferno Solutions
    Hosting by Hoster.ru
    Хостинг:

    Закладки на сайте
    Проследить за страницей
    Created 1996-2021 by Maxim Chirkov
    Добавить, Поддержать, Вебмастеру