1. PHP / Говнокод #4163

    +155

    1. 1
    2. 2
    3. 3
    4. 4
    5. 5
    <?php
    $url = $_GET['url'];
    $url = str_replace('http://', '', $url);
    echo '<a href="http://$url">123</a>';
    ?>

    Запостил: Мартин, 03 Сентября 2010

    Комментарии (68) RSS

    • опять пхп, опять мартышкокод. Судя по всему, чей-то эксперимент самообучения в пхп

      странная, конечно, логика - убирать протокол, а потом вставлять... Наверное, это, типа что бы он обязательно был, но один раз.
      а к тому же не учтено, что протокол может оказаться не только вначале (хотя, если правильно, то должен быть проэскейпен)
      про echo, наверное, скажет кто-нибудь другой, но больше меня насмешило хардкод название 123.
      Ответить
    • если это будет доступно не только из локалхоста, то привет code injecting + xss
      Ответить
      • Скорее небо упадёт на землю и Дунай повернёт вспять мелкомягкие сделают винду опенсорц, чем мартышки научатся писать на PHP без code injecting и XSS.
        Ответить
        • попытка подумать об этом за них обернулась эпичным фейлом (я про "волшебные кавычки")

          дело в том, что в книжках для начинающих об этом ни гугу. Я тоже, помнится, узнал о проблеме безопасности, считаю, поздновато. Нет, ничего не сломали, но, помнится, зафиксировал попытку ввода "странных" данных - скобки, кавычки и пр. Тогда даже не понял, зачем...
          Ответить
          • А нас в универе на курсаче натаскали, препод которому сдавали, ох как любил в геты и формы пихать одинарные кавычки! =)
            Ответить
            • ну на одних кавычках далеко не уедешь... по-моему, следовало бы в универах для информатиков сделать специальный курс "информационная безопасность"... тогда бы был толк.
              даа что говорить! если вот в школах деток учат математике, физике, химии, биологии, а межличностным и межполовым отношениям - нет. Напротив, были когда-то предметы в школе "этика", "религия" - так ведь тоже отменили, мол, за ненадобностью! То есть, ребенок тригонометрию среди ночи тебе отчеканит, а как решать повседневные проблемы и конфликты - он либо не знает, либо "просвещается" там и так, как точно только во вред...
              Ответить
              • Оффтопите. Хотя, скажем так, очень даже есть с чем согласиться.
                Ответить
                • по-моему, оффтоп не редкость и весьма даже увлекательно (за пруфами полазьте по гк.ру, залезая в посты, где комментов около 300 или больше)
                  Ответить
                  • Да я понимаю... Просто как раз со внезапного оффтопа обычно такие 300-комментные простыни и стартуют. Не хотет.
                    Ответить
              • у меня, между прочим, даже мифология была.. отдельный такой предмет. этика тоже. в технаре идеология была =)
                Ответить
            • Ну как, натаскали?
              Ответить
        • и еще, проблема посерьезней в том, что анализ безопасности требует не только опыта, но и внимательных и тщательных проверок (недосмотрел с недосыпу - и полезли в дыру там, где, как казалось, ее точно быть не может)... А у обезьянок если мозг есть, то только в жопе спинной....
          Ответить
          • При организации проекта моделью MVC из форм и псевдо "GET" уж очень трудно пробиться куда-нибудь и чего-нибудь поломать, контроллером все можно отловить и проверить.
            Ответить
            • конечно, можно, особенно если писать параноидальный код. Просто не всегда известно, что где и как надо проверять - что бы и надежность, и функциональность была, и производительность
              Ответить
              • Ну, не критичное, можно повесить на JS, а вот от чего может база пострадать, или хостинг то да, надо поднатужиться и правильно написать, так что, решения есть всегда.
                Ответить
                • всмысле, "серверу не больно?"
                  Ответить
                  • Не понял вопроса?
                    Больно конечно, кому будет приятно, если сайт работал работал, а на него, к примеру повесили инклудом брут хешированных паролей от левого сайта, хрена, мощности серваку не занимать, но все же ресурсов будет жечь немерено.
                    Ответить
          • А обезьянки никогда до моделей проектирования информационных систем и не доберутся, так и будут сидеть и весь сайт в index.php хреначить, объемом 8к строк...
            Ответить
            • есть надежда, что некоторыми на 9- ом К копипаста приестся и будут искать способы, как писать поменьше, а заодно - и понятнее. и наконец до финиша единицы доползут
              Ответить
            • > index.php хреначить, объемом 8к строк...
              Тонкий намёк на Rumba CMS? У неё ещё автор с таким пафосным ником ходит - Maestro - и удаляет неугодные комменты X))
              Ответить
              • http://habrahabr.ru/blogs/webdev/103167/

                вот где профи..., эх..., дорасти бы...
                Ответить
          • Смотря как к делу подходить. Если повесить перед собой плакат "товарищ! при составлении запросов приводи целочисленные вещи к целому типу, а строки пропускай через mysql_real_escape_string()!", можно с недосыпу и обломаться. А если всю грязную работу поручить query builder'у, шансы что-то по-крутому зафейлить резко падают. Хотя необходимости использовать голову это тоже не отменяет.
            Ответить
            • где здесь mysql? - т.е. разговор не только о слое работы с бд, но и других.
              Ответить
              • Надо было яснее выразиться. Тот самый недосып сказывается =) Конкретно я комментировал вот это:
                > анализ безопасности требует не только опыта, но и внимательных и тщательных проверок (недосмотрел с недосыпу - и полезли в дыру
                ...а составление запросов привёл лишь как пример того, что в ряде случаев ручную заботу о безопасности возможно эффективно заменить автоматикой. До обезьянок это, кстати, частенько не доходит. Собственно, почему запросы-то вспомнились: мне известны случаи, когда последователи тупых авторов тупой литературы по PHP писали несколько сот килобайт кода с использованием ручного приведения типов и экранирования. Сляпать примитивнейший квери билдер хотя бы для INSERT'ов соображалки недоставало. Вопрос поиска дырок в их поделках сводился лишь к внимательному чтению кода.
                Ответить
                • не всегда все так просто. иногда дыра появляется в результате не до мелочей продуманного и предусмотренного взаимодействия нескольких частей системы - одна часть передает другой данные, думая, что другая их проверит, а другая полностью доверяет первой, почему-то подумав, что за нее все сделала первая.
                  вот в пример громкий провал мелкомягких: http://habrahabr.ru/company/eset/blog/102549/
                  Ответить
                  • За статью спасибо, интересно. А по теме:
                    > не всегда все так просто
                    Так я и говорю,
                    > в ряде случаев
                    .
                    Ответить
                    • отлично, дискуссию можно считать завершенной.

                      а хабрахабр рекомендуется в закладки для ежедневного просмотра. Потому как очень мало столь интересных ИТ-ресурсов
                      Ответить
                  • Недавно в проекте который дорабатывал столкнулся с этим, посмотрел как человек написал, в контроллер данные поступили, он их прямиком в модель, думаю, ну щас там обработает, ага.., щас.. прям в базу без обработки...
                    Ответить
                    • я как то при изучении темы безопасного программирования задался вопросом:

                      при прохождении "потока" данных по слоям приложения где и как нужно эти данные фильтровать?

                      эмпирически потыкав и набив шишек пришел к выводу:
                      фильтровать данные каждый слой должен в своей манере.
                      то есть, когда приходят данные
                      1. Request проверяет правильность формата данных, то есть, если ожидается число, то он должен гарантированно вернуть число или сигнал о нештатной ситуации
                      2. DAO должен эскейпить входящие данные, как минимум через mysql_real_escape_string
                      3. View должен проэкскейпить все данные, к примеру, на htmlentities

                      При этом, например, Request НЕ должен эскейпить данные "сразу" для View'а - иначе рискуем получить кашу и в логике и в представлении
                      Ответить
                      • А я почти к такому же выводу пришел, только на View делаю JS, в котроллере привожу к типу int если где это надо, ну и остальное проверяем, а в модели перед занесением делаю mysql_real_escape_string, пока сбоев не было =)
                        Ответить
                        • > пока сбоев не было
                          или вас не уведомили
                          Ответить
                          • Думаю уведомили бы, если бы произошло что то подозрительное с данными, админы сайта не дремлют))
                            Ответить
                            • ну или просто тихо слили нужные данные, и аукнулось не у вас, а где-то рядом...
                              это я не вас пытаюсь уязвить, такой сценарий возможен у каждого, и вполне возможно, что происходил у меня....
                              Ответить
                        • > на View делаю JS
                          это как понимать? типа rich client application?
                          Ответить
                      • Кеп думает так:
                        Внутри приложения, между подсистемами должны ходить девственно-наивные данные. Если там есть строка '2' < "3" -- то так там и должно быть.
                        Вью должен превращать < в энтити. DAO должен экранировать ".



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

                        А все почему?
                        Потому что ХТМЛ -- нифига не единственный вью. И SQL не единственный DAO.
                        Чем больше прогер об этом помнит -- тем лучше получается приложение.

                        Напиши консольный UI к Вашему приложению. Заставьте его хранить данные в LDAPе. И сразу станет понятно -- где оно красивое, а где -- говно)
                        Ответить
                        • для меня это не было очевидно с самого начала. пробовал даже хранить в бд данные уже "заэнтитированные". но набил шишек и все понял. каждая компонента приложения должна сама проверять, что ей суют, и не беспокоиться о тех, кому еще может передать
                          Ответить
                          • Для меня изначально -- тоже. С опытом приходит)
                            полезно заюзать какой-нить dependency injection, склеить им пяток подсистем, и покрыть каждую юнит-тестами.

                            Сразу мозг становится на место)
                            Ответить
                            • юнит-тесты... весьма необходимая штука, но не панацея от плохого кода
                              а вот мозги проветрить - оно самое. лучше, чем продакшн потом обвалится
                              Ответить
                              • >>но не панацея от плохого кода
                                но ОЧЕНЬ помогает. Если у Вас каша из логики, вью и дао -- то хрен Вы это оттестируете.

                                Именно желание тестировать логику заставило многих писать нормальный код)))
                                Ответить
                                • > Если у Вас каша из логики, вью и дао -- то хрен Вы это оттестируете.
                                  по старинке, дебагом принтами ))))
                                  а так все верно - для тестов нужна модульность и точки входа и выхода.

                                  > Именно желание тестировать логику заставило многих писать нормальный код)))
                                  и сюда еще вспомните test-driven development
                                  Ответить
                                  • >>по старинке, дебагом принтами ))))
                                    А вот что бы не было пагубного желания дебагов и принтов надо настроить continues integration на чем нибудь типа cruise control или teamcity.

                                    Что бы безжалостный робот, не понимающий принтов и дебагов запускал ВСЕ юнит тесты после каждого коммита, и слал письма если тесты упали.
                                    Ответить
                                    • ситуация падения тестов - обычная для процесса разработки, а коммитить нужно чаще, чем заканчивать разработку какого-то модуля

                                      а необходимость дебага - напротив, внештатная ситуация (а мы-то были уверены, что все хорошо!)
                                      Ответить
                                      • ну, закоммиченный код должен, как минимум, компилироваться и работать. Если хочется коммитить чаще -- надо бранчеваться или делать шелф.
                                        Ответить
                                    • *шепотом* Continuous Integration
                                      Ответить
                                  • на счет старинки вспомнил, как было как-то в гостях у знакомого пыхаписта, у которого сына (лет 15ти) отдали обучаться программированию в какой-то крутой кружок.
                                    И там его (как и полагается) учат борланд сям. Тем самым, с IDE на тубробвижине под дос, как меня в 98м:)

                                    Парень писал какую-то игрушку на псевдографике (типа змейки), и запутался в ней. И папаша говорит ему: "сделай принт переменной и посмотри".

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

                                    А потом оказалось что пхпшник пишет в эдитплюсе, и ищет баги с помощью echo, хотя он не глупый совершенно.
                                    Ответить
                                    • > А потом оказалось что пхпшник пишет в эдитплюсе, и ищет баги с помощью echo, хотя он не глупый совершенно.

                                      мне как-то "свезло" работать в одной шарашке, там писали в эдитплюсах, дебажили принтами, разрабатываемую версию клали на тестовый сервак через фтп, и постоянно бегали спрашивали, кто с какими файлами работает, что бы не потерять результаты чужой работы = )
                                      натурально, что мои предложения каждому поставить у себя локальный хампп и эклипс, и начать пользоваться свн'ом не были поняты.
                                      тогда я поработал немного и сбег оттуда - до того, как надо будет раслебывать какой нить тотальный пиздец
                                      Ответить
            • а если юзать PDO + prepared statements, то об sql-inj можно забыть впринципе
              Ответить
              • Кэп, я к тому же клоню, в общем-то. И, опять же, факт в том, что обезьянкам этого не понять.
                Ответить
      • Может я слоу, но где там code-inj? Там только xss же.
        Ответить
        • Там мелькал какой-то странный термин code injecting, прежде чем они провалились в гуммунитарную ноосферу. Самого интересного нет.
          Ответить
        • показать все, что скрытоcode-inj в 4ой строке: echo '<a href="http://$url">123</a>';
          сюда можно целую хтмл страницу вместо $url вляпать -это и будет code injecting, и как частный случай, xss - смотря что туда вляпывать. Например, тот же дефейс произвести - это не xss будет
          Ответить
          • Это Вы, батенька, погорячились. Сюда не то что code injecting, сюда даже законный URL не воткнется!
            Так-то.
            Ответить
            • тьху ты, магический $ сбил с толку, тут же одинарные кавычки, точно
              спасибо за тыкание носом
              Ответить
              • Вообще вы правы. Пхп инекция стала работать вместе с ссылкой, когда я подсказал автору почему код не работал (сам привел оригинальный, как он написал). Он тоже спутал одинарные ковычки с двойными)
                Ответить
    • Ну и нафига постить на ГК даже нерабочий код?
      Весь интерес в том, чтобы автор этого говна с "чистой" совестью мог сказать: "Но ведь работает же!" ))
      Ответить
      • работает. но не так, как надо )))
        Ответить
      • > Весь интерес в том, чтобы автор этого говна с "чистой" совестью мог сказать: "Но ведь работает же!" ))
        Не знал. На будущее учту. Думал что оригинальный код автора будет интереснее, чем работающий после моей подсказки (сказал, чтобы заменил на echo '<a href="'.http://$url.'">123</a>';
        Ответить
        • А это вызовет: Parse error: syntax error, unexpected ':', т.к. "http" будет принято за константу. А оставшаяся часть строки и вовсе закомментирована :)
          Ответить
    • А развели-то, развели... code injecting... xss...
      Кавычки одинарные!
      Ответить
    • http_build_url($url, array('scheme' => 'http'));
      Ответить

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