Discussion :

[hbx360]

Bonjour,

Je souhaiterai savoir si ma méthode pour saisir un nom de fichier au clavier est sécurisé.

Le but est de : en fonction du nombre de lettres taper, il y est un realloc tout les 25 caractères bien sûr l'utilisateur n'a pas connaissance de cela.

Si par exemple quelqu'un tape le nom de fichier : "Photo de vacances en bord de mer" alors il y aura un realloc.

Est-ce que cette méthode est dangereuse. Si oui dois-je plutôt passé par un tableau et est-ce qu'un tableau est sécuriser si on met je dis un chiffre au hasard 500 cases.

Voici mon code :

Le .h

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
 
#ifndef __FILENAME_H__
#define __FILENAME_H__
 
#include <stdlib.h>
#include <stdio.h>
#include "ctypes.h"
 
#define FILENAME_ALLOC_CARA 25 // Nbre de caractère alloué pour récupéré le nom de fichier.
#define FILENAME_OutputLg 9+3 // +2 Le numéro du fichier et +1 pour cara fin de chaîne.
 
struct SFilename{
  char *pFilename; // Nom du fichier htm.
  s32 gnFilenameNbCaraMax; // Nb Max de caractère pour le nom de fichier.
  s32 gnFilenameNbCara; // Itérateur.
  FILE *pFile; // Pointeur sur le fichier.
};
 
struct SFilename gFilename;
 
// Init paramètre du nom du fichier.
void Filename_Init(void);
// Réallocation de la taille du buffer si nom du fichier trop grand.
void Filename_Realloc(void);
// Libération de la mémoire.
void Filename_Release(void);
// Récupération du nom du fichier avec stdin (saisie clavier), avant d'appuyer sur entré.
void Filename_Recovery(void);
 
#endif

Le .c

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
 
 
#include "filename.h"
 
// Init paramètre du nom du fichier.
void Filename_Init(void)
{
  gFilename.pFilename = NULL;
  gFilename.gnFilenameNbCaraMax = 0;
  gFilename.gnFilenameNbCara = 0;
}
 
void Filename_Realloc(void)
{
  if(gFilename.gnFilenameNbCara >= gFilename.gnFilenameNbCaraMax)
  {
    gFilename.gnFilenameNbCaraMax += FILENAME_ALLOC_CARA;
    gFilename.pFilename = (char *)realloc(gFilename.pFilename, gFilename.gnFilenameNbCaraMax*sizeof(char));
    if(gFilename.pFilename == NULL)
    {
      printf("Une erreur inatendu est survenu. Nous nous excusons pour la géne occasionnée. Merci de relancer le programme.\n");
      exit(1);
    }
  }
}
 
void Filename_Release(void)
{
  free(gFilename.pFilename);
  gFilename.pFilename = NULL;
}
 
void Filename_Recovery(void)
{ 
  s32 nCara = 0;
  do
  {
    Filename_Realloc();
 
    gFilename.pFilename[gFilename.gnFilenameNbCara++] = nCara = fgetc(stdin); // stdin récupère la saisie clavier avant l'appel à entré.
  }while(nCara != 10); // 10 (caractère LF).
 
  gFilename.pFilename[--gFilename.gnFilenameNbCara] = '\0'; 
}

Merci par avance.

[Sve@r]

Bonjour

Assez intéressant comme approche. Car tu as en fait réécrit la fonction getline().

La vraie question est "qu'est-ce que tu appelles dangereux" ? Dangereux vis à vis de ton programme ? De l'OS ? D'un autre danger (comme par exemple de l'utilisateur X qui chercherait à utiliser cette saisie pour faire une injection) ?

Du point de vue C pur le seul danger est d'exploser le tas (tu taperais un nom tellement long qu'il dépasserait en taille la capacité physique de ta mémoire). Ok ça semble ridicule au premier abord mais imagine que quelqu'un tape la commande cat /dev/zero |ton_programme (le fichier "/dev/zero" sous Unix a pour rôle de fournir des '\0' en flot ininterrompu permettant ainsi de tester par exemple la charge d'un outil quelconque mangeur d'infos ; et le pipe remplace le stdin du programme situé à sa droite par le stdout du programme qui se trouve à sa gauche). Bref ainsi ton programme accumulera et stockera des '\0' à l'infini => crash certain à plus ou moins long terme.
Du point de vue de l'OS, tu pourrais avoir un nom plus long que ce qu'il est capable de gérer (macro MAXPATHLEN je crois).
En ce qui concerne les autres dangers là personne ne peut prévoir. Ca dépend surtout de ce que tu fais de la chaine saisie.

Pour ta seconde question (le tableau) tu peux effectivement simplifier grandement ton code en écrivant ta saisie de cette façon

Code c :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
char nomFic[MAXPATHLEN + 1];
fputs("Entrez votre nom de fichier :", stdout);
fgets(nomFic, MAXPATHLEN + 1, stdin);

