Discussion :

[Invité1]

Je commence en c et j'ai découvert la joie des buffer overflows

J'ai fait un petit exercice et je me demandais si le code était sécuritaire. Le but de mon petit exercice était justement qu'il soit sécuritaire (ne pas utiliser gets() ou de choses du genre) . . .

Voilà la partie du code en question:

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
 
       char *input;
	FILE *filename;
 
	input = (char *)malloc(32);
 
	fgets(input,32,stdin);
	filename = fopen("yourname.txt","w");
	if(!filename){
		puts("FILE ERROR");
		return 0;
	}
	fprintf(filename,input);
	fclose(filename);
	free(input);

P.S. Pour Emmanuel Delahaye (je suis presque certain que vous allez lire ce post), je crois que j'ai enfin compris pourquoi il vaut mieux utiliser fgets() et aussi comment l'utiliser

Merci beaucoup!
Alex

[stephl]

Quelques remarques:

• Même s'il est peu vraisemblable que l'allocation de 32 bytes échoue ici, il est préférable de tester que l'allocation de mémoire a fonctionné avant d'essayer de stocker quelque chose à cet endroit.
• La mémoire n'est pas libérée si l'ouverture de fichier échoue.
• Le fprintf() pose problème si input contient des caractères %, car fprintf() s'attendra alors à trouver des paramètres supplémentaires. La façon correcte de faire est:
  

  Code :

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

  fprintf(filename, "%s", input);

• filename n'est pas un nom très bien choisi. Le filename, selon moi, c'est le premier paramètre passé à fopen(). J'aurais plutôt appelé la variable file, ou f, ou outfile, etc...

[nicolas.sitbon]

La façon correcte de faire est:

Code :

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

fprintf(filename, "%s", input);

Code :

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

fputs (input, filename);

[oniric]

Même s'il est peu vraisemblable que l'allocation de 32 bytes échoue ici, il est préférable de tester que l'allocation de mémoire a fonctionné avant d'essayer de stocker quelque chose à cet endroit.

comme le dit steph si tu veux jouer la securite jusqu'au bout, tu peux te coder une fonction par dessus malloc qui protege. Personnelement pour certain projet critique j'ai besoin de le faire. Il suffit de faire une fonction du type :

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
13
 
void *xmalloc(size_t size)
{
  void *ptr = malloc(size);
 
  if (ptr == NULL)
  {
    write(STDERR_FILENO, "Memory Exausted\n", 16);
    exit(1);
  }
 
  return ptr;
}

[stephl]

Code :

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

fputs (input, filename);

Je suis d'accord, c'est une autre solution. J'ai juste voulu indiquer la manière correcte de faire en utilisant fprintf(), car le problème ne venait pas de la fonction utilisée, mais de l'appel qui en était fait.

[Invité1]

Merci de vos conseils.

J'ai refait le petit bout de code . . . Est-ce que c'est mieux?

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
 
	char *input;
	FILE *fname;
 
	input = (char *)malloc(32);
 
	if(input==NULL){
		puts("Memory allocation error");
		return 0;
	}
 
	fgets(input,32,stdin);
	fname = fopen("yourname.txt","w");
	if(!fname){
		puts("FILE ERROR");
		free(input);
		return 0;
	}
	fprintf(fname,"%s",input);
	fclose(fname);
	free(input);
 
	return 0;

Merci!
Alex

[stephl]

Merci de vos conseils.

J'ai refait le petit bout de code . . . Est-ce que c'est mieux?

Oui, c'est mieux. Mais je ne comprends toujours pas le nom de variable choisi (fname). Le filename, c'est ce que l'on passe à fopen() pour indiquer à quel fichier on souhaite accéder (ici, yourname.txt). fopen() ne retourne pas un nom de fichier, mais plutôt une sorte de handle, comme un ID (plus précisément, c'est un pointeur vers une structure comme l'indique la définition). On peut assimiler cela au fichier si on le souhaite et ainsi nommer la variable file, outputfile, etc... Personnellement, je trouve étrange d'utiliser le nom fname. Mais bon, ce n'est qu'un détail.

[Invité1]

Merci!

Ahh . . . pour le fname . . . Je savais juste pas quoi mettre et puis c'est juste un code d'exemple . . . C'est vrai que c'est pas un très bon nom

