Eles costumam publicar códigos e memes nojentos sobre programação. Mas um dia eu vi algo absolutamente incrível lá.
Este código ganhou o título honorário de "melhor trabalho" da semana.
Decidi desmontar esse código, mas há tantas coisas erradas que é difícil para mim até escolher o primeiro problema para análise.
Se você é um programador iniciante, meu material o ajudará a entender quais erros terríveis foram cometidos por aqueles que escreveram este código.
28 linhas de erros de código
Para tornar mais conveniente, vamos digitar este código no editor:
<script>
function authenticateUser(username, password) {
var accounts = apiService.sql(
"SELECT * FROM users"
);
for (var i = 0; i < accounts.length; i++) {
var account = accounts [i];
if (account.username === username &&
account.password === password)
{
return true;
}
}
if ("true" === "true") {
return false;
}
}
$('#login').click(function() {
var username = $("#username").val();
var password = $("#password").val();
var authenticated = authenticateUser(username, password);
if (authenticated === true) {
$.cookie('loggedin', 'yes', { expires: 1 });
} else if (authenticated === false) {
$("error_message").show(LogInFailed);
}
});
</script>
E ... Bem, realmente - eu não sei por onde começar a analisá-lo. Mas você ainda precisa começar. Decidi dividir as deficiências deste código em três categorias:
- Problemas de segurança.
- Problemas com conhecimentos básicos de programação.
- Problemas de formatação de código.
Problemas de segurança
Este código provavelmente está sendo executado no cliente. Isso é indicado pelo fato de que está envolvido em uma tag
<script>(e também usa jQuery aqui).
Mas não me entenda mal. Este código pareceria igualmente horrível se fosse para o servidor. Mas executá-lo no cliente significa que todos os que puderem ler esse código terão acesso ao banco de dados que é usado nele.
Vamos dar uma olhada na função
authenticateUser:
function authenticateUser(username, password) {
var accounts = apiService.sql(
"SELECT * FROM users"
);
for (var i = 0; i < accounts.length; i++) {
var account = accounts [i];
if (account.username === username &&
account.password === password)
{
return true;
}
}
if ("true" === "true") {
return false;
}
}
A partir da análise de seu código, podemos concluir que em algum lugar existe um objeto
apiServiceque nos fornece um método .sqlcom o qual podemos executar consultas SQL ao banco de dados. Isso significa que se você abrir o console do desenvolvedor enquanto visualiza a página que contém esse código, pode executar qualquer consulta no banco de dados.
Por exemplo, você pode executar uma consulta como esta:
apiService.sql("show tables;");
Em resposta, uma lista completa de tabelas do banco de dados será retornada. E tudo isso é feito usando a própria API do projeto. Mas, o que já está aí, vamos imaginar que isso não seja um problema real. Vamos dar uma olhada melhor nisso:
if (account.username === username &&
account.password === password)
Este código me diz que as senhas são armazenadas no projeto sem hash. Grande jogada! Isso significa que posso pegar o depurador e ver as senhas dos usuários do projeto. Também acredito que uma grande porcentagem de usuários usa o mesmo par de nome de usuário / senha em mídias sociais, serviços de e-mail, aplicativos bancários e outros lugares semelhantes.
Também vejo o problema de como os cookies são definidos neste código
loggedin:
$.cookie('loggedin', 'yes', { expires: 1 });
Acontece que ele usa jQuery para definir um cookie que informa ao aplicativo da web se o usuário está autenticado.
Lembre-se: nunca defina esses cookies usando JavaScript.
Se você precisa armazenar esse tipo de informação no cliente, os cookies são usados com mais frequência para isso. Não posso dizer nada de ruim sobre essa ideia. Mas definir um cookie usando JavaScript significa que o atributo não pode ser configurado
httpOnly. Isso, por sua vez, significa que qualquer script malicioso pode acessar esses cookies.
E sim, eu sei que apenas algo como um par chave: valor é armazenado aqui,
'loggedin': 'yes', então, mesmo que o script de outra pessoa leia algo assim, não haverá muito dano com ele. Mas isso é uma prática muito ruim de qualquer maneira.
Além disso, quando abro o console do Chrome, sempre posso inserir um comando como
$.cookie('loggedin', 'yes', { expires: 1000000000000 });. Como resultado, descobri que loguei no site para sempre, mesmo sem ter uma conta nele.
Problemas com conhecimento básico de programação
Podemos conversar e falar sobre esses problemas que podem ser encontrados neste trecho de código, e não temos muito tempo.
Portanto, obviamente, uma função
authenticateUseré um exemplo de código muito mal escrito. Isso demonstra que a pessoa que o escreveu não possui alguns conhecimentos básicos de programação.
function authenticateUser(username, password) {
var accounts = apiService.sql(
"SELECT * FROM users"
);
for (var i = 0; i < accounts.length; i++) {
var account = accounts [i];
if (account.username === username &&
account.password === password)
{
return true;
}
}
if ("true" === "true") {
return false;
}
}
Talvez em vez de obter uma lista completa de usuários do banco de dados, basta selecionar um usuário com um determinado nome e senha? O que acontecerá se milhões de usuários forem armazenados neste banco de dados?
Já falei sobre isso, mas vou me repetir, fazendo a seguinte pergunta: "Por que eles não hash as senhas no banco de dados?"
Vamos agora dar uma olhada no que a função retorna
authenticateUser. Pelo que posso ver, posso concluir que leva dois argumentos de tipo stringe retorna um único valor de tipo boolean.
Portanto, o seguinte trecho de código, embora tenha sido escrito de forma horrível, não pode ser chamado de sem sentido:
for (var i = 0; i < accounts.length; i++) {
var account = accounts [i];
if (account.username === username &&
account.password === password)
{
return true;
}
}
Se você traduzir para a linguagem comum, obterá algo assim: “Existe um usuário com o nome X e com a senha Y? Se sim, retorne verdadeiro. "
Agora, vamos dar uma olhada neste pedaço de código:
if ("true" === "true") {
return false;
}
Absurdo. Por que as funções simplesmente não retornam
false? Por que ela precisa de uma condição que é sempre verdadeira?
Agora vamos analisar o seguinte código:
$('#login').click(function() {
var username = $("#username").val();
var password = $("#password").val();
var authenticated = authenticateUser(username, password);
if (authenticated === true) {
$.cookie('loggedin', 'yes', { expires: 1 });
} else if (authenticated === false) {
$("error_message").show(LogInFailed);
}
});
A parte jQuery deste código parece boa. Mas o problema aqui é a organização do trabalho com cookies
loggedin.
Já dissemos que, mesmo sem uma conta, você pode simplesmente abrir o console do Chrome e executar o comando
$.cookie('loggedin', 'yes', { expires: 1 });e ser autenticado por um dia.
Como esta página sabe quem é o usuário autenticado? Talvez ele apenas exiba algo especial, destinado apenas àqueles que foram aprovados na verificação de nome de usuário e senha e não exibem nenhum dado pessoal? Não saberemos disso.
Problemas de formatação de código
O estilo é provavelmente o menor e mais insignificante problema neste código. Mas indica claramente que a pessoa que criou esse código copiou e colou seus fragmentos individuais de algum lugar.
Aqui está um exemplo de uso de aspas duplas ao formatar strings:
var username = $("#username").val();
var password = $("#password").val();
E em outros lugares, aspas simples são usadas:
$.cookie('loggedin', 'yes', { expires: 1 });
Isso pode parecer sem importância, mas na verdade nos diz que o desenvolvedor pode ter copiado algum código de, digamos, StackOverflow, e nem mesmo o reescreveu, seguindo o guia de estilo usado no projeto. Claro, este é um pequeno detalhe, mas indica que o desenvolvedor realmente não se preocupa em entender como o código funciona. Este desenvolvedor só precisa do código para funcionar de alguma forma.
Gostaria de esclarecer aqui meu ponto de vista sobre esse problema. Eu procuro todos os dias no Google por algo relacionado a código, mas acho que é muito mais importante entender, por exemplo, como definir cookies, em vez de fazer um trecho de código copiado impensadamente de algum lugar funcionar. E se por algum motivo o programa parar de funcionar? Como um programador que não entende o que está acontecendo em seu código pode encontrar um erro?
Resultado
Estou absolutamente certo de que este código é falso. Aqui, vi pela primeira vez um exemplo de execução síncrona de uma consulta SQL:
var accounts = apiService.sql(
"SELECT * FROM users"
);
Normalmente, essas tarefas são resolvidas da seguinte forma:
var accounts = apiService.sql("SELECT * FROM users", (err, res) => {
console.log(err); //
console.log(res); //
});
Eles também são resolvidos assim:
var accounts = await apiService.sql(
"SELECT * FROM users"
);
Mesmo que o método
apiService.sqlretorne um resultado em modo síncrono (o que eu duvido), ele precisa abrir uma conexão com o banco de dados, executar uma consulta, enviar o resultado para o ponto de chamada. E essas operações (como você pode imaginar) não podem ser realizadas de forma síncrona.
Mas mesmo que este seja um código completamente real, tenho certeza de que foi escrito por um programador novato, júnior. Tenho certeza de que, quando trabalhei como programador nas primeiras semanas, também escrevi um código terrível (desculpe: D).
Mas isso não é culpa do programador júnior.
Digamos que este seja um código real em execução em algum lugar. Algum júnior faria tudo para que esse código funcionasse. Este desenvolvedor ainda precisa aprender a lidar adequadamente com consultas SQL, cookies e tudo mais. E sob essa luz, é completamente normal.
Mas o chefe desse programador deve controlar o trabalho, apontar erros e fazer com que o iniciante entenda seus erros. Isso levará ao fato de que esse código terrível não chega à produção.
Sei com certeza que existem empresas que não se importam com a qualidade do código que entra em produção. O código resolve o problema? Em caso afirmativo, ele está sendo implantado sem quaisquer perguntas. O código foi escrito por um jovem e nem mesmo testado por um programador mais experiente? Na produção isso!
Em geral, existem tristezas na vida.
Atualização em 8 de agosto de 2020
Quando estava escrevendo este artigo, acreditava que o código que estava analisando era falso. Mas depois de discutir o material no Reddit, recebi um link para este tópico. Como se viu, esse código é usado em algum sistema interno que suporta 1.500 usuários. Então, eu estava obviamente errado. Este é um código totalmente real.
Você já encontrou fragmentos de código francamente ruins na produção, cheios de todos os tipos de problemas?
