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

    +147

    1. 01
    2. 02
    3. 03
    4. 04
    5. 05
    6. 06
    7. 07
    8. 08
    9. 09
    10. 10
    11. 11
    12. 12
    13. 13
    function get_avatar($id){
    
    $mysqli = connectDB();
    
    $avatar_get = $mysqli->query("SELECT `avatar` FROM `users` WHERE `id`='$id'");
    
    $line = $result_set->fetch_assoc();
    
    closeDB($mysqli);
    
    return $line["avatar"];
    
    }

    пожалуйста помогите найти ошибку

    Запостил: norto, 17 Марта 2014

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

    • в ДНК
      Ответить
      • Еще ошибка есть: вопрос не на тот сайт запостил ;)

        P.S. А коннект на каждый запрос - это няшно. Я такого чуда еще ни разу не видел ;)
        Ответить
        • А вдруг аватары хранятся (о, ужас!) в отдельной базе?
          Ответить
        • А вдруг это единственный запрос на всю страницу? :)
          Ответить
          • Кстати, как это ни пугающе, но американский коллега-"DBA" однажды высказал (перевод): "а чего это ваше приложение коннекты по минуте держит?! Сконнектились, сделали запрос, отконнектились. Так все делают!" Началось с того, что он сконфигурил таймауты на 5 секунд и у нас посыпались фоновые тяжёлые пересчёты и подгрузки всяких сторонних медиа. :) Чуда не случилось, пришлось ставить костыли для автореконнекта...
            Ответить
    • 1. Нет постановки задачи. Без постановки искать ошибку бессмысленно. Вдруг так и задумывалось?

      2. Кто такой $result_set? Кто его породил?
      Ответить
    • ну так замени в 7 строке result_set на avatar_get
      Ответить
    • Ощибка проектирования - нужно использовать ORM.
      Ответить
    • Я, конечно, не специалист в мускул и похапэ, но меня смущают обратные апострофы в запросе.
      Ответить
      • Всё правильно: в MySQL именно так экранируются имена полей и таблиц.
        Ответить
      • Это специфика MySQL. В нём имена экранируются обратными апострофами, в MS SQL — квадратными скобками, в остальных СУБД — двойными кавычками.

        Проверить корректность запроса — не проблема, всего-то ввести его в консоль СУБД руками и посмотреть, что он возвращает.

        На ошибку здесь явно указал guest: объект запроса сохраняется в переменной $avatar_get, а метод для выполнения запроса дёргается у переменной $result_set, которая наверняка ещё не инициализирована.

        Кстати, новички в пхп часто спотыкаются на том, что запрос выполняется не в одну строку.

        Но самое интересное, откуда приходит значение $id. Если оно выше по коду приходит из параметров запроса ($_GET, $_POST) и не фильтруется, то будет очень плохо...
        Ответить
        • Не не будет туда чего не суй будет только аватар выводится. Даже если вывод ошибок в браузер ничего страшного не будет.
          Ответить
          • в смысле
            в качестве $id что, нельзя передать "Роберт'); DROP TABLE Students;--"?
            Ответить
            • Проморгал этот комментарий.

              Нельзя. mysqli::query() исполняет только один запрос. См. ниже идею багфикса.
              Ответить
              • спасибо, теперь я спокоен
                Ответить
                • Только благодаря чуду ограниченности этой функции некоторые пыхосайты до сих пор не потеряли базу.
                  Ответить
                  • Ну именно по этой причине и ограничили.
                    Ответить
            • Тем не менее, если входящий параметр не обрабатывается, можно наинъектить всякого.
              Что удивительно, даже школьные поделки перешли на mysqli, а вот подготовленных выражений как не было, так и нет.
              Ответить
              • я буквально неделю назад видел вот такой запрос

                select * from $table where id=$id; где $table и $id брались из урла. Урлы были вида /pages/192 pages это таблица 192 id.
                $table обрабатывалось mysql_real_escape_string и автор был уверен что инъекция не пройдет.
                Ответить
                • На старом сайте ксакепа прямо в url была часть sql запроса, прямо с where. Была там какая-то фильтрация, но ломали их один хуй раз в 3 месяца. В последний раз их сильно доснули через sql запросы.
                  Ответить
                  • > но ломали их один хуй раз в 3 месяца
                    Набирали себе сотрудников?
                    Ответить
              • Подготовленные выражения — такое же зло, как магические кавычки, потому что создают иллюзию защищённости.
                Ответить
                • поясните мысль
                  Ответить
                  • См. выше пример, который заботливо предоставил Vasiliy. Если повезёт, то с базы можно сделать снимок конфиденциальной информации.
                    Ответить
                    • в подготовленное выражение ты не можешь дописать sql
                      ты можешь только забиндить значения к параметрам, которые уйдут в СУБД как отдельные значения
                      или я не понял, о каком примере ты говоришь
                      Ответить
                    • В подготовленных выражениях нельзя забиндить имена таблиц и полей. Только параметры для where, только хардкор. Т.е. структуру запроса испортить не получится.

                      А если сайт отдает левым юзерам записи, которые он им не должен бы отдавать - это уже у автора опилки в голове. От этой уязвимости СУБД и ее интерфейсы не застрахуют, разве что завести по юзеру в базе на каждого юзера на сайте...
                      Ответить
                      • > разве что завести по юзеру в базе на каждого юзера на сайте...
                        ... и сделать каждому из них свои вьюхи.

                        Ну либо особый ORM, умеющий в права на записи. Есть такие?

                        В любом случае, больно уж жутко получается ради подстраховки пыхомакаки от фейла...
                        Ответить
                    • Подготовленные выражения защищают от инъекций и больше ничего.
                      Ответить
                • Пешеходные переходы — зло, создают иллюзию защищённости; знаю, раскатывали и на них.

                  Дай дураку половой орган стеклянный, он осколками и порежется.
                  Ответить
          • Верно. Несколько запросов в query протащить нельзя... но можно посоветовать использовать multi_query, чтобы наверняка оставить лазейку!
            Ответить
          • > чего не суй будет только аватар выводится
            Это почему же?
            0' UNION SELECT whatever AS avatar FROM anywhere #комментим лишнюю кавычку
            Несколько колонок можно слепить через CONCAT. Судя по коду тут будет выводиться всего одна строка - можно слепить табличку через GROUP_CONCAT.
            Ответить
            • по моему нельзя union после where щас проверю точно напишу.
              Можно эх печалька
              Ответить
              • > по моему нельзя union после where
                Можно. А на самом деле даже нужно.
                Ответить

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