Analyse PL/SQL avec SonarQube – Criticals

PLSQL_CriticalDans le post précédent de cette série sur l’analyse de code PL/SQL avec SonarQube, nous avons examiné les régles Blockers existantes dans notre Quality Profile.

Nous avons rencontré à cette occasion 3 violations aux ‘best practices’ de programmation PL/SQL dont les conséquences sont d’une gravité telle qu’aucune tolérance n’est envisageable pour celles-ci. Ce qui justifie donc leur statut de ‘Blockers’.

Nous avons également constaté un total de 18 défauts pour ces 3 règles, donc ce nombre limité nous laisse supposer que celles-ci sont connues de l’équipe de projet

Enfin, ces défauts ont pour conséquence un bug de logique applicative dans l’application – une opération qui ne sera jamais effectuée car la condition correspondante ne sera jamais rencontrée – voire un crash pur et simple.

Ces trois règles Blockers sont donc orientées robustesse ou solidité (Reliability) de l’application, ce qui nous convient très bien. En effet, nous souhaitons mettre en avant les défauts qui impactent directement l’utilisateur, donc la fiabilité de l’application mais également tout ce qui touche à la Sécurité et à la Performance, tout ce qui peut éviter un possible piratage de l’application, une corruption de données ou des traitements trop long qui là encore vont impacter l’utilisateur final.

Voyons si nous rencontrons des défauts de ce type parmi les règles Critical.

Les règles de type Critical

Criticals

Nous disposons à nouveau de 3 règles seulement, concernant cette catégorie des violations Critical. La première de celles-ci, ‘Sensitive SYS owned functions should not be used’ concerne l’utilisation de packages Oracle, tels que:

  • UTLDefectsUTL_FILE pour gérer des fichiers et des répertoires.
  • UTL_SMTP pour gérer des emails.
  • UTL_TCIP pour gérer des connections TCP, pour aller lire ou écrire sur un serveur distant par exemple.

Il existe bien d’autres packages Oracle de fonctions telles que celles-ci, mais ce sont en tout cas ceux que nous pouvons rencontrer dans le code analysé.

Je dois dire que je ne connaissais pas cette norme. Je ne suis pas un expert en sécurité, et en fait, il existe assez peu d’experts sécurité en matière de base de données.

Bien évidemment, je connais le principe d’injection SQL, qui permet à un hacker de pénétrer au niveau de la base de données, mais dans ce cas, celui-ci aura les mêmes droits qu’un simple utilisateur. Par contre, s’il rencontre une de ces procédures exécutées par un utilisateur de type ‘SYS’, alors, selon ces experts, c’est « GAME OVER ». Cela revient à donner les droits d’administrateur de la base de données, et donc de faire tout ce qu’il est possible de faire, y compris de supprimer tous les accès.

Manifestement, avec 320 défauts identifiés par SonarQube dans le code analysé, ces normes ne sont pas connues. Les remplacer prendra également un peu de temps.

PLSQL_SysSunburst

Comme nous pouvons le voir dans ce diagramme Sqale (plugin commercial de SonarQube), 40 jours sont nécessaires pour remédier à ces 320 défauts. Je recommanderais donc un chantier spécifique sur ces corrections, à court terme.

La deuxième de ces 3 règles concerne l’utilisation d’instructions DELETE ou UPDATE sans clause WHERE, ce qui revient à supprimer ou à mettre à jour tous les enregistrements d’une table. Il peut arriver qu’on effectue un DELETE brutal sur une table temporaire mais un UPDATE est très peu fréquent. Dans tous les cas, nous souhaitons pouvoir vérifier si l’utilisation de cette instruction se justifie ou non.

Update Avec SonarQube, un drill-down au niveau du code nous permet d’identifier immédiatement l’instruction et de vérifier, en l’occurence, qu’il s’agit fort probablement d’un défaut de programmation. Quelqu’un va devoir s’expliquer !

D’autant que ce même bloc de code est répété à nouveau 8 fois dans le même programme ! Nous avions déjà rencontré ce cas avec les Blockers, et un bug Copié-Collé à plusieur reprises. Une de mes recommandations sera de vérifier si au moins une personne dans l’équipe de projet nécessite une petite formation aux best practices de programmation PL/SQL.

Enfin nous rencontrons 1 unique violation concernant la règle ‘Avoid CROSS JOIN queries’, qui identifie des requêtes sur 2 tables (ou plus) sans préciser de jointure entre celles-ci, ce qui a pour effet de retourner le produit cartésien de l’ensemble des données de ces 2 tables. Un tel défaut peut avoir pour conséquence un bug logique pour l’utilisateur, très certainement un problème de performance, voire même une possible erreur (donc corruption) de données.

On pourrait d’ailleurs justifier de remonter cette règle, voire même les précédentes, au niveau des Blockers. Vous allez me dire que dans ce cas, nous n’aurons plus de règles Criticals ? Si, car nous allons déplacer certaines règles de type Major en Criticals, celles qui présentent également un impact sur l’utilisateur en matière de Robustesse, Sécurité et Performance.

C’est ce que nous verrons dans le prochain post sur les règles Major. A bientôt.

Laisser un commentaire

Votre adresse e-mail ne sera pas publiée. Les champs obligatoires sont indiqués avec *