A menudo publican fragmentos de código repugnante y memes sobre programación. Pero un día vi algo absolutamente asombroso allí.
Este código se ha ganado el título honorífico de "mejor trabajo" de la semana.
Decidí desensamblar este código, pero hay tantas cosas incorrectas que me resulta difícil incluso elegir el primer problema para analizarlo.
Si eres un programador principiante, mi material te ayudará a comprender los terribles errores que cometieron quienes escribieron este código.
28 líneas de errores de código
Para hacerlo más conveniente, escriba este código en el 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>
Y ... Bueno, de verdad, no sé por dónde empezar a analizarlo. Pero aún necesitas empezar. Decidí dividir las deficiencias de este código en tres categorías:
- Temas de seguridad.
- Problemas con los conocimientos básicos de programación.
- Problemas de formato de código.
Temas de seguridad
Es probable que este código se esté ejecutando en el cliente. Esto se indica por el hecho de que está envuelto en una etiqueta
<script>(y, además, aquí se usa jQuery).
Pero no me malinterpretes. Este código se vería igual de horrible si fuera para el servidor. Pero ejecutarlo en el cliente significa que todos los que puedan leer este código tendrán acceso a la base de datos que se utiliza en él.
Echemos un vistazo a la función
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;
}
}
Del análisis de su código, podemos concluir que en algún lugar existe un objeto
apiServiceque nos brinda un método .sqlcon el que podemos ejecutar consultas SQL a la base de datos. Esto significa que si abre la consola del desarrollador mientras visualiza la página que contiene este código, puede ejecutar cualquier consulta en la base de datos.
Por ejemplo, puede ejecutar una consulta como esta:
apiService.sql("show tables;");
En respuesta, se devolverá una lista completa de las tablas de la base de datos. Y todo esto se hace utilizando la propia API del proyecto. Pero, lo que ya está ahí, imaginemos que esto no es un problema real. Echemos un mejor vistazo a esto:
if (account.username === username &&
account.password === password)
Este código me dice que las contraseñas se almacenan en el proyecto sin hash. ¡Gran movimiento! Esto significa que puedo tomar el depurador y ver las contraseñas de los usuarios del proyecto. También creo que un gran porcentaje de usuarios usa el mismo par de nombre de usuario / contraseña en redes sociales, servicios de correo electrónico, aplicaciones bancarias y otros lugares similares.
También veo el problema de cómo se establecen las cookies en este código
loggedin:
$.cookie('loggedin', 'yes', { expires: 1 });
Resulta que usa jQuery para configurar una cookie que le dice a la aplicación web si el usuario está autenticado.
Recuerde: nunca configure estas cookies usando JavaScript.
Si necesita almacenar este tipo de información sobre el cliente, las cookies se utilizan con mayor frecuencia para ello. No puedo decir nada malo de tal idea. Pero configurar una cookie con JavaScript significa que el atributo no se puede configurar
httpOnly. Esto, a su vez, significa que cualquier script malicioso puede acceder a estas cookies.
Y sí, sé que aquí solo se almacena algo como un par clave: valor,
'loggedin': 'yes', por lo que incluso si el guión de otra persona lee algo como esto, no habrá mucho daño. Pero esta es una práctica muy mala de todos modos.
Además, al abrir la consola de Chrome, siempre puedo ingresar un comando como
$.cookie('loggedin', 'yes', { expires: 1000000000000 });. Como resultado, resulta que inicié sesión en el sitio para siempre, incluso sin tener una cuenta en él.
Problemas con los conocimientos básicos de programación.
Podemos hablar y hablar sobre problemas similares que se pueden encontrar en este código, y no tenemos mucho tiempo.
Entonces, obviamente, una función
authenticateUseres un ejemplo de código muy mal escrito. Demuestra que la persona que lo escribió carece de conocimientos básicos de programación.
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;
}
}
¿Quizás en lugar de obtener una lista completa de usuarios de la base de datos, sea suficiente seleccionar un usuario con un nombre y una contraseña? ¿Qué sucede si millones de usuarios se almacenan en esta base de datos?
Ya hablé de esto, pero me repetiré, haciendo la siguiente pregunta: "¿Por qué no ponen hash a las contraseñas en su base de datos?"
Echemos ahora un vistazo a lo que devuelve la función
authenticateUser. Por lo que puedo ver, puedo inferir que toma dos argumentos de tipo stringy devuelve un solo valor de tipo boolean.
Por lo tanto, el siguiente fragmento de código, aunque está escrito horriblemente, no puede considerarse sin sentido:
for (var i = 0; i < accounts.length; i++) {
var account = accounts [i];
if (account.username === username &&
account.password === password)
{
return true;
}
}
Si lo traduce al lenguaje común, obtendrá algo como esto: “¿Hay un usuario con el nombre X y la contraseña Y? Si es así, devuelva verdadero ".
Ahora echemos un vistazo a este código:
if ("true" === "true") {
return false;
}
Disparates. ¿Por qué las funciones no regresan
false? ¿Por qué necesita una condición que siempre sea cierta?
Ahora analicemos el siguiente 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);
}
});
La parte jQuery de este código se ve bien. Pero el problema aquí es la organización del trabajo con cookies
loggedin.
Ya hemos dicho que, incluso sin una cuenta, simplemente puede abrir la consola de Chrome y ejecutar el comando
$.cookie('loggedin', 'yes', { expires: 1 });y estar autenticado por un día.
¿Cómo sabe esta página quién es el usuario autenticado? ¿Quizás solo muestra algo especial, destinado solo para aquellos que pasaron con éxito la verificación de nombre de usuario y contraseña y no muestran ningún dato personal? No lo sabremos.
Problemas de formato de código
El estilo es probablemente el problema más pequeño e insignificante de este código. Pero indica claramente que la persona que creó este código copió y pegó sus fragmentos individuales desde algún lugar.
A continuación, se muestra un ejemplo del uso de comillas dobles al formatear cadenas:
var username = $("#username").val();
var password = $("#password").val();
Y en otros lugares, se utilizan comillas simples:
$.cookie('loggedin', 'yes', { expires: 1 });
Puede parecer poco importante, pero en realidad nos dice que el desarrollador puede haber copiado algún código de, digamos, StackOverflow, sin siquiera reescribirlo, siguiendo la guía de estilo utilizada en el proyecto. Por supuesto, este es un pequeño detalle, pero indica que al desarrollador realmente no le importa comprender cómo funciona el código. Este desarrollador solo necesita el código para funcionar de alguna manera.
Me gustaría aclarar aquí mi punto de vista sobre este problema. Busco en Google cosas relacionadas con el código todos los días, pero creo que es mucho más importante entender cómo configurar una cookie, por ejemplo, en lugar de hacer que un fragmento de código copiado sin pensar de algún lugar funcione. ¿Qué pasa si por alguna razón el programa deja de funcionar? ¿Cómo puede un programador que no comprende lo que sucede en su código encontrar un error?
Salir
Estoy absolutamente seguro de que este código es falso. Aquí vi por primera vez un ejemplo de ejecución sincrónica de una consulta SQL:
var accounts = apiService.sql(
"SELECT * FROM users"
);
Por lo general, estas tareas se resuelven de la siguiente manera:
var accounts = apiService.sql("SELECT * FROM users", (err, res) => {
console.log(err); //
console.log(res); //
});
También se resuelven así:
var accounts = await apiService.sql(
"SELECT * FROM users"
);
Incluso si el método
apiService.sqldevuelve el resultado en modo síncrono (lo cual dudo), necesita abrir una conexión a la base de datos, ejecutar una solicitud, enviar el resultado al punto de llamada. Y estas operaciones (como puede adivinar) no se pueden realizar de forma sincrónica.
Pero incluso si este es un código completamente real, estoy seguro de que fue escrito por un programador novato, junior. Estoy seguro de que cuando trabajé como programador durante las primeras semanas, también escribí un código terrible (lo siento: D).
Pero esto no es culpa del programador junior.
Digamos que este es un código real que se está ejecutando en alguna parte. Algún joven haría todo lo posible para que este código funcione. Este desarrollador todavía tiene que aprender a manejar correctamente las consultas SQL, las cookies y todo lo demás. Y bajo esta luz, es completamente normal.
Pero el jefe de este programador debe controlar el trabajo, señalar los errores y lograr que el principiante comprenda sus errores. Esto conducirá al hecho de que un código tan terrible no llegue a producción.
Estoy seguro de que hay empresas a las que no les importa la calidad del código que entra en producción. ¿El código resuelve el problema? Si es así, se está implementando sin preguntas. ¿El código está escrito por un junior y ni siquiera lo ha probado un programador más experimentado? ¡En producción!
En general, hay dolores en la vida.
Actualización al 8 de agosto de 2020
Cuando estaba escribiendo este artículo, creí que el código que estaba analizando era falso. Pero después de discutir el material en Reddit, me dieron un enlace a este hilo. Al final resultó que, este código se utiliza en algún sistema interno que admite 1.500 usuarios. Así que obviamente estaba equivocado. Este es un código completamente real.
¿Se ha encontrado con fragmentos de código francamente malos en producción, llenos de todo tipo de problemas?
