Nous tirons dans le pied, traitant les données d'entrée

Le lien dans l'article d'aujourd'hui est différent de l'habituel. Ce n'est pas un projet pour lequel le code source a été analysé, mais une série de réponses de la même règle de diagnostic dans plusieurs projets différents. Quel est l'intérêt ici? Le fait que certains des fragments de code considérés contiennent des erreurs reproductibles lorsque vous travaillez avec l'application, tandis que d'autres contiennent des vulnérabilités (CVE). De plus, à la fin de l'article, nous discuterons un peu du sujet des défauts de sécurité.

Brève préface

Toutes les erreurs qui seront examinées aujourd'hui dans l'article ont un schéma similaire:

• le programme reçoit des données du flux stdin ;
• une vérification est effectuée pour la réussite de la lecture des données;
• si les données ont été lues avec succès, le caractère de report est supprimé de la ligne.

Cependant, tous les fragments qui seront considérés contiennent des erreurs et sont vulnérables aux entrées truquées. Étant donné que les données sont reçues d'un utilisateur qui peut violer la logique d'exécution de l'application, il y avait une grande tentation d'essayer de casser quelque chose. Ce que j'ai fait.

Tous les problèmes ci-dessous ont été découverts par l' analyseur statique PVS-Studio , qui recherche des erreurs dans le code non seulement pour C, C ++, mais aussi pour C #, Java.

Bien sûr, trouver un problème avec un analyseur statique est bien, mais trouver et reproduire est un niveau de plaisir complètement différent. :)

Freeswitch

Le premier fragment de code suspect a été trouvé dans le code du module fs_cli.exe , qui fait partie du kit de distribution FreeSWITCH:

static const char *basic_gets(int *cnt) { .... int c = getchar(); if (c < 0) { if (fgets(command_buf, sizeof(command_buf) - 1, stdin) != command_buf) { break; } command_buf[strlen(command_buf)-1] = '\0'; /* remove endline */ break; } .... } 

Avertissement PVS-Studio : V1010 CWE-20 Les données contaminées non contrôlées sont utilisées dans l'index: 'strlen (command_buf)'.

L'analyseur avertit d'un appel suspect par index au tableau command_buf . Il est considéré comme suspect car des données externes non vérifiées sont utilisées comme index. Externe - car ils sont obtenus via la fonction fgets du flux stdin . Non vérifié - car aucune vérification n'a été effectuée avant utilisation. L'expression fgets (command_buf, ....)! = Command_buf ne compte pas, car de cette façon, nous vérifions uniquement le fait de recevoir les données, mais pas leur contenu.

Le problème avec ce code est que, dans certaines conditions, «\ 0» sera écrit en dehors du tableau, ce qui entraînera un comportement non défini. Pour ce faire, il suffit d'entrer une chaîne de longueur nulle (une chaîne de longueur nulle du point de vue du langage C, c'est-à-dire une chaîne dans laquelle le premier caractère sera '\ 0').

Estimons ce qui se passe si vous passez une chaîne de longueur nulle à l'entrée:

• fgets (command_buf, ....) -> command_buf ;
• fgets (....)! = command_buf -> false ( alors la branche de l' instruction if est ignorée);
• strlen (command_buf) -> 0 ;
• command_buf [strlen (command_buf) - 1] -> command_buf [-1] .

Oups!

Ce qui est intéressant ici, c'est que cet avertissement de l'analyseur peut être «ressenti avec vos mains». Pour répéter le problème, vous avez besoin de:

• amener le programme à cette fonction;
• affinez l'entrée afin que l'appel getchar () renvoie une valeur négative;
• passez pour la fonction fgets une ligne avec un terminal zéro au début, qu'elle doit lire avec succès.

En fouillant un peu dans la source, j'ai composé une séquence spécifique pour reproduire le problème:

• Exécutez fs_cli.exe en mode batch ( fs_cli.exe -b ). Je note que pour effectuer d'autres étapes, la connexion fs_cli.exe au serveur doit réussir. Pour ce faire, il suffit, par exemple, d'exécuter FreeSwitchConsole.exe localement en tant qu'administrateur.
• Nous effectuons l'entrée de sorte que l'appel getchar () renvoie une valeur négative.
• Entrez une ligne avec un terminal zéro au début (par exemple, '\ 0Oops').
• ....
• PROFIT!

Voici une lecture vidéo du problème:

Ncftp

Un problème similaire a été découvert dans le projet NcFTP, mais il s'est déjà rencontré à deux endroits. Étant donné que le code semble similaire, ne considérez qu'un seul endroit problématique:

 static int NcFTPConfirmResumeDownloadProc(....) { .... if (fgets(newname, sizeof(newname) - 1, stdin) == NULL) newname[0] = '\0'; newname[strlen(newname) - 1] = '\0'; .... } 

Avertissement PVS-Studio : V1010 CWE-20 Les données corrompues non contrôlées sont utilisées dans l'index: 'strlen (nouveau nom)'.

Ici, contrairement à l'exemple de FreeSWITCH, le code est écrit pire et plus sujet aux problèmes. Par exemple, l'écriture de «\ 0» se produit indépendamment du fait que la lecture a réussi à l'aide de fgets ou non. Autrement dit, il existe encore plus de possibilités pour briser la logique normale d'exécution. Allons d'une manière éprouvée - à travers des lignes de longueur nulle.

Le problème reproduit est un peu plus compliqué qu'avec FreeSWITCH. La séquence des étapes est décrite ci-dessous:

• démarrage et connexion au serveur à partir duquel vous pouvez télécharger le fichier. Par exemple, j'ai utilisé speedtest.tele2.net (à la fin, la commande de lancement d'application ressemble à ceci: ncftp.exe speedtest.tele2.net );
• télécharger un fichier depuis le serveur. Localement, un fichier du même nom mais avec des propriétés différentes devrait déjà exister. Vous pouvez, par exemple, télécharger un fichier à partir du serveur, le modifier et réessayer d'exécuter la commande de téléchargement (par exemple, obtenez 512 Ko.zip );
• répondez à la question sur le choix d'une action avec une ligne commençant par le caractère «N» (par exemple, maintenant amusons-nous );
• entrez '\ 0' (ou quelque chose de plus intéressant);
• ....
• PROFIT!

