cours/content/secu_logicielle/td7-analyse_statique_dynamique/index.md at 60aecd0591309fa86373c7fe4f1c0dc88e8dbedf

ephase/cours

Fork 0

Yorick Barbanneau 553cdc440c Add TD7

2023-05-09 21:57:14 +02:00

16 KiB

Raw Blame History

title

date

Partie 1

getpw

Le message d'avertissement de gcc est:

 `getpw' function is dangerous and should not be used.

D'après le manuel de getpw, il n'y a pas de paramètre de taille, cette fonction est donc sensible aux attaques par dépassement de tampon (buffer overflow).

Nous avons diminué la taille de buf à 12 et observons une erreur de segmentation lors de la fin du programme. Avec analyse de la pile par pframe nous pouvons observer que le retour de getpw() écrase des élément au dessus de lui-même sur l'adresse de retour.

Il pourrait être possible d'intégrer un shellcode dans le champ gecos de notre utilisateur afin d'exploiter ce dépassement de tampon.

mktemp

Cette fonction ne se charge pas de créer le fichier, elle défini juste un nom. Il est alors possible pour un attaquant de créer le fichier avant celui du programme en ratissant le plus large possible. Nous avions vu ce cas de figure lors du premier TD sur les défis Nebula.

Partie 2

Question 1

Effectivement cppcheck retourne une erreur dans le code source entrainant une fuite de mémoire :

cppcheck test-leak.c
Checking test-leak.c ...
test-leak.c:9:2: error: Memory leak: d [memleak]
 return c;
 ^

Dasn le code source, les espaces mémoire doivent être désalloués :

dans la fonction f() pour d
dans la fonction g() pour c (référencé sous e dans cette dernière)

En terme de sécurité, une fuite mémoire permet à une attaque par déni de service.

Question 2

Ni cppcheck ni la version asan de l'exécutable ne remontent d'erreur. Seul valgrind nous averti d'un espace mémoire accessible à la terminaison du programme.

Valgrind nous donne des information sur la fuite de mémoire:

valgrind --leak-check=full --show-leak-kinds=all ./test-reachable-leak
==14383== Memcheck, a memory error detector
==14383== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==14383== Using Valgrind-3.19.0 and LibVEX; rerun with -h for copyright info
==14383== Command: ./test-reachable-leak
==14383== 
==14383== 
==14383== HEAP SUMMARY:
==14383==     in use at exit: 10 bytes in 1 blocks
==14383==   total heap usage: 1 allocs, 0 frees, 10 bytes allocated
==14383== 
==14383== 10 bytes in 1 blocks are still reachable in loss record 1 of 1
==14383==    at 0x48407B4: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==14383==    by 0x109146: main (test-reachable-leak.c:6)
==14383== 
==14383== LEAK SUMMARY:
==14383==    definitely lost: 0 bytes in 0 blocks
==14383==    indirectly lost: 0 bytes in 0 blocks
==14383==      possibly lost: 0 bytes in 0 blocks
==14383==    still reachable: 10 bytes in 1 blocks
==14383==         suppressed: 0 bytes in 0 blocks
==14383== 
==14383== For lists of detected and suppressed errors, rerun with: -s
==14383== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

Partie 3

Effectivement, cppcheck nous averti à propos d'une zone mémoire créée mais non initialisées. La version asan ne fait aucun rapport mais Valgrind est lui un peu plus locace :

[...]
Conditional jump or move depends on uninitialised value(s)
   at 0x10917A: f (test-uninitialized.c:5)
   by 0x1091D0: main (test-uninitialized.c:13)
[...]
Use --track-origins=yes to see where uninitialised values come from
[...]

Lors du check de test-uninitialized-printf.c Valgrind nous averti sur l'usage de valeur non-initialisée dont dépendent des jump ou move conditionnels. Cette erreur provient de la variable c créée par un malloc mais non initialisée.

Avec l'option --track-origins=yes suggérée par sa première exécution, Valgrind nous informe de l'origine de l'allocation:

[...]
==14921== Conditional jump or move depends on uninitialised value(s)
==14921==    at 0x48BBEC7: __vfprintf_internal (vfprintf-process-arg.c:58)
==14921==    by 0x48B14FA: printf (printf.c:33)
==14921==    by 0x10917F: f (test-uninitialized-printf.c:5)
==14921==    by 0x1091A9: main (test-uninitialized-printf.c:10)
==14921==  Uninitialised value was created by a heap allocation
==14921==    at 0x48407B4: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==14921==    by 0x109194: main (test-uninitialized-printf.c:9)
[...]

Partie 4

Lors du lancement du programme test-undefined, nous pouvons constater que la valeur afficher de notre variable i n'est pas celle attendue:

./test-undefined
-2147483648

Comme nous l'avons vu en cours, nous sommes en présence d'un cas non définis par la norme C.

Valgrind lui de fait part d'aucun problème, il ne se soucis pas du type de la variable i mais de même espace qu'elle occupe en mémoire. Et il n'a pas changé.

valgrind ./test-undefined
==3381== Memcheck, a memory error detector
==3381== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==3381== Using Valgrind-3.19.0 and LibVEX; rerun with -h for copyright info
==3381== Command: ./test-undefined
==3381== 
-2147483648
==3381== 
==3381== HEAP SUMMARY:
==3381==     in use at exit: 0 bytes in 0 blocks
==3381==   total heap usage: 1 allocs, 1 frees, 1,024 bytes allocated
==3381== 
==3381== All heap blocks were freed -- no leaks are possible
==3381== 
==3381== For lists of detected and suppressed errors, rerun with: -s
==3381== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

La version usan indique effectivement un overflow de notre variable i :

./test-undefined.usan 
test-undefined.c:7:3: runtime error: signed integer overflow: 2147483647 + 1 cannot be represented in type 'int'
-2147483648

L'option de compilation -fsanitize=undefined utilisée pour cette version de l'exécutable ajoute des éléments pour vérifier les comportements indéfinis.

Partie 5

Question 1

cppcheck nous indique bien un overrun :

cppcheck test-overrun.c     
Checking test-overrun.c ...
Array 'c[10]' accessed at index 10, which is out of bonds.[arrayIndexOutOfBounds]
 c[10] = 1;
  ^

Le fichier en .asan nous indique un : heap-buffer-overflow avec plus de détails (registres pc bp et sp, adresses, etc) :

/test-overrun.asan 
=================================================================
==6036==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60200000001a 
  at pc 0x55f0c2aa51cb bp 0x7ffc2aa141d0 sp 0x7ffc2aa141c8
WRITE of size 1 at 0x60200000001a thread T0
[...]
Shadow bytes around the buggy address:
  0x0c047fff7fb0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7fc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7fd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7fe0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c047fff7ff0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c047fff8000: fa fa 00[02]fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8010: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff8020: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
[...]

L'élément du tableau c[10] est incorrect car malloc(10) correspond à un tableau de 10 éléments avec des indices de 0 à 9 donc c[10] est out-of-bond.

LA version usan ne reporte aucune erreur, tout comme Valgrind.

Question 2

Pour cette version, Valgrind ne reporte aucune erreur. Il ne se rend pas compte que l'accès en dehors de la variable dans le segment data. La version usan de notre exécutable elle affiche des erreurs:

./test-overrun-data.usan 
test-overrun-data.c:7:3: runtime error: index 10 out of bounds for type 'char [10]'
test-overrun-data.c:7:8: runtime error: store to address 0x55e89a7ca102 
  with insufficient space for an object of type 'char'
0x55e89a7ca102: note: pointer points here
 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00
              ^ 
test-overrun-data.c:8:3: runtime error: index -1 out of bounds for type 'char [10]'

cppcheck indique lui aussi deux erreurs pour c[10] et c[-1] :

cppcheck test-overrun-data.c 
Checking test-overrun-data.c ...
test-overrun-data.c:7:3: error: Array 'c[10]' accessed at index 10, which is out of bounds. [arrayIndexOutOfBounds]
 c[10] = 1;
  ^
test-overrun-data.c:8:3: error: Array 'c[10]' accessed at index -1, which is out of bounds. [negativeIndex]
 c[-1] = 1;
  ^

La version asan ne reporte non plus aucune erreur avec la version de GCC installée au CREMI. Cependant sur nos machines -- avec Debian Unstable comme système -- la version asan reporte une erreur.

Dans la version installée au CREMI, l'address sanitizer inclus dans GCC, l'option -fno-common n'est pas activée par défaut. Sans cette option il n'est pas possible pour lui de détecter le problème : il ne sais définir les limites des données globales non initialisés (stockées dans le segment BSS)

Valgrind ne se plaint pas et pour cause: nous se sommes toujours pas en présence d'un problème de fuite de mémoire.

Question 3

Dans la version asan, l'option de compilation -fsanitize=address active par défaut l'option asan-stack chargée de vérifier les erreurs de type out-of-bounds et use-after-free.

Lorsque nous exécutons test-overrun-stack avec GDB, après l'instruction c[-1] = 1 nous obtenons la pile suivante:

[...]
0x7fffffffe430            0x00007fffffffe520
0x7fffffffe428    ret@    0x00007ffff7dfb18a
0x7fffffffe420      bp sp 0x0000000000000001 <- overflow de c[10]
0x7fffffffe418            0x00007ffff7ffdad0
0x7fffffffe410            0x0000010000000000 <- overlow de c[-1]
[...]

Avec l'aide le la commande print, nous pouvons alors vous les adresses de c

(gdb) p &c[0]
$4 = 0x7fffffffe416 ""
(gdb) p &c[9]
$5 = 0x7fffffffe41f ""

Ici c[10] passe dans le segment de pile au dessus, déclenchant l'erreur dans notre exécutable asan.

Partie 8

Question 1

Le bug provient d'un problème de priorité : l'égalité est prioritaire sur tout le reste dans la condition:

// extract bit 0 to check for parity of the number of arguments
if (argc & 1 == 0) {
    // ...

Elle sera toujours fausse. La correction est simple:

// extract bit 0 to check for parity of the number of arguments
if ((argc & 1) == 0) {
    // ...

Question 2

C'est à la ligne 718 qu'il faut chercher l'erreur:

 while ((ret = xfs_bulkstat(fsfd,
                                &lastino, GRABSZ, &buf[0], &buflenout) == 0)) {

La parenthèse fermante définissant la priorité de l'affectation est mal placée, la comparaison se transforme donc en affectation. Utiliser cette notation permet d'affecter ret pendant la condition de la boucle, mais en contrepartie augmente le risque d'erreur.

Question 3

L'instruction assert est mal utilisée, le simple signe égal indique une affectation et non une comparaison

assert(n = int(*(p+1)), "just checkin'...");

La vérification sera toujours vrai, quoi qu'il arrive. Aucune erreur sera remontée et le programme continuera son exécution alors qu'il ne devrait pas.

assert() n'est utilisé que pour du debug dans le cas où la macro NDEBUG n'est pas définie au moment de la compilation. La portée de cette erreur était donc limitée.

Question 4

L'outil d'analyse statique a détecté l'erreur car la méthode screen->empty() retourne un booléen, or ici aucune valeur de retour est utilisée.

Question 5

for (i = 0; g_objArray[i] && i < MAX_WCOMP; i++) {
    MultiMoveRelXY(g_objArray[i], x - center, deltay);

L'erreur d'accès est causée par g_objArray[i], en effet le programme teste la validité de la case mémoire avant la validité de i. Le programme accède à la case mémoire avant de vérifier les limites.

Il est simple de fixer le problème: inverser les conditions comme ci-dessous:

for (i = 0; i < MAX_WCOMP && g_objArray[i]; i++) {
    MultiMoveRelXY(g_objArray[i], x - center, deltay);

Partie 9

Question 1

La portion de code est appelée dead code car quel que soit les conditions, cette portion ne sera jamais atteinte. Ici, decode est définit à false et jamais modifiée (l. 160), ainsi de la condition l. 174 sera toujours fausse et donc les instructions dans la if jamais atteintes.

Question 2

La valeur de skip est modifiée à la ligne 751, puis est suivie d'une instruction continue qui relance un tour de boucle. La valeur de skip est alors réinitialisée à 1 au début de cette boucle. La modification effectuée alors est totalement inutile.

Question 3

Le bloc d'instruction du case ligne 1240 n'est terminé ni par un break -- permettant de sortir du switch -- ni d'un return. Ainsi l'exécution du programme continuer dans le case suivant (ou le default). Des portion de code peuvent alors être exécutées dans des cas non prévus.

Question 4

Ici le test de la boucle uint x >= 0 est toujours vrai, on entre alors dans une boucle infinie.

Question 5

Partie 10

Question 1

Question 2

pos pointe vers une variable locale a locale fonction Texture::render(). Lors de la fin de cette dernière, alors pos pointera vers une zone mémoire qui n'est plus disponible.

Question 3

Dans la fonction setParent( ExecutionNode parent), si parent est NULL alors une erreur de type Null pointer dereferences se produit lors de l'instruction getContext().setParent(parent.getContext());. Le développeur teste cependant la validité du pointeur juste avant.

Question 4

L'analyseur a détermine que pos ne pouvait pas être égal à 0 pour deux raisons:

la signature de la fonction strchr renvoie un pointeur, p contient donc une adresse
dans le cas ou aucun caractère correspondant n'est trouvé, le programme ne rentre pas dans la boucle c'est la condition ((pos = strchr(lastpos, '&')) != NULL) qui le défini.

Si pos est défini, alors sont adresse est incrémentée de 1 (l. 1716), pos n'est donc jamais égal à 0, cependant il est possible que la case mémoire pointée par p le soit, indiquant une fin de chaine de caractères.

Question 5

Là encore on est face à un problème entre le type pointeur (qui contient une adresse, sont un int) et la zone mémoire vers la quelle il pointe.

L'instruction suivante alloue une zone mémoire égale à un int:

Device_Entry *entry = calloc(1, sizeof(entry));

Alors que l'instruction suivante alloue nue zone mémoire de la taille de Device_Entry:

Device_Entry *entry = calloc(1, sizeof(*entry));

Question 6

Lors de la première exécution de la boucle for (cur_module = wmodules; cur_module; wmodules = next_module), le programme libère la zone mémoire utilisée par la variable cur_module (l. 60). Cependant cette variable est utilisée ensuite dans le test de la boucle for. Tous les modules ne seront donc pas libérés (s'il y en a plus d'un).

for (cur_module = wmodules; cur_module; wmodules = next_module) {
    next_module = cur_module->next;
    cur_module->context->destroy(cur_module->data);
    free(cur_module);
}

Dans la correction proposée, cur_module est aussi utilisée dans la définition de la boucle for, ainsi là aussi notre boucle ne fera pas plus d'un tour.

Partie 11

Question 1

La dernière fonction appelée par ne donne pas lieu à un test comme les précédente, ainsi si tous les autres test réussissent mais pas ce dernier, alors le résultat sera vrai.

Le développeur aura du garder le fonctionnement des élément précédent et ainsi écrire:

if (!checkPriv(fromDBbackend, QLatin1String("CheckPriv_Cleanup")) {
    result = false;
}

Question 2

TAINTED_STRING signifie qu'un ou plusieurs éléments externes influnce(nt) le comportement de notre programme alors que ces éléments ne sont pas examinés.

Ici le code permet de lancer des commandes SQL contenues dans le fichier pointe par optFile sans aucun contrôle. Un attaquant pourrait alors réaliser une attaque par injection SQL.

16 KiB Raw Blame History

Partie 1

getpw

mktemp

Partie 2

Question 1

Question 2

Partie 3

Partie 4

Partie 5

Question 1

Question 2

Question 3

Partie 8

Question 1

Question 2

Question 3

Question 4

Question 5

Partie 9

Question 1

Question 2

Question 3

Question 4

Question 5

Partie 10

Question 1

Question 2

Question 3

Question 4

Question 5

Question 6

Partie 11

Question 1

Question 2

16 KiB

Raw Blame History