1

Esses dias um cara me disse: "evite formar SQLs ao concatenar pedaços de Strings. Fazer isso é pedir para ter problemas de segurança com injeção de SQL, e por esse motivo é considerado uma péssima prática de programação."

E quanto a mesclar variáveis a uma String pra montar um SQL, tipo isso:

 con.atualizar("INSERT INTO CLIENTES (nome, sexo, nascimento, cpf, renda"
                    + " VALUES: ('"+nome+"', '"+sexo+"', '"+nasc+"', '"+cpf+"', '"+renda+"');");

E o método atualizar:

public void atualizar(String sql)throws SQLException{

        PreparedStatement st = con.prepareStatement(sql);

        try{
        st.executeUpdate(sql);

        }catch (SQLException e){
            System.out.println("Erro na atualização. RollBack será efetuado.");
            con.rollback();
        }
    }

Pode ser problemático também?

3
  • 1
    Use PreparedStatement, isso ai além de deixar o código ruim de ler, ainda da brecha para essa falha.
    – user28595
    Commented 10/02/2018 às 19:45
  • @Articuno Sim, no método atualizar eu pego essa String passada por parâmetro e com ela eu crio um PreparedStatement: public void atualizar(String sql)throws SQLException{ PreparedStatement st = con.prepareStatement(sql); Commented 10/02/2018 às 19:47
  • 1
    Entao edite a pergunta e adicione o trecho completo, porque nesse trecho ai, a brecha ainda continua.
    – user28595
    Commented 10/02/2018 às 19:48

1 Resposta 1

2

Sim, pode causar problemas, pois permitiria que o usuário pudesse manipular a query. Nesta resposta há uma demonstração simples, porém perigosa de como um atacante poderia explorar isso. Por isso existe a classe PreparedStatement, para prover segurança ao se tratar dados que venham direto do usuário. Nesta outra resposta há uma boa explicação do porquê utilizar essa classe.

O PreparedStatement tem como característica permitir parametrizar entradas de usuários a serem concatenadas com a query. No código apresentado, apesar de utilizar esta classe, você só criou a variável, mas não parametrizou nada e está passando direto na query.

Para parametrizar, você precisa isolar os dados recebidos da query e deixar a cargo do PreparedStatement a concatenação. Atente-se ao tipo de dado recebido para poder chamar o método set equivalente correto. Supondo alguns dos tipos das suas variáveis, ficaria mais ou menos assim:

con.atualizar(nome, sexo, nasc, cpf, renda);

[...]           

public void atualizar(String nome,char sexo, java.util.Date nasc, String cpf, double renda)throws SQLException{

    String query = "INSERT INTO CLIENTES (nome, sexo, nascimento, cpf, renda)"
                + " VALUES: (?, ?, ?, ?, ?);"

    try{

      PreparedStatement st = con.prepareStatement(query);

      st.setString(1, nome);
      st.setString(2, sexo);
      st.setDate(3, new java.sql.Date(nasc.getTime()));
      st.setString(4, cpf);
      st.setDouble(5, renda);

      st.executeQuery();

    }catch (SQLException e){
        System.out.println("Erro na atualização. RollBack será efetuado.");
        con.rollback();
    }
}

Claro que nao se pode confiar apenas nisso como unica maneira de prevenir brechas de injeção, é só uma camada das várias que se deve ter para proteger sua aplicação.

7
  • Só não entendi que variável "sql" é essa no teu código. Neste trecho: PreparedStatement st = con.prepareStatement(sql); Commented 10/02/2018 às 21:53
  • 1
    @LucasPletsch bem notado, eu havia esquecido de alterar. executeUpdate é somente para instrucóes de update, para insert e select é executeQuery.
    – user28595
    Commented 10/02/2018 às 21:56
  • outra coisa: Se eu fizer da forma que você mostrou, eu não poderia ter um método genérico de "Atualizar" as tabelas. Entende? O meu método aceita qualquer insert, ou delete, drop, etc. Mas pelo que você está me dizendo, também não é seguro ter um método desse tipo, certo? Ou posso ter, a menos que coloque outras camadas de segurança? Commented 10/02/2018 às 21:56
  • 1
    @LucasPletsch então sua modelagem está errada. A query nao deveria ser conhecida fora das classes de modelo, pelo que entendi do código inicial, você está passando a query para o modelo, o que quebra o MVC, pois as classes do modelo são as unicas que deveriam conhecer como conversar com o banco. Se precisa de variantes para este mesmo método, utilize sobrecarga de métodos.
    – user28595
    Commented 10/02/2018 às 21:58
  • 1
    @LucasPletsch num cenário mais simplicado de modelagem, sim. Claro que existem outros cenários onde você pode generalizar isso com outros métodos, mas eu não tenho muita prática com outros, senão explicaria na resposta. Mas sobrecarga de métodos já ajudaria no seu caso.
    – user28595
    Commented 10/02/2018 às 22:02

Você deve fazer log-in para responder a esta pergunta.

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