La reproduction du problème est également enregistrée sur vidéo:

Openldap

Dans le projet OpenLDAP (plus précisément, dans l'un des utilitaires associés), ils ont marché sur le même râteau que dans FreeSWITCH. Une tentative de suppression d'un caractère de saut de ligne se produit uniquement si la ligne a été lue avec succès, mais il n'y a pas non plus de protection contre les lignes de longueur nulle.

Extrait de code:

 int main( int argc, char **argv ) { char buf[ 4096 ]; FILE *fp = NULL; .... if (....) { fp = stdin; } .... if ( fp == NULL ) { .... } else { while ((rc == 0 || contoper) && fgets(buf, sizeof(buf), fp) != NULL) { buf[ strlen( buf ) - 1 ] = '\0'; /* remove trailing newline */ if ( *buf != '\0' ) { rc = dodelete( ld, buf ); if ( rc != 0 ) retval = rc; } } } .... } 

Avertissement PVS-Studio : V1010 CWE-20 Les données contaminées non contrôlées sont utilisées dans l'index: 'strlen (buf)'.

Nous jetons l'excédent pour que l'essence du problème devienne plus évidente:

 while (.... && fgets(buf, sizeof(buf), fp) != NULL) { buf[ strlen( buf ) - 1 ] = '\0'; .... } 

Ce code est meilleur que NcFTP, mais reste vulnérable. Si à la demande fgets pour passer une chaîne de longueur nulle à l'entrée:

• fgets (buf, ....) -> buf ;
• fgets (....)! = NULL -> true (le corps de la boucle while commence à s'exécuter);
• strlen (buf) - 1 -> 0 - 1 -> -1 ;
• buf [-1] = '\ 0' .

libidn

Malgré le fait que les erreurs discutées ci-dessus sont assez intéressantes (elles sont reproduites de manière stable et elles peuvent être «touchées» (à moins que je ne puisse pas toucher le problème OpenLDAP)), elles ne peuvent pas être appelées vulnérabilités, ne serait-ce que pour la raison que les identifiants CVE ne sont pas attribués aux problèmes.

Cependant, certaines vulnérabilités réelles ont le même schéma de problème. Les deux extraits de code ci-dessous s'appliquent au projet libidn.

Extrait de code:

 int main (int argc, char *argv[]) { .... else if (fgets (readbuf, BUFSIZ, stdin) == NULL) { if (feof (stdin)) break; error (EXIT_FAILURE, errno, _("input error")); } if (readbuf[strlen (readbuf) - 1] == '\n') readbuf[strlen (readbuf) - 1] = '\0'; .... } 

Avertissement PVS-Studio : V1010 CWE-20 Les données contaminées non contrôlées sont utilisées dans l'index: 'strlen (readbuf)'.

La situation est similaire, sauf que, contrairement aux exemples précédents, où l'enregistrement a été effectué à l'indice -1 , la lecture a lieu ici. Cependant, ce comportement n'est toujours pas défini. Cette erreur a reçu son propre identifiant CVE ( CVE-2015-8948 ).

Après avoir détecté un problème, ce code a été modifié comme suit:

 int main (int argc, char *argv[]) { .... else if (getline (&line, &linelen, stdin) == -1) { if (feof (stdin)) break; error (EXIT_FAILURE, errno, _("input error")); } if (line[strlen (line) - 1] == '\n') line[strlen (line) - 1] = '\0'; .... } 

Un peu surpris? Ça arrive. Nouvelle vulnérabilité, identifiant CVE correspondant: CVE-2016-6262 .

Avertissement PVS-Studio : V1010 CWE-20 Les données contaminées non contrôlées sont utilisées dans l'index: 'strlen (ligne)'.

Lors d'une autre tentative, le problème a été résolu en ajoutant une vérification de la longueur de la chaîne d'entrée:

 if (strlen (line) > 0) if (line[strlen (line) - 1] == '\n') line[strlen (line) - 1] = '\0'; 

Jetons un coup d'œil aux dates. L'engagement de «clôture» CVE-2015-8948 - 08/10/2015. Valider la clôture CVE-2016-62-62 - 14/01/2016. Autrement dit, la différence entre les corrections ci-dessus est de 5 mois ! Ici, vous vous souvenez d'un tel avantage de l'analyse statique que la détection d'erreurs dans les premières étapes de l'écriture de code ...

Analyse statique et sécurité

Il n'y aura pas d'autres exemples de code, mais plutôt des statistiques et un raisonnement. Dans cette section, l'opinion de l'auteur peut ne pas coïncider avec l'opinion du lecteur beaucoup plus qu'auparavant dans cet article.

Remarque Je vous recommande de lire un autre article sur un sujet similaire - " Comment PVS-Studio peut-il aider à trouver des vulnérabilités? ". Il existe des exemples intéressants de vulnérabilités qui ressemblent à de simples erreurs. De plus, dans cet article, j'ai parlé un peu de la terminologie et de la raison pour laquelle l'analyse statique est indispensable si vous êtes préoccupé par le sujet de la sécurité.

Regardons les statistiques sur le nombre de vulnérabilités découvertes au cours des 10 dernières années pour évaluer la situation. J'ai pris les données du site Web CVE Details .

Une situation intéressante se profile. Jusqu'en 2014, le nombre de CVE enregistrés n'a pas dépassé la barre des 6000 unités, et à partir de - il n'est pas descendu en dessous. Mais le plus intéressant ici, bien sûr, ce sont les statistiques pour 2017 - le leader absolu (14714 unités). Quant à l'année en cours - 2018 -, elle n'est pas encore terminée, mais bat déjà des records - 15 310 unités.

Est-ce à dire que tous les nouveaux logiciels sont remplis de trous comme un tamis? Je ne pense pas, et voici pourquoi:

• Intérêt accru pour le sujet des vulnérabilités. Certes, même si vous n'êtes pas très proche du sujet de la sécurité, vous avez rencontré à plusieurs reprises des articles, des notes, des rapports et des vidéos sur le sujet de la sécurité. En d'autres termes, une sorte de «battage médiatique» a été créé. C'est mauvais? Probablement pas. En fin de compte, tout se résume au fait que les développeurs sont plus préoccupés par la sécurité des applications, ce qui est bien.
• Une augmentation du nombre d'applications développées. Plus de code - plus susceptible de se produire toute vulnérabilité qui reconstituera les statistiques.
• Outils de recherche de vulnérabilité améliorés et assurance qualité du code. Plus de demande -> plus d'offre. Les analyseurs, fuzzers et autres outils sont de plus en plus avancés, ce qui joue entre les mains de ceux qui veulent rechercher des vulnérabilités (quel que soit le côté des barricades sur lequel ils se trouvent).

Ainsi, la tendance émergente ne peut pas être qualifiée de exclusivement négative - les éditeurs sont plus préoccupés par la sécurité de l'information, les outils pour détecter les problèmes sont améliorés, et tout cela est sans aucun doute positif.

Est-ce à dire que vous pouvez vous détendre et ne pas vous «baigner»? Je pense que non. Si vous êtes préoccupé par le sujet de la sécurité de vos applications, vous devez prendre autant de mesures de sécurité que possible. Cela est particulièrement vrai si le code source est dans le domaine public, car il:

• plus susceptibles d'intégrer des vulnérabilités externes;
• plus enclins à "sonder" par ces "messieurs" qui sont intéressés par des trous dans votre application dans le but de les exploiter. Bien que les sympathisants dans ce cas pourront vous aider davantage.

Je ne veux pas dire que vous n'avez pas besoin de traduire vos projets sous open source. Gardez simplement à l'esprit les contrôles de qualité / sécurité appropriés.

L'analyse statique est-elle une mesure supplémentaire? Oui L'analyse statique fait un bon travail pour détecter les vulnérabilités potentielles qui pourraient devenir réelles à l'avenir.

Il me semble (j'avoue que je me trompe) que beaucoup considèrent les vulnérabilités comme un phénomène de haut niveau. Oui et non. Les problèmes de code qui semblent être de simples erreurs de programmation peuvent également être de graves vulnérabilités. Encore une fois, quelques exemples de telles vulnérabilités sont fournis dans l' article mentionné précédemment . Ne sous-estimez pas les erreurs «simples».

Conclusion

N'oubliez pas que les données d'entrée peuvent être de longueur nulle, et cela doit également être pris en compte.

Conclusions pour savoir si tout le battage publicitaire avec des vulnérabilités est juste du battage médiatique, ou si le problème existe, faites-le vous-même.

Pour ma part, à moins que je ne propose d'essayer votre projet PVS-Studio , si vous ne l'avez pas déjà fait.

Meilleurs vœux!

Si vous souhaitez partager cet article avec un public anglophone, veuillez utiliser le lien vers la traduction: Sergey Vasiliev. Tirez-vous dans le pied lors de la manipulation des données d'entrée