2

No momento meu site esta como www.site.com.br/blablabla_id_2 para a definição de páginas.

Utilizo o mysqli para fazer pesquisas no banco de dados com esse id.

$id = $_GET['id'];

$result = mysqli_query($con,"SELECT * FROM post WHERE id_post = '$id'");

if(mysqli_num_rows($result) == 0){

    header("Location: 404.php");

}else{

    //Resto do código

}

Terei algum problema com sql injection?

12
  • 9
    Sim vai ter problema, não basta trocar o driver mysql_* pelo mysqli é preciso tratar as entradas do form e usar prepared statements. Aqui tem outras informações sobre sql injection
    – rray
    Commented 7/11/2014 às 20:49
  • 2
    Uma boa prática é pegar o seu código em um ambiente "caixa de areia" e tentar hackear ele. Veja se você consegue fazer algo do tipo tentar acessar a URL "www.site.com.br/blablabla_id_2'--DROP TABLE (coloca o nome de uma tabela aqui)" Commented 7/11/2014 às 20:53
  • 2
    Não precisa de nada de mirabolante, basta usar $id = mysql_real_escape_string( $_GET['id'] ); e fazer isso em todos os parâmetros que já resolve o problema do injection. Depois, para o código ficar mais elegante, até pode usar prepared statements se quiser, em vez do escaping. E se não pretende trocar de base de dados, melhor ficar no mysqli mesmo. Não tem necessidade nenhuma de usar camadas intermediárias e simulação de recursos, como outras libs da moda fazem. Lembrando que prepared statements foram feitos para performance em loops, a proteção é só um "brinde".
    – Largato
    Commented 7/11/2014 às 22:43
  • 1
    Pra quem não quiser ler o Q&A todo que o @MarcioSimao recomendou, segue o resumo: Pro mysqli_real_escape_string() ficar seguro, lembrem se-de usar mysqli_set_charset() antes, setando para um desses 3 parâmetros: utf8, latin1 ou ascii, senão alguns caracteres especiais podem "vazar" e ainda permitir algum ataque (se bem que o bug apontado é pra mysql e não mysqli, mas não custa nada fazer certo).
    – Largato
    Commented 8/11/2014 às 0:37
  • 1
    Eu li o Q&A indicado, e o que diz lá é o oposto. O PDO simula os prepared statements, portanto com PDO você tem o bug com e sem PS. Fora isso, não posso falar pelos outros, mas eu particularmente sempre prefiro usar camada nativa do que essas máscaras de compatibilidade (assim como usar JQuery pra coisa que se resolve com JS, por exemplo). Mas entendo perfeitamente que cada um sabe o que faz de melhor com a experiência que tem. Eu sei que provavelmente nunca vou precisar de PDO pra nada, mas respeito quem prefere. Só não consigo imaginar que essa coisa de trocar de banco sempre seja normal.
    – Largato
    Commented 8/11/2014 às 0:59

4 Respostas 4

4

Juntando o que eu já havia comentado com o Q&A apontado pelo Marcio, segue uma modificação que resolve o problema nas primeiras duas linhas de código, economizando uma camada de aplicação desnecessária para o caso em questão:

mysqli_set_charset( 'utf8' );                   // pode usar 'latin1' ou 'ascii'
$id = mysqli_real_escape_string( $_GET['id'] ); // aqui estamos 100% sanitizados.

$result = mysqli_query($con,"SELECT * FROM post WHERE id_post = '$id'");
if(mysqli_num_rows($result) == 0){
    header("Location: 404.php");
}else{
    //Resto do código
}

Notas:

  • A primeira linha é extremamente importante para evitar uma vulnerabilidade que permite certos caracteres escaparem da sanitização. Setando a página correta, a interpretação do escape funciona como esperado.

  • Na segunda linha fazemos a sanitização propriamente dita.

  • Importante notar que apesar de você estar trabalhando com dados numéricos, não removi as aspas da sua query propositalmente, como uma proteção extra. Se preferir, pode removê-las, mas forçando algo no sentido do $id ser mesmo numérico (somando com zero, por exemplo).

1
  • 1
    Agradeço a colaboração de todos. Acho que vou ficar com esse código que mais se assemelha ao meu. Obrigado! Espero que evite mesmo os sql injections!
    – yuri
    Commented 8/11/2014 às 20:55
3

Eu faria conforme o marcio Simao propôs, usando PDO, mas se no seu caso for complexo mudar todo o codigo de mysqli para PDO, segue abaixo, uma alternativa:

    <?php 
    class DB{
        private $conn;

        public function getConnection(){
            $this->conn = new mysqli("localhost", "root", "", "mvc");
        }
        public function execReader($SQL){
            return $this->conn->query($SQL);
        }            
    }

    $id = filter_input(INPUT_GET, 'id', FILTER_SANITIZE_NUMBER_INT);
    $SQL = "SELECT * FROM produtos WHERE id = ".  addslashes($id);

    $DB = new DB();
    $DB->getConnection();
    $query = $DB->execReader($SQL);
    if ($query == 0) {
        header('Location: 404.php');
        exit();
    }

    $vo = new ProdutoVO();
    while($reg = $query->fetch_array(MYSQLI_ASSOC)){
        $vo->setId($reg["id"]);
        $vo->setNome($reg["nome"]);
        $vo->setMarca($reg["marca"]);
        $vo->setPreco($reg["preco"]);
    }

    var_dump($vo);
    ?>
1
  • tirei 3 linhas em branco do seu código pra não aparecer barra de scroll e caber tudo na tela de uma vez. Se achar que não ficou bom, fique à vontade para reverter a edição.
    – Largato
    Commented 8/11/2014 às 2:19
3

Seu código está totalmente vulnerável a SQL injection, para prevenir invasões você pode utilizar dois recursos bastante úteis, os filtros de entrada e o PDO para que sua query seja parametrizada.

Seu código com a utilização destes recursos se torna:

$id = filter_input(INPUT_GET, 'id', FILTER_SANITIZE_NUMBER_INT);

// Depois de criar o objeto PDO com o driver do seu banco de dados, utilize o código abaixo

$Ps = $Pdo->prepare('SELECT * FROM post WHERE id_post = :id');

$Ps->execute([':id', $id]);

if ($Ps->rowCount() == 0) {
    header('Location: 404.php');
    die();
} else {
    // Pelo menos um registro foi localizado
}
2

Amigo, para resolver esse problema e facilitar sua vida em relação a consultas em banco de dados dentre outras diversas funções, tente utilizar o "PDO PHP", um exemplo que resolveria seu problema seria esse:


prepare("INSERT INTO REGISTRY (name, value) VALUES (:name, :value)");
$stmt->bindParam(':name', $name);
$stmt->bindParam(':value', $value);

// insert one row
$name = 'one';
$value = 1;
$stmt->execute();

// insert another row with different values
$name = 'two';
$value = 2;
$stmt->execute();
?>

Dê uma olhada na documentação do Prepared Statements PHP e na do PDO

1

Esta não é a resposta que você está procurando? Pesquise outras perguntas com a tag .