Get best of the week

Commencez à collecter les erreurs dans les fonctions de copie

memcpy

J'ai remarqué à plusieurs reprises que les programmeurs font des erreurs dans les fonctions de copie de données simples. Ce sujet nécessitera beaucoup plus de temps à l'avenir pour étudier et sélectionner du matériel afin d'écrire un article complet. Mais je voulais partager quelques exemples que j'ai récemment remarqués.

Le phénomène Baadera-Meinhof? Non je ne crois pas


En tant que membre de l'équipe PVS-Studio, je rencontre un grand nombre d'erreurs que nous découvrons dans divers projets. Comme DevRel - j'aime en parler :). Aujourd'hui, j'ai décidé de parler de fonctions de copie de données incorrectement implémentées.

J'ai rencontré de telles fonctions infructueuses plus d'une fois. Mais je ne les ai pas écrites, car je n'y attache aucune importance. Cependant, depuis que j'ai remarqué cette tendance, il est temps de commencer à les collecter. Pour commencer, je partagerai les deux derniers cas constatés.

Quelqu'un peut soutenir que deux cas - ce n'est pas une régularité. Et que, peut-être, j'ai attiré l'attention sur eux uniquement parce qu'ils m'ont rencontré après un court laps de temps et ont déclenché le phénomène Baader-Meinhof .

Le phénomène Baader-Meinhof, également une illusion de fréquence, est une distorsion cognitive dans laquelle des informations récemment découvertes qui réapparaissent après une courte période de temps sont perçues comme inhabituellement souvent répétées.

Je pense que ce n'est pas le cas. J'avais déjà l'expérience d'une telle observation sur les fonctions de comparaison, qui a ensuite été confirmée par le matériel recueilli: "Le mal vit dans les fonctions de comparaison ".

D'accord, passons au point. Jusqu'à présent, l'introduction pour ne donner que deux exemples a été trop longue :).

Exemple N1


Dans l' article sur la vérification Zephyr RTOS, j'ai décrit une telle tentative infructueuse d'implémenter la fonction strdup analogique :

static char *mntpt_prepare(char *mntpt)
{
  char *cpy_mntpt;

  cpy_mntpt = k_malloc(strlen(mntpt) + 1);
  if (cpy_mntpt) {
    ((u8_t *)mntpt)[strlen(mntpt)] = '\0';
    memcpy(cpy_mntpt, mntpt, strlen(mntpt));
  }
  return cpy_mntpt;
}

Avertissement PVS-Studio: V575 [CWE-628] La fonction 'memcpy' ne copie pas la chaîne entière. Utilisez la fonction 'strcpy / strcpy_s' pour conserver la valeur null du terminal. shell.c 427

L'analyseur signale que la fonction memcpy copie la ligne, mais ne copie pas le terminal zéro, ce qui est très suspect. Il semble que ce terminal 0 soit copié ici:

((u8_t *)mntpt)[strlen(mntpt)] = '\0';

Non, il y a une faute de frappe ici, à cause de laquelle le terminal zéro est copié sur lui-même. Notez que l'écriture dans le tableau mntpt , pas cpy_mntpt . Par conséquent, la fonction mntpt_prepare renvoie une chaîne incomplète avec un terminal zéro.

En fait, le programmeur voulait écrire comme ceci:

((u8_t *)cpy_mntpt)[strlen(mntpt)] = '\0';

Il n'est pas clair pourquoi le code est écrit si confus et non standard. En conséquence, une grave erreur a été commise dans une fonction petite et simple. Ce code peut être simplifié avec l'option suivante:

static char *mntpt_prepare(char *mntpt)
{
  char *cpy_mntpt;

  cpy_mntpt = k_malloc(strlen(mntpt) + 1);
  if (cpy_mntpt) {
    strcpy(cpy_mntpt, mntpt);
  }
  return cpy_mntpt;
}

Exemple N2


void myMemCpy(void *dest, void *src, size_t n) 
{ 
   char *csrc = (char *)src; 
   char *cdest = (char *)dest; 
   for (int i=0; i<n; i++) 
     cdest[i] = csrc[i]; 
}

Nous n'avons pas identifié ce code nous-mêmes à l'aide de PVS-Studio, mais je l'ai rencontré accidentellement sur le site Web de StackOverflow: C et analyse de code statique: est-ce plus sûr que memcpy?

Cependant, si vous vérifiez cette fonction à l'aide de l'analyseur PVS-Studio, il remarquera à juste titre:

  • V104 Conversion implicite de «i» en type memsize dans une expression arithmétique: i <n test.cpp 26
  • V108 Type d'index incorrect: cdest [pas de type memsize]. Utilisez plutôt le type memsize. test.cpp 27
  • V108 Type d'index incorrect: csrc [pas de type memsize]. Utilisez plutôt le type memsize. test.cpp 27

En effet, ce code contient une faille, comme indiqué dans les réponses à StackOverflow. Vous ne pouvez pas utiliser une variable int comme index . Dans un programme 64 bits, presque certainement (nous ne considérons pas les architectures exotiques), la variable int sera 32 bits et la fonction ne pourra pas copier plus de INT_MAX octets. Ceux. pas plus de 2 gigaoctets.

Avec un tampon plus grand à copier, un débordement de la variable de signe se produira, ce qui du point de vue de C et C ++ est un comportement non défini. Et au fait, n'essayez pas de deviner exactement comment l'erreur se manifestera. Il s'agit en fait d'un sujet difficile, que vous pouvez lire dans l'article "Un comportement indéfini est plus proche que vous ne le pensez ".

Il est particulièrement amusant que ce code apparaisse comme une tentative de supprimer une sorte d'avertissement de l'analyseur Checkmarx qui s'est produit lors de l' appel de la fonction memcpy . Le programmeur n'a rien trouvé de mieux que de fabriquer son propre vélo. Et malgré la simplicité de la fonction de copie, cela s'est avéré être faux. En fait, la personne a probablement fait encore pire qu'elle ne l'était. Au lieu de comprendre la raison de l'avertissement, il a masqué le problème en écrivant sa propre fonction (confondant l'analyseur). Plus a ajouté une erreur en utilisant int pour le compteur . Oh oui, un tel code peut toujours entraver l'optimisation. Il est inefficace d'utiliser votre propre code au lieu de la fonction memcpy optimisée et efficace . Ne faites pas cela :)

Conclusion


Eh bien, je ne suis qu'au début du voyage et, probablement, il faudra plus d'un an avant d'accumuler du matériel pour une publication approfondie sur ce sujet. En fait, ce n'est que maintenant que je vais commencer à écrire de tels cas. Merci de votre attention et regardez ce que l'analyseur PVS-Studio trouvera intéressant dans votre code C / C ++ / C # / Java.



Si vous souhaitez partager cet article avec un public anglophone, veuillez utiliser le lien vers la traduction: Andrey Karpov. Démarrage de ma collection de bogues trouvés dans les fonctions de copie .

More articles:


All Articles