9
votes

Message de chaîne contaminé de la couverture à l'aide de getenv

Coverité en cours d'exécution sur mon code Résultats dans le message d'erreur de chaîne tainted. J'utilise la variable "Chemin" déclaré dans la pile, alors je ne suis donc pas sûr de savoir pourquoi je vois des erreurs. Je ne peux que penser qu'à utiliser getenv () code> directement dans le strncpy () code> causant l'erreur. Serait le correctif ci-dessous éliminer cette erreur? XXX PRE>

à P>

char path[1024] = {NULL, };
char * adriver = getenv("A");
if(adriver)
    strncpy(path, adriver, strlen(adriver));


7 commentaires

Il n'est pas judicieux d'utiliser NULL comme initialisateur pour un Char , surtout si la mise en œuvre définit null comme ((vide *) 0) , qui est une valeur légitime en C (mais pas légitime en C ++). Vous pouvez utiliser Char Chemin [1024] = ""; à la place. Vous abusez également strncpy () ; Vous devez le limiter à la longueur du chemin (moins 1). Si une personne exécute votre code avec un chemin de plus de 1023 caractères, vous déborde de la matrice et peut finir par casser et ne passera pas une chaîne de terminaison des nuls. Il est peu probable que l'un de ceux-ci soit un facteur dans le message "String Tainted".


Après avoir examiné le code à nouveau, je me rends compte que vous faites un memset superflu () après l'initialisation chemin à tous les octets zéro.


Pour ajouter au commentaire de @jonathanleffler Sir, la deuxième approche est meilleure, car elle vous permet d'économiser deux appels redondants getenv () .


À une fois, JS1 observé - avec précision - que: dans le deuxième morceau de code, vous devez réaliser que < Code> if (! AdRiver) est seulement vrai si AdRiver est null . En d'autres termes, vous avez inversé la signification de la déclaration si de la première pièce de code. Vous vouliez probablement dire si (AdRiver) .


@Jonathanleffler merci, j'ai fait les changements. Mais je ne suis toujours pas sûr que cela élimine le problème de la chaîne contaminée.


@Jaychung: Je ne suis pas surpris que les changements que j'ai suggusés ne résolvent pas le problème des cordes contaminées. Je ne sais rien de spécifique sur la couverture et les chaînes contaminées. Par analogie avec Perl (un jeu dangereux à jouer), je suppose que vous devez faire une analyse de la valeur renvoyée par getenv () avant de faire grand chose avec elle afin de vous débarrasser de la contaminée. Mais je n'ai pas devenu manuelle bashing pour en savoir plus - vous pouvez le faire aussi bien que possible.


Vous pourriez trouver chaîne contaminée en C de certaines aide. Une recherche Google sur les termes "String taisted Coverity" présente un certain nombre d'autres éléments connexes. Je n'ai pas trouvé de réponse définitive, mais je n'ai pas non plus visité le site de couverture pour voir quels manuels sont disponibles.


4 Réponses :


8
votes

Non, cela ne corrigera probablement pas l'erreur.

Coverity vous indique que les données de la variable d'environnement "A" pourrait être à peu près n'importe quoi; Ces données ne sont pas sous le contrôle de votre programme.

Par conséquent, vous devez avoir des vérifications de la santé mentale sur les données avant de l'utiliser.

Votre correctif proposé aura actuellement un débordement de mémoire tampon, si quelqu'un définit une variable d'environnement A à une chaîne contenant 1025 caractères.

En outre, aucune version de code n'a jamais nul-terminer la chaîne "Chemin". En effet, vous utilisez Strncpy, qui ne sera pas nul-terminer si la limite d'octets est appliquée (ce qui sera dans ce cas, car vous dites "limiter la chaîne copiée à la longueur que je viens de recevoir la chaîne"). < / p>

Ce que vous devriez faire, c'est la taille de la chaîne en premier. Si c'est trop important, renvoyez une sorte de code d'erreur; Le chemin de la variable A est trop gros, votre code ne fonctionnera pas comme si vous le souhaitez. Si ce n'est pas trop grand, copiez-le dans le tampon de chemin. Si vous souhaitez utiliser STRNCPY, assurez-vous de laisser de la place à une nul à la fin, puis de l'ajouter explicitement, car Strncpy n'est pas garantie de mettre un nul là-bas.


5 commentaires

