Site Compass

Desafío

Reivernet Group contrató a Redwerk para que les ayudara a auditar uno de sus productos de software antes de su lanzamiento oficial. Se trataba de Site Compass, una aplicación de cartografía de redes. Site Compass está pensada para empresas de construcción que quieren mejorar su rentabilidad.

Site Compass ayuda a los usuarios a gestionar las obras centralizando todos los datos necesarios relacionados con la planificación presupuestaria, las licitaciones y la gestión de activos. La aplicación también simplifica la elaboración de informes, incluida la generación de informes de auditoría y listas de materiales.

Para Reivernet Group era crucial que un tercero independiente realizara la revisión del código. Necesitaban una evaluación precisa e imparcial de la calidad de su producto. Nuestro ámbito de trabajo incluía:

• Revisión de la arquitectura. En este paso, debíamos asegurarnos de que los cimientos del sistema estaban bien diseñados, cumplían los requisitos y podían adaptarse al crecimiento futuro.
• Revisión de la base de datos. Aquí, nos aseguramos de que la base de datos está optimizada para el rendimiento, la seguridad y la integridad de los datos.
• Revisión de la calidad del código. Esta parte incluye la evaluación de la legibilidad, el mantenimiento, la coherencia y el cumplimiento de las normas de codificación modernas. Se nos pidió que revisáramos únicamente el código backend.
• Revisión de la cobertura de las pruebas. Aquí nos aseguramos de que el código se ha probado adecuadamente e identificamos las áreas en las que se necesitan pruebas adicionales.
• Revisión de la seguridad. Nuestro objetivo era evaluar la aplicación con respecto a las 10 principales vulnerabilidades web de OWASP y proporcionar estrategias de mitigación.

Además de informar de los problemas encontrados, ofrecemos recomendaciones detalladas sobre cómo solucionarlos y cuánto tiempo se necesita para refactorizar el código.

Solución

Site Compass está construido en C# utilizando Uno Platform, que permite que las aplicaciones para iOS, Android y Windows estén todas en un mismo conjunto de código. Tenemos años de experiencia con todas las tecnologías utilizadas en este proyecto, por lo que fue una tarea bastante fácil para nosotros.

Para acelerar el proceso y asegurarnos de detectar todos los errores, completamos la comprobación manual con la automatización, utilizando herramientas especializadas como NDepend y PVS-Studio. He aquí un breve resumen de los problemas que encontramos en cada paso.

Revisión de la arquitectura

La aplicación tiene una arquitectura en capas con capas separadas de presentación, código y base de datos. Este enfoque es eficaz para soluciones multiplataforma como Site Compass debido a su base de código compartida, lo que reduce el tiempo y el coste de desarrollo. Sin embargo, puede llevar a que algunos tipos o clases de la base de código se vuelvan demasiado genéricos y difíciles de mantener.

En cuanto a los problemas críticos, descubrimos que algunos espacios de nombres eran mutuamente dependientes. Las dependencias circulares dificultan la modificación o prueba de un espacio de nombres sin afectar al otro. Este estrecho acoplamiento puede dificultar la flexibilidad del código y aumentar el tiempo de desarrollo.

Una solución sería trasladar uno o varios tipos de los espacios de nombres de bajo nivel al de alto nivel o viceversa. Otra opción es utilizar la inversión de control (IoC) para introducir una capa de abstracción entre los componentes dependientes y promover el acoplamiento débil.

Además, descubrimos que algunas clases eran demasiado profundas en el árbol de herencia, con una profundidad de herencia que alcanzaba los 5-8 niveles. Esto supone una desviación de la programación orientada a objetos, que favorece la composición frente a la herencia.

Las cadenas de herencia largas son problemáticas porque violan el principio de encapsulación, lo que significa que las clases “padre” pueden exponer detalles de implementación a las “hijas”. Esto último contradice el propósito de tener capas separadas, ya que cualquiera puede alterar la estructura interna, lo que conduce a la inestabilidad.

Para solucionarlo habría que analizar las funcionalidades estrechamente acopladas y cambiar su diseño mediante la composición.

Estructura de la base de datos

No encontramos ningún problema con la arquitectura y la escalabilidad de la base de datos. Azure Cosmos DB ofrece suficiente flexibilidad para escalarla y solucionar los problemas de rendimiento. El único aspecto que podría afectar al rendimiento de la aplicación es un escenario en el que la región del usuario difiera de la región de la nube, lo que provocaría tiempos de carga lentos.

Calidad del código backend

Informamos de varios problemas críticos y de gravedad media relativos a la calidad del código del backend.
Identificamos 38 métodos con entre 7 y 14 parámetros. Son demasiados parámetros. Los métodos con demasiados parámetros son dolorosos de llamar y pueden degradar el rendimiento.

Una solución es añadir más propiedades/campos al tipo declarante para manejar numerosos estados. Una alternativa es proporcionar una clase o una estructura dedicada a manejar el paso de argumentos.

El siguiente problema son los tipos demasiado grandes, que dan lugar al fenómeno denominado “clase dios”. La clase dios u objeto dios es una única clase que intenta hacerlo todo. Ejerce un control excesivo sobre otras clases del sistema, a menudo creciendo tanto que se convierte en responsable de realizar todas las tareas. El resultado es un código difícil de entender, mantener y probar.

Arreglar una clase dios requiere dividirla en clases más pequeñas con una única responsabilidad y límites bien definidos. Intente mantener la interfaz de la clase dios al principio y delegue las llamadas a las nuevas clases extraídas. Al final, la clase dios debería ser una pura fachada sin lógica propia. Entonces, puedes mantenerla por conveniencia o desecharla y empezar a usar sólo las nuevas clases.

Otra cuestión que afecta a la mantenibilidad y comprobabilidad del código es el uso de campos estáticos no legibles. Si el valor nunca cambia, hazlo de sólo lectura y establécelo directamente en el constructor estático o en línea con su declaración. Si el valor cambia ocasionalmente, deberíamos utilizar un campo de instancia en su lugar. Cada objeto tendrá su propia “caja”, evitando problemas de estado compartido.

También reportamos métodos demasiado complejos, potencialmente muertos o mal comentados y métodos con nombres excesivamente largos.

Cobertura de las pruebas

Sólo un pequeño porcentaje del código se cubría con pruebas. Una cobertura de pruebas baja significa que grandes partes del código quedan sin probar, lo que provoca que los errores se cuelen en la producción.

El control de calidad temprano y continuo es vital porque los errores detectados en producción son más caros y requieren más tiempo para solucionarlos, lo que altera los calendarios de lanzamiento y exige correcciones urgentes. Además, unas pruebas iniciales inadecuadas restan eficacia a las pruebas de regresión, ya que no está claro cuál era el comportamiento original en las áreas no probadas.

Revisión de seguridad

Nuestra revisión de seguridad incluyó la comprobación del código en busca de vulnerabilidades de inyección, como inyecciones NoSQL, LDAP y OS.

Nos aseguramos de que no hubiera casos de autenticación rota, exposición de datos sensibles, parsers XML mal configurados y configuraciones por defecto, incompletas o ad hoc.

Nuestros revisores también buscaron lagunas que permitieran amenazas persistentes. Estas permitirían a los atacantes pasar desapercibidos en caso de infracción, poniendo en peligro más sistemas y datos.