Merci beaucoup

Alex

[Obsidian]

Merci de vos conseils.

J'ai refait le petit bout de code . . . Est-ce que c'est mieux?

Tu progresses vite et bien, ça fait plaisir à voir ! Histoire de chipoter, puisque ton code est presque irréprochable, tu peux aussi prendre l'habitude d'initialiser tes variables et pointeurs à la déclaration :

Code :

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

1
2
char *input=NULL;
FILE *fname=NULL;

Tel qu'il est écrit, ton code présent ne peut pas planter, mais ça te rendra de grands services lors de développements de projets de plus grande envergure.

C'est pas forcément nécessaire si tu sais ce que tu veux car cela a un coût lors d'une allocation dynamique (y compris variables locales dans la pile), car il faut alors les effacer logiciellement, mais ce coût est minime pour de si petites variables.

[nicolas.sitbon]

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
45
#include <stdio.h>
#include <stdlib.h>
 
#define MY_FILE    "yourname.txt"
#define INPUT_SIZE 32
 
int main (void)
{
   int ret = EXIT_SUCCESS;
   FILE *fname = fopen (MY_FILE,"w");
 
   if (fname != NULL)
   {
      char *input = malloc (INPUT_SIZE * sizeof *input);
 
      if (input != NULL)
      {
         if (fgets (input, INPUT_SIZE, stdin) != NULL)
         {
            fputs(input, fname);
         }
         else if (ferror (stdin) != 0)
         {
            perror ("fgets");
            ret = EXIT_FAILURE;
         }
 
         free (input), input = NULL;
      }
      else
      {
         perror ("malloc");
         ret = EXIT_FAILURE;
      }
 
      fclose (fname), fname = NULL;
   }
   else
   {
      perror ("fopen");
      ret = EXIT_FAILURE;
   }
 
   return ret;
 }

[milouz123]

Je pense que ce code serait un peu plus lisible avec les clauses de sortie en cas d'erreur directement après chaque test.

[nicolas.sitbon]

Je pense que ce code serait un peu plus lisible avec les clauses de sortie en cas d'erreur directement après chaque test.

Tu n'as jamais du faire de gros projets!
Bonne pratique de programmation : un point d'entrée, un point de sortie.

[Emmanuel Delahaye]

J'ai refait le petit bout de code . . . Est-ce que c'est mieux?

C'est inutilement complexe et il manque l'essentiel.

1 - pourquoi allouer une taille fixe ?
2 - tous ces 'return' me donnent le tournis. Il y a une logique séquentielle à respecter pour écrire du code sérieux, sinon, un jour ou l'autre, ou oublie de libérer une ressource...

Le principe, c'est

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
 
T *p = allocation();
if (p != NULL)
{
   /* utilisation de p */
 
   liberation (p), p = NULL;
}
else
{
   /* erreur */
}

Et si on a affaire à un deuxième objet alloué, on imbrique.

Ca donne le code qu'a posté Nicolas... qui a été à bonne école !

Par contre attention. fgets() est certes sûre si on passe une taille cohérente et 2ème paramètre, par contre, il y a 2 choses importantes à garder en mémoire :

- si il y a la place, le caractère de fin de ligne ('\n') est stocké dans la chaine saisie.
- si il n'y a pas la place, la chaine est tronquée (mais reste valide), le '\n' n'est pas stocké dans la chaine, ainsi que les caractères qui le précèdent si il y en a. Le fait que le '\n' nait pas été lu fait que l'appel suivant de fgets() n'est pas bloquant.

Selon les besoins, il faut appliquer les traitements adéquates.

Si tu veux faire une fonction vraiment intéressante et utile (je m'en sers tous les jours), écris une saisie de ligne de taille quelconque à base de fgetc() et de realloc(). Là, l'usage de l'allocation dynamique sera justifié.

Code :

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

1
2
3
4
5
 
char *get_line_dyn (void)
{
   /* ... */
}

qui s'utilise évidemment comme ceci :

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
 
char *line = get_line_dyn();
if (line != NULL)
{
   /* utilisation de line */
 
   free (line), line = NULL;
}
else
{
   /* erreur */
}