2

Tenho na aplicação um código que valida a entrada de dados, ajustando de acordo com o que for necessário, isso seria considerado uma boa ou uma opção ruim?

 public static string ValidaDados(string str)
 {
           //Função simples para evitar ataques de injeção SQL
           if (str == string.Empty || str == "")
               return str;

           string sValue = str; 

           //Valores a serem substituidos
           sValue = sValue.Replace("'", "''");
           sValue = sValue.Replace("--", " ");
           sValue = sValue.Replace("/*", " ");
           sValue = sValue.Replace("*/", " ");
           sValue = sValue.Replace(" or ", "");
           sValue = sValue.Replace(" and ", "");
           sValue = sValue.Replace("update", "");
           sValue = sValue.Replace("-shutdown", "");
           sValue = sValue.Replace("--", "");
           sValue = sValue.Replace("'or'1'='1'", "");
           sValue = sValue.Replace("insert", "");
           sValue = sValue.Replace("drop", "");
           sValue = sValue.Replace("delete", "");
           sValue = sValue.Replace("xp_", "");
           sValue = sValue.Replace("sp_", "");
           sValue = sValue.Replace("select", "");
           sValue = sValue.Replace("1 union select", "");

           //Retorna o valor com as devidas alterações
           return sValue;

 }

Exemplo de uso:

        var tbuscar = new UsuarioAplicacao();
        var retorno = tbuscar.ListarPorLoginSenha(ValidaDados(tabela.LOGIN), ValidaDados(tabela.SENHA));

Exemplo de como está hoje:

public TB_USUARIO ListarPorLoginSenha(string login, string senha)
{
    var strQuery = "";
    strQuery += " select  ";
    strQuery += "  b.DESCRICAO as PERFIL,  ";
    strQuery += "  b.ADMINISTRADOR as ADMINISTRADOR,  ";
    strQuery += "  c.DATA_FIM as DATAFINALASSINATURA, ";
    strQuery += "  c.SITUACAOASSINATURA,            ";
    strQuery += "  a.* ";
    strQuery += "  from TB_USUARIO a ";
    strQuery += "  inner join TB_PERFIL_ACESSO b on a.IDPERFIL = b.IDPERFIL ";
    strQuery += "  left join TB_ASSINATURA c on c.IDUSUARIO = a.IDUSUARIO ";
    strQuery += string.Format("  where a.login = '{0}' and a.senha = '{1}' ", login, senha);
    strQuery += "  and a.USUARIOATIVO = 'S' and a.USUARIOEXCLUIDO = 'N' ";
    strQuery += "  ORDER BY a.IDUSUARIO";

    using (contexto = new Contexto())
    {
        var retornoDataReader = contexto.ExecutaComandoComRetorno(strQuery);
        return TransformaReaderEmListaObjetos(retornoDataReader).FirstOrDefault();
    }

}
8
  • 2
    Utiliza qualquer ORM Commented 14/02/2017 às 12:11
  • 2
    Está usando ADO.NET, EF ou o que? Sabe que essas tecnologias cuidam disto se fizer o certo e não precisa de nada disto? Tem algum motivo para tentar limpar assim?
    – Maniero
    Commented 14/02/2017 às 12:15
  • Exemplifique em qual momento utiliza isso, em qual controller, em qual método e porque está usando isso, acredito que existam possibilidades melhores, mas, falta um contexto para que eu possa opinar.
    – novic
    Commented 14/02/2017 às 12:16
  • 1
    Não uso ORM, uso ADO.net, em uma tela de login eu valido os dados que estou recebendo com este código descrito na pergunta
    – Harry
    Commented 14/02/2017 às 12:22
  • 1
    @itasouza você usa Parameters ? se usar não tem necessidade! um dica também se não usar começar então agora ... rsrsrs
    – novic
    Commented 14/02/2017 às 12:25

2 Respostas 2

6

Você precisa usar SQLCommand. Desta forma a sua query será montada pelo ADO.NET de forma confiável. Se você tentar fazer a limpeza, certamente vai errar, fora o enorme trabalho que dá.

Essa classe tem uma forma de montar o textos com "variáveis" internas onde o dado parametrizado será colocado de forma limpa. Você sempre deve usar esta classe ou alguma outra que faça o mesmo (depende da tecnologia usada).

Vejamos o exemplo da documentação:

private static void UpdateDemographics(int customerID, string demoXml, string connectionString) {
    using (var connection = new SqlConnection(connectionString)) {
        var command = new SqlCommand("UPDATE Sales.Store SET Demographics = @demographics WHERE CustomerID = @ID;", connection);
        command.Parameters.Add("@ID", SqlDbType.Int);
        command.Parameters["@ID"].Value = customerID;

        command.Parameters.AddWithValue("@demographics", demoXml);

        try {
            connection.Open();
            WriteLine($"RowsAffected: {command.ExecuteNonQuery()}");
        } catch (Exception ex) { //só para facilitar, normalmente é se usa assim
            WriteLine(ex.Message);
        }
    }
}

Coloquei no GitHub para referência futura.

Esses "@" são os parâmetros da query. Para passar o valor para a query você usa Parameters.Add() ou suas variações, como demonstrado no código acima.

No seu código após a edição da pergunta seria algo assim:

public TB_USUARIO ListarPorLoginSenha(string login, string senha) { //esse retorno não parece certo
    var strQuery = @"select b.DESCRICAO as PERFIL, b.ADMINISTRADOR as ADMINISTRADOR, c.DATA_FIM as DATAFINALASSINATURA, c.SITUACAOASSINATURA, a.*
                      from TB_USUARIO a
                      inner join TB_PERFIL_ACESSO b on a.IDPERFIL = b.IDPERFIL
                      left join TB_ASSINATURA c on c.IDUSUARIO = a.IDUSUARIO
                      where a.login = @Login and a.senha = @Senha and a.USUARIOATIVO = 'S' and a.USUARIOEXCLUIDO = 'N'
                      order by a.IDUSUARIO";
    using (var conn = new SqlConnection(/* coloca aqui como pega a string */)) {
        var cmd = new SqlCommand(strQuery, conn);
        cmd.Parameters.Add(new SqlParameter("@Login", login));
        cmd.Parameters.Add(new SqlParameter("@Senha", senha));
        //a partir daqui eu não sei o que esse código faz, teria que adaptá-lo para usar o SQLCommand ou fazer de outra forma
        using (var contexto = new Contexto()) {
            return TransformaReaderEmListaObjetos(contexto.ExecutaComandoComRetorno(cmd)).FirstOrDefault();
        }
    }
}
2
  • Adicionei mais informações
    – Harry
    Commented 14/02/2017 às 12:58
  • @itasouza quebrei seu galho, mas esse código não funcionará sem adaptação. Eu fiz o que dava com o que eu sei do seu código.
    – Maniero
    Commented 14/02/2017 às 13:18
1

Use SQLCommands para fazer a inserção dos dados, ele já faz essa limpeza por você. Limpar da forma que você estava sugerindo iria remover as palavras de qualquer texto, mesmo que não fosse uma tentativa de SQL Injection.

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 .