Vous êtes à 100% sur la raison pour laquelle la couverture s'est plainte. Mais, il n'y a aucune raison d'utiliser strncpy ici. En fait, ce n'est utile que pour un petit cas de coin. Vous avez déjà calculé la longueur pour savoir s'il va s'adapter. Il est probablement beaucoup plus efficace d'utiliser memcpy pour le copier, puis ajoutez manuellement le nul à la fin.


La raison principale strncpy () n'est probablement pas utile, c'est que si le contenu de la variable d'environnement est légitime mais trop long, puis tronquer la valeur est la mauvaise chose à faire. Une telle troncature pourrait être une question de sécurité en soi, mais il est probablement peu probable de produire le béon souhaité. Le programme doit terminer avec une erreur à la place ou au moins rejeter la valeur de la longueur totale.


La fin d'une chaîne est 0, pas null.


Tu as raison ... pour être pédants un zéro est un ascii "nul", pas "null". Corrigée. ;-)


Pour être ultra-pédant, une chaîne C est terminée avec un octet 0, ce qui n'est pas un caractère ASCII "NUL". Sur certains processeurs (nommément DSP), les octets sont de 32 bits. ASCII n'est pas.



-1
votes
char *path = NULL, *A = getenv("A");

if(A != NULL)
   {
   path = malloc(strlen(A)+1);
   strcpy(path, A);
   }

3 commentaires

Utilisez STRUP pour la copie d'une chaîne et une mémoire allouée en même temps.


Solution simple, mais avec une différence sémantique: chemin est null par opposition à "" si l'environnement n'a pas de valeur pour Un .


STRUP ... Les personnes âgées américaines n'ont pas eu de telles nanties ... ont dû marcher dans les deux sens pour aller à l'école 'n' tout ... vous y jeunes gens ont bien compris ... où est mon spitoon ??? :-P



0
votes

la solution ci-dessous pourrait-elle éliminer cette erreur?

Non, mais cela ferait presque certainement: xxx

Le problème principal avec votre code se produit lorsque getenv ("A") retourne une chaîne C'est 1024 octets ou plus; Copier cela dans Chemin Comme vous êtes des résultats dans un débordement tampon, car il n'est pas garanti que Strlen (getenv ("a")) sera inférieur ou égal à 1024. Vous pouvez résoudre ce problème en utilisant: xxx

... mais chemin ne sera pas terminé avec un '\ 0 ' caractère; chemin ne sera pas une chaîne dans cette situation, car par définition, une chaîne doit être terminée avec un '\ 0' caractère ... Cela signifie que vous ne seriez pas en mesure de l'utiliser comme une chaîne.

C'est ce que la couverture est probablement placée ici, et mon code élimine le problème en utilisant une VLA (réseau de longueur variable) qui est grand Assez pour stocker la chaîne entière, plus le '\ 0' à la fin de la chaîne.


0 commentaires

7
votes

Votre code est incorrect: vous avez un débordement potentiel de tampon dans les deux alternatives.

Je ne suis pas sûr de forte> Coverity fort> Diagnostime correctement le problème, vous n'avez pas publié le message d'erreur exact. La couverture est éventuellement indiquant que vous utilisez une chaîne de l'environnement, qui pourrait avoir une longueur, ce qui entraînerait potentiellement un débordement de tampon lorsqu'il est copié par votre code dans un tampon de 1024 octets, c'est une bonne chose qui vous a fait remarquer. Voici pourquoi: p>

Strncpy code> ne fait pas ce que vous pensez que ça fait. Cette fonction devrait jamais forte> être utilisée, sa sémantique sont sujettes à une erreur, ce n'est pas le bon outil à votre usage. Strncpy (DEST, SRC, N) CODE> Copies Pas plus que N CODE> caractères de SRC code> à DEST code> et remplit le reste de la matrice à DEST code> avec '\ 0' code> octets jusqu'à ce que n code> a été écrit. DEST code> doit pointer sur un tableau d'au moins N code> caractères. Si src code> est plus court, le comportement est inefficace car le rembourrage est généralement inutile, mais si src code> a une longueur d'au moins n code>, DEST code> ne sera pas null terminé par Strncpy code>, conduisant à un comportement non défini dans de nombreux cas. P>

Votre code: P>

char *path = getenv("A");
if (path == NULL)
    path = "";


1 commentaires

Jusqu'à présent, seules 2 choses ont été supprimées de la couverture avertissement pour moi: [1] in c use STRUP suivi d'un appel à une macro qui a fait un snapintf [2] C ++ <<< / code> à un stringstream puis retourner .str () . Je ne sais pas pourquoi STRUP par lui-même n'a pas fonctionné dans le fichier C ++.