En dehors du '\n' validant la saisie, la fonction fgets() bloque quand "n - 1" caractère ont été lus (elle en garde un pour le '\0'). Donc là au-moins t'es certain de ne jamais dépasser la taille définie. Le seul danger c'est que si plus de n caractères ont été saisis, le surplus reste dans stdin (à toi de gérer ce cas).

Sinon ton code semble à priori correct (j'ai fait une lecture rapide). Je n'y vois qe trois défauts possibles

1. si realloc échoue il faut quand-même libérer la zone qui reste allouée
2. on ne sort jamais d'une sous-fonction par exit() (on remonte à l'appelant qui prend une décision et etc jusqu'au main).
3. je n'ai pas trouvé ton allocation initiale. Je présume donc que tu utilises aussi realloc() pour ça. T'en as le droit à condition que le pointeur passé à la fonction contienne explicitement la valeur NULL

Et sinon tu pourrais envoyer tes messages d'erreur sur stderr pour être plus conforme aux usages Unix/Linux.

[hbx360]

Merci beaucoup pour ta réponse et tes explications

Assez intéressant comme approche. Car tu as en fait réécrit la fonction getline().

Ha bin si j'avais su

La vraie question est "qu'est-ce que tu appelles dangereux" ? (comme par exemple de l'utilisateur X qui chercherait à utiliser cette saisie pour faire une injection) ?

Oui Je pensais à cela. L'exemple souvent cité de scanf qui n'est pas sécurisé ; quelqu'un de malveillant qui voudrait insérer du code en mémoire et faire en sorte qu'il soit exécuté par le programme en utilisant la saisie.
Mais apparement si j'ai bien compris le fait de faire de l'allocation dynamique avec realloc ne permettra pas à un hacker de réécrire du code malveillant dans la mémoire qui à été réallouer.

1. si realloc échoue il faut quand-même libérer la zone qui reste allouée

Ok je pensais que la libération de la mémoire ne se faisait que pour malloc.

2. on ne sort jamais d'une sous-fonction par exit() (on remonte à l'appelant qui prend une décision et etc jusqu'au main).

J'ai fait comme ça parce que j'étudie le code source du jeu Arkanoïd de Clement Corde qui à fait une interview sur developpez.net et qui avait travaillé 6 ans chez Titus donc c'est un peu ma référence mais je prend note.

3. je n'ai pas trouvé ton allocation initiale. Je présume donc que tu utilises aussi realloc() pour ça. T'en as le droit à condition que le pointeur passé à la fonction contienne explicitement la valeur NULL

Oui idem j'ai pris exemple sur le code de Clément je fais pas de malloc. Le pointeur passé à la fonction est bien à NULL car j'ai une fonction d'init dit moi éventuellement si c'est ok :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
void Filename_Init(void)
{
  gFilename.pFilename = NULL; // Je pense que tu parles bien de ce pointeur là. 
// C'est celui là que je passe à la fonction realloc.
  gFilename.gnFilenameNbCaraMax = 0;
  gFilename.gnFilenameNbCara = 0;
}

Et sinon tu pourrais envoyer tes messages d'erreur sur stderr pour être plus conforme aux usages Unix/Linux.

Ok merci pour le conseil mais Clément ne les mets pas systématiquement. Exemple d'un de ses codes :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
 
gpSprBuf = (u8 *)realloc(gpSprBuf, gnSprBufSz + nSprBufAddSz);
if (gpSprBuf == NULL)
{
printf("Spr: realloc failed.\n");
SprRelease();
exit(1);
}
 
void SprRelease(void)
{
	free(gpSprBuf);		// On libère les datas.
	free(gpSprDef);		// On libère les définitions.
 
}

C'est difficile quand on est débutant de savoir quoi faire car on à souvent des exemples ou des conseille contradictoire.
Après on est un peu perdu.

Merci encore pour ton aide.

[Sve@r]

Oui Je pensais à cela. L'exemple souvent cité de scanf qui n'est pas sécurisé ; quelqu'un de malveillant qui voudrait insérer du code en mémoire et faire en sorte qu'il soit exécuté par le programme en utilisant la saisie.

Oui car on est obligé de donner à scanf() un tableau où stocker l'info saisie mais qu'on ne peut pas lui limiter cette saisie. Ce qui entraine d'ailleurs souvent sur ce forum le conseil de ne pas utiliser scanf(). D'ailleurs cette fonction attend une saisie "formatée" (le "f" de "scanf") et ce que tape un humain est tout sauf formaté.

Mais apparement si j'ai bien compris le fait de faire de l'allocation dynamique avec realloc ne permettra pas à un hacker de réécrire du code malveillant dans la mémoire qui à été réallouer.

Non. Le principe du code malveillant est de rentrer une chaine qui dépasse la taille de la zone à remplir. Le surplus va alors dans la mémoire qui suit et si on se débrouille bien on peut alors faire exécuter ce surplus comme un code. Mais toi pas de souci vu que tu agrandis la zone à remplir au fur et à mesure donc tout ce bel enchainement s'effondre. Zéro souci de ce côté.

Ok je pensais que la libération de la mémoire ne se faisait que pour malloc.

Exact. Mais si realloc échoue, c'est comme s'il n'avait jamais existé ce qui te fait alors retomber dans le cas malloc. Donc généralement on a un second pointeur qui récupère le realloc et s'il est ok on le recopie dans le pointeur d'origine. Sinon le ptr d'origine reste intact pour le free().

J'ai fait comme ça parce que j'étudie le code source du jeu Arkanoïd de Clement Corde qui à fait une interview sur developpez.net et qui avait travaillé 6 ans chez Titus

Ah oui, Titus. Très grande référence chez les marchands d'esclaves. On ne bat pas un esclave de chez Titus (Astérix: les lauriers de César)

j'ai une fonction d'init dit moi éventuellement si c'est ok :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
void Filename_Init(void)
{
  gFilename.pFilename = NULL; // Je pense que tu parles bien de ce pointeur là. 
// C'est celui là que je passe à la fonction realloc.
  gFilename.gnFilenameNbCaraMax = 0;
  gFilename.gnFilenameNbCara = 0;
}

L'intialisation est correcte. Mais travaillerais-tu avec des globales ? Ca c'est pas bien....

Ok merci pour le conseil mais Clément ne les mets pas systématiquement. Exemple d'un de ses codes :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
gpSprBuf = (u8 *)realloc(gpSprBuf, gnSprBufSz + nSprBufAddSz);
if (gpSprBuf == NULL)
{
printf("Spr: realloc failed.\n");
SprRelease();
exit(1);
}
 
void SprRelease(void)
{
	free(gpSprBuf);		// On libère les datas.
	free(gpSprDef);		// On libère les définitions.
 
}

Ah oui, effectivement si Clément fait ainsi nous, pauvres humains, ne pouvons que nous incliner

Avoir une grande réputation n'est pas forcément synonyme de programmation correcte. Par exemple j'ai un jeu récent (2015) quand je le lance ça me met une exception "pure virtual function call" preuve que le codeur n'a pas été tiptop a appeler une fonction virtuelle pure (c'est interdit) mais le jeu a quand-même été diffusé ainsi. Alors si je ferme pas la fenêtre le jeu se lance quand-même mais si je la ferme il part en "ce programme a cessé de fonctionner".
Tu peux coder à la "va vite" en mettant par exemple exit() en cas de souci. Parce que tu sais ce qu'il se passe, que tu connais ton environnement (tu sais par exemple que ton OS rattrappera le coup). Mais celui qui ne sait pas et qui tente d'agir de la même façon va droit au mur (parce que par exemple il ne travaille pas dans le même environnement). Tu ne peux pas prendre comme modèle des codes qui vont plus vite, plus loin et parfois oublient certaines limites. Les bons programmeurs respectent les règles, les programmeurs brillants les transgressent parfois. Malheureusement tu n'es pas encore assez... brillant

C'est difficile quand on est débutant de savoir quoi faire car on à souvent des exemples ou des conseille contradictoire.
Après on est un peu perdu.

Oui je vois. Ca rejoint ma phrase précédente.

[hbx360]

Ok merci beaucoup pour tes conseilles.

L'intialisation est correcte. Mais travaillerais-tu avec des globales ? Ca c'est pas bien....

Oui car Clément ...

[hbx360]

@Sve@r

Dans ce cours https://c.developpez.com/cours/berna...gne/node71.php pour fopen si l'ouverture à échouer
il ne fait pas de return juste un exit(1).

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
 
#include <stdio.h>
FILE *fp;
 
if ((fp = fopen("donnees","r")) == NULL)
   {
   fprintf(stderr,"Impossible d'ouvrir le fichier données en lecture\n");
   exit(1);
   }

Donc que faut-il penser ? Surtout que c'est un cours validé par developpez.com donc c'est sérieux.

[Bousk]

- Le cours date de 1998
- Il n'y a aucun contexte sur l'utilisation de ces 3 lignes de code
- exit est un raccourci facile pour indiquer qu'il faut arrêter le traitement
> L'arrêt effectif est laissé à la discrétion du programmeur et dépend du programme, des systèmes en place, de la façon dont sont gérées les erreurs, ...

[Sve@r]

> L'arrêt effectif est laissé à la discrétion du programmeur et dépend du programme, des systèmes en place, de la façon dont sont gérées les erreurs, ...

... et reste soumis à la validation de Clément

[hbx360]

... et reste soumis à la validation de Clément

Oui MDR

Merci pour vos réponses.