Site Compass

Défi

Reivernet Group a engagé Redwerk pour l’aider à auditer l’un de ses produits logiciels avant sa sortie officielle. Le logiciel en question était Site Compass, une application de cartographie réseau. Site Compass s’adresse aux entreprises de construction qui souhaitent améliorer leur rentabilité.

Site Compass aide les utilisateurs à gérer les chantiers en centralisant toutes les données nécessaires associées à la planification budgétaire, aux appels d’offres et à la gestion des actifs. L’application simplifie également la génération de rapports, y compris la création de rapports d’audit et de nomenclatures.

Il était crucial pour Reivernet Group qu’une partie indépendante effectue la revue du code. Ils avaient besoin d’une évaluation précise et impartiale de la qualité de leur produit. Notre périmètre de travail comprenait :

• Revue de l’architecture. À cette étape, nous devions nous assurer que les fondations du système étaient bien conçues, répondaient aux exigences et pouvaient s’adapter à la croissance future.
• Revue de la base de données. Ici, nous nous assurons que la base de données est optimisée en termes de performances, de sécurité et d’intégrité des données.
• Revue de la qualité du code. Cette partie comprend l’évaluation de la lisibilité, de la maintenabilité, de la cohérence du code et de son respect des normes de codage modernes. Nous avons été invités à examiner uniquement le code backend.
• Revue de la couverture des tests. Ici, nous nous assurons que le code est adéquatement testé et identifions les domaines où des tests supplémentaires sont nécessaires.
• Revue de sécurité. Notre objectif était d’évaluer l’application par rapport aux 10 principales vulnérabilités Web OWASP et de proposer des stratégies d’atténuation.

En plus de signaler les problèmes trouvés, nous fournissons des recommandations détaillées sur la manière de les résoudre et le temps nécessaire pour refactoriser le code.

Solution

Site Compass est développé en C# avec Uno Platform, ce qui permet d’avoir des applications iOS, Android et Windows dans un seul et même jeu de code. Nous avons des années d’expérience avec toutes les technologies utilisées dans ce projet, ce qui a rendu la tâche assez simple pour nous.

Pour accélérer le processus et nous assurer de détecter chaque erreur, nous avons complété le contrôle manuel par l’automatisation, en utilisant des outils spécialisés tels que NDepend et PVS-Studio. Voici un bref aperçu des problèmes rencontrés à chaque étape.

Examen de l’architecture

L’application possède une architecture en couches avec des couches de présentation, de code et de base de données distinctes. Cette approche est efficace pour les solutions multiplateformes comme Site Compass, grâce à son code partagé qui réduit le temps et le coût de développement. Cependant, elle peut entraîner que certains types ou classes du code deviennent trop génériques et difficiles à maintenir.

Concernant les problèmes critiques, nous avons constaté que certains espaces de noms étaient mutuellement dépendants. Les dépendances circulaires rendent difficile la modification ou le test d’un espace de noms sans affecter l’autre. Ce couplage fort peut nuire à la flexibilité du code et augmenter le temps de développement.

Une solution consisterait à déplacer un ou plusieurs types des espaces de noms de bas niveau vers celui de haut niveau, ou vice versa. Une autre option consiste à utiliser l’inversion de contrôle (IoC) pour introduire une couche d’abstraction entre les composants dépendants et promouvoir un couplage lâche.

Nous avons également constaté que certaines classes étaient trop profondément imbriquées dans l’arbre d’héritage, avec une profondeur d’héritage atteignant 5 à 8 niveaux. Cela s’écarte de la programmation orientée objet qui privilégie la composition à l’héritage.

Les longues chaînes d’héritage sont problématiques car elles violent le principe d’encapsulation, signifiant que les classes « parentes » peuvent exposer des détails d’implémentation aux « enfants ». Ce dernier va à l’encontre de l’objectif d’avoir des couches distinctes, car n’importe qui peut manipuler la structure interne, entraînant une instabilité.

Corriger cela nécessiterait d’analyser les fonctionnalités étroitement couplées et de modifier leur conception en utilisant la composition.

Structure de la base de données

Nous n’avons trouvé aucun problème avec l’architecture et l’évolutivité de la base de données. Azure Cosmos DB offre suffisamment de flexibilité pour l’adapter et résoudre les problèmes de performance. Le seul aspect qui pourrait affecter les performances de l’application est un scénario où la région de l’utilisateur diffère de la région du cloud, entraînant des temps de chargement lents.

Qualité du code backend

Nous avons signalé plusieurs problèmes critiques et de gravité moyenne concernant la qualité du code backend.
Nous avons identifié 38 méthodes avec 7 à 14 paramètres. C’est trop de paramètres. Les méthodes comportant trop de paramètres sont pénibles à appeler et peuvent dégrader les performances.

Une solution consiste à ajouter plus de propriétés/champs au type déclarant pour gérer de nombreux états. Une alternative consiste à fournir une classe ou une structure dédiée à la gestion du passage des arguments.

Le problème suivant concerne les types trop volumineux, résultant du phénomène dit de « classe-dieu » (god class). Une classe-dieu ou objet-dieu est une seule classe qui tente de tout faire. Elle exerce un contrôle excessif sur les autres classes du système, devenant souvent si volumineuse qu’elle est responsable de l’exécution de toutes les tâches. Il en résulte un code difficile à comprendre, à maintenir et à tester.

La correction d’une classe-dieu nécessite de la diviser en classes plus petites avec une responsabilité unique et des limites bien définies. Essayez d’abord de maintenir l’interface de la classe-dieu et déléguez les appels aux nouvelles classes extraites. Finalement, la classe-dieu devrait être une façade pure sans sa propre logique. Vous pouvez alors la conserver par commodité ou la supprimer et commencer à utiliser uniquement les nouvelles classes.

Un autre problème affectant la maintenabilité et la testabilité du code est l’utilisation de champs statiques non immuables (non-readonly). Si la valeur ne change jamais, rendez-la immuable et définissez-la directement dans le constructeur statique ou en ligne avec sa déclaration. Si la valeur change occasionnellement, nous devrions utiliser un champ d’instance à la place. Chaque objet aura sa propre « boîte », évitant les problèmes d’état partagé.

Nous avons également signalé des méthodes trop complexes, potentiellement mortes, mal commentées, ou avec des noms excessivement longs.

Couverture des tests

Seul un faible pourcentage du code était couvert par des tests. Une faible couverture de tests signifie que de larges portions du code restent non testées, ce qui entraîne l’apparition de bugs en production.

Une assurance qualité précoce et continue est vitale car les bugs trouvés en production sont plus coûteux et plus longs à corriger, perturbant les plannings de publication et nécessitant des correctifs urgents. De plus, un test initial inadéquat rend le test de régression moins efficace, car il est difficile de déterminer le comportement original des zones non testées.

Examen de la sécurité

Notre examen de sécurité a inclus la vérification du code pour les vulnérabilités d’injection, telles que les injections NoSQL, LDAP et OS.

Nous nous sommes assurés qu’il n’y avait pas d’occurrences d’authentification brisée, d’exposition de données sensibles, d’analyseurs XML mal configurés, et de configurations par défaut, incomplètes ou ad hoc.

Nos examinateurs ont également recherché des failles permettant des menaces persistantes. Celles-ci permettraient aux attaquants de rester indétectés en cas de violation, compromettant davantage de systèmes et de données.