Это предотвратит SQL-инъекции? - PullRequest
Новая
       22

Это предотвратит SQL-инъекции?

0 голосов
/ 19 июля 2011

ОБНОВЛЕНИЕ 1:

Я получаю следующую ошибку: 

Fatal error: Cannot pass parameter 2 by reference in /var/www/page1.php on line 42 Call Stack: 0.0008 341836 1. {main}() /var/www/page1.php:0

При использовании: 

if( $_SERVER['REQUEST_METHOD'] == 'POST') {
    $fields = array(
        'col1'  => 'cb1',
        'col2'  => 'cb2',
        'col3'  => 'cb3',
        'col4'  => 'cb4',
    );
    $parts = array();
    foreach($fields as $dbfield => $field)
        $parts[] = '`' . $dbfield . '` = :' . $dbfield;
    $dbh = new PDO('mysql:host=localhost;dbname=database', 'user', 'pass');
    $dbh->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
    $sth = $dbh->prepare('UPDATE `table1` SET ' . join(', ', $parts) . ' WHERE `id `= :id');
    // temp simulation value
    $id = 1;
    $sth->bindParam(':id', $id, PDO::PARAM_INT, 4);
    foreach($fields as $dbfield => $field)
        $sth->bindParam(':' . $dbfield, isset($_POST[$field]) ? 1 : 0, PDO::PARAM_INT, 1);
    $sth->execute();
}

ОРИГИНАЛЬНЫЙ ВОПРОС:

Будет ли следующий код предотвращать SQL-инъекции? 

<?php
    if( $_SERVER['REQUEST_METHOD'] == 'POST' ) {
        // all the way upto 50
        $fields = array('col1'=>'cb1', 'col2'=>'cb2', 'col3'=>'cb3', 'col4'=>'cb4');

        $update = '';

        foreach($fields as $dbfield => $field) {
            if ($update) $update.= ',';

            $update.= ' '.$dbfield.'=';

            $update .= isset($_POST[$field]) ? 1 : 0;
        }

        $DBH = new PDO( "mysql:host=localhost;dbname=database", "user", "pass" );
        $DBH -> setAttribute( PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION );

        $STH = $DBH -> prepare( "update table1 set " . $update . " where id = :id" );

        // temp simulation value
        $id = 1;

        $STH -> bindParam( ':id', $id, PDO::PARAM_INT, 4 );

        $STH -> execute();
    }
?>

<html>
    <head>
        <title></title>
    </head>

    <body>
        <form method="post">
            <input type="checkbox" name="cb1" />
            <input type="checkbox" name="cb2" />
            <input type="checkbox" name="cb3" />
            <input type="checkbox" name="cb4" />
            <!-- all the way to 50 -->

            <input type="submit" value="submit" />
        </form>
    </body>
</html>

Ответы [ 4 ]

3 голосов
/ 19 июля 2011

Ну, в вашем конкретном случае SQL-инъекция не может пройти через этот код, потому что вы создаете свое обновление только из заранее известных имен столбцов, 1 и 0.Тем не менее, общий способ создания этого обновления будет очень опасной практикой для повторения в других контекстах.Дело в том, что вы используете PDO, но затем вынуждаете его отправлять необработанную строку без экранирования ваших данных, что побеждает все, что может сделать PDO, чтобы попытаться защитить вас.

Чтобы реальноPDO помогает вам против внедрения SQL, вам нужно написать запрос с ? или именованными заполнителями для данных и использовать bindParam для предоставления значений.

Пример динамического построения с использованием привязки именованных параметров можетвыглядеть так: 

if( $_SERVER['REQUEST_METHOD'] == 'POST') {
    $fields = array(
        'col1'  => 'cb1',
        'col2'  => 'cb2',
        'col3'  => 'cb3',
        'col4'  => 'cb4',
    );
    $parts = array();
    foreach($fields as $dbfield => $field)
        $parts[] = '`' . $dbfield . '` = :' . $dbfield;
    $dbh = new PDO('mysql:host=localhost;dbname=database', 'user', 'pass');
    $dbh->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
    $sth = $dbh->prepare('UPDATE `table1` SET ' . join(', ', $parts) . ' WHERE `id `= :id');
    // temp simulation value
    $id = 1;
    $sth->bindParam(':id', $id, PDO::PARAM_INT, 4);
    foreach($fields as $dbfield => $field) {
        $value = isset($_POST[$field]) ? 1 : 0;
        $sth->bindParam(':' . $dbfield, $value, PDO::PARAM_INT, 1);
    }
    $sth->execute();
}
1 голос
/ 19 июля 2011

Поскольку вы вообще не вставляете чужие данные в свою таблицу, я бы сказал, что вы в безопасности.(Однако вы должны прочитать ответы других. Я не могу думать о каждой возможности.)

Однако я не могу сказать, что является заполнителем в вашем коде для вашего упрощенного примера.Если вы решите использовать реальные пользовательские данные, то вам следует начать использовать PDO правильно.На данный момент у вас есть только один заполнитель.Все ваши поля не имеют заполнителей.Во-первых, вы фактически победили цель использования подготовленного запроса.

Если бы вы могли описать, что вы пытаетесь сделать, возможно, мы можем предложить более чистое решение.

0 голосов
/ 19 июля 2011

Это динамический SQL: 

 $STH = $DBH -> prepare( "update table1 set " . $update . " where id = :id" ).

Динамический SQL не может быть защищен от SQL-инъекций, кроме как путем проверки входных данных $update по белому списку утвержденных значений.

Подробнее см. Здесь: Как предотвратить внедрение SQL с динамическими именами таблиц?

Безопасен ли ваш код? (Да)
Ваш код вставляет белый список значений в поля массива и вставляет 1, если входные данные совпадают, и 0, если нет.
Я не вижу возможности внедрения SQL-кода, но это похоже на запах кода.

Предложение по очистке кода от запаха
Лично я бы реструктурировал таблицу в: 

Simple_settings
---------------
new_id integer auto_increment primary key 
user_id integer  <<-- optional can be used to check if a user is allowed to alter
                      the setting.
page_id integer  <<-- replaces your id 
settingname varchar(20)
onoff boolean

Теперь вы можете обновить таблицу, используя обычный PDO: 

UPDATE table1 onoff = :isfieldset WHERE settingname = :dbfield 
AND page_id = :id

Если у вас есть несколько настроек, вам придется делать их в цикле, но, по крайней мере, ваш код будет простым, и ваша таблица не будет иметь миллиард столбцов.
В качестве дополнительного бонуса вам не нужно менять расположение столов при изменении количества настроек.
Обратите внимание, что , если злой пользователь вводит свои собственные данные в: dbfield, то update завершится ошибкой.
Если вы используете это для вставки, остальная часть вашего кода не будет читать эту настройку, так что вы тоже должны быть в порядке.

Осталась одна проблема
Если у вас установлены недействительные флажки (поскольку текущему пользователю не разрешено изменять все настройки) ваш код в настоящее время не проверяет это, если есть какая-то бизнес-логика, вам нужно будет проверить, разрешено ли этому пользователю чтобы войти в эти настройки. Вы можете сделать это с помощью триггера before update в файле settings_table или использовать (предыдущий / добавленный) select против таблицы user_allowed_settings, чтобы увидеть, какому пользователю разрешено устанавливать какие настройки.

0 голосов
/ 19 июля 2011

Нет, потому что, даже если вы используете параметризованные переменные запроса / связывания, переменная $update все еще формируется из неанизированного пользовательского ввода.

...