für sich genommen die Probleme sind:
- Sie können prüfen, ob
f nicht NULL ist.
- Sie überprüfen nicht, dass
arrmap nicht NULL ist.
- Sie überprüfen nicht, ob die erste
fread() erfolgreich war.
- Der Wert, der gelesen wird, wird nicht vollständig validiert. negative oder Nullwerte werden die Dinge schlecht abwerfen.
- Sie überprüfen nicht, dass der zweite
fread() erfolgreich war.
- Sie geben immer
-1 zurück, unabhängig davon, ob etwas funktioniert oder fehlgeschlagen ist.
Welche davon sind Sicherheitsprobleme? Auf einigen Ebenen, alle von ihnen. In einigen Kontexten können Sie davon ausgehen, dass f und arrmap ohne Überprüfung legitim sind. Nicht zu überprüfen, dass die Lesevorgänge erfolgreich sind, insbesondere der erste, ist ein ernstes Problem. Die negativen Werte für n_map wären ein ernstes Problem. Erfolg zu beanspruchen, wenn jeder Lesevorgang fehlschlug, wäre ein Problem.
Wenn die Schleife abgeschlossen ist, wird n_map auf -1 festgelegt (es war vor dem Nach-Dekrement Null). Sie geben also -1 bei Fehlschlag oder Erfolg zurück. Das ist nicht hilfreich. Es sollte fast sicher den Wert n_map zurückgeben, der von Datei gelesen wurde, so kann der Aufrufer ermitteln, wie viele Werte in dem Array sind.
Es ist normalerweise am besten, keine Größenbeschränkung wie 256 in das Programm zu schreiben. Die Schnittstelle sollte die Array-Größe enthalten und Sie sollten anhand der übergebenen Array-Größe überprüfen.
auf die ursprüngliche Schnittstelle arbeiten, könnten Sie verwenden:
#include "stderr.h"
#include <stdio.h>
#include <inttypes.h>
extern int32_t read_arrbuff(FILE *f, uint32_t *arrmap);
int32_t read_arrbuff(FILE *f, uint32_t *arrmap)
{
int32_t n_map;
if (fread(&n_map, sizeof(n_map), 1, f) != 1)
err_syserr("failed to read data (count)\n");
if (n_map > 256 || n_map <= 0)
return -1;
for (int32_t i = 0; i < n_map; i++)
{
if (fread(&arrmap[i], sizeof(uint32_t), 1, f) != 1)
err_syserr("failed to read data (value %" PRId32 ")\n", i);
}
return n_map;
}
int main(int argc, char **argv)
{
err_setarg0(argv[0]);
if (argc != 1)
err_usage("");
char file[] = "data";
/* Create data file */
FILE *fp = fopen(file, "wb");
if (fp == NULL)
err_syserr("failed to open file '%s' for writing\n", file);
int32_t nmap = 32;
if (fwrite(&nmap, sizeof(nmap), 1, fp) != 1)
err_syserr("failed to write to file '%s'\n", file);
for (int32_t i = 0; i < nmap; i++)
{
if (fwrite(&i, sizeof(i), 1, fp) != 1)
err_syserr("failed to write to file '%s'\n", file);
}
fclose(fp);
/* Read data file */
fp = fopen(file, "rb");
if (fp == NULL)
err_syserr("failed to open file '%s' for reading\n", file);
uint32_t amap[256];
int32_t rc = read_arrbuff(fp, amap);
printf("rc = %" PRId32 "\n", rc);
for (int32_t i = 0; i < rc; i++)
printf("%3" PRId32 " = %3" PRId32 "\n", i, amap[i]);
fclose(fp);
return 0;
}
können Sie diskutieren, ob die einseitige durch err_syserr() auferlegten Ausfahrten geeignet sind. (Die Erklärungen und Quelle für die err_*() Funktionen ist in stderr.h und stderr.c und erhältlich ist aus GitHub.)
Eine alternative Version der maximale Arraygröße von einem Funktionsparameter zu nehmen ist:
#include "stderr.h"
#include <assert.h>
#include <stdio.h>
#include <inttypes.h>
extern int32_t read_arrbuff(FILE *f, int32_t a_size, uint32_t arrmap[a_size]);
int32_t read_arrbuff(FILE *f, int32_t a_size, uint32_t arrmap[a_size])
{
int32_t n_map;
assert(f != NULL && arrmap != NULL && a_size > 0 && a_size <= 256);
if (fread(&n_map, sizeof(n_map), 1, f) != 1)
{
err_sysrem("failed to read data (count)\n");
return -1;
}
if (n_map > a_size || n_map <= 0)
{
err_sysrem("count %" PRId32 " is out of range 1..%" PRId32 "\n",
n_map, a_size);
return -1;
}
for (int32_t i = 0; i < n_map; i++)
{
if (fread(&arrmap[i], sizeof(uint32_t), 1, f) != 1)
{
err_syserr("failed to read data (value %" PRId32 " of %" PRId32 ")\n",
i, n_map);
}
}
return n_map;
}
int main(int argc, char **argv)
{
err_setarg0(argv[0]);
if (argc != 1)
err_usage("");
char file[] = "data";
/* Create data file */
FILE *fp = fopen(file, "wb");
if (fp == NULL)
err_syserr("failed to open file '%s' for writing\n", file);
int32_t nmap = 32;
if (fwrite(&nmap, sizeof(nmap), 1, fp) != 1)
err_syserr("failed to write to file '%s'\n", file);
for (int32_t i = 0; i < nmap; i++)
{
if (fwrite(&i, sizeof(i), 1, fp) != 1)
err_syserr("failed to write to file '%s'\n", file);
}
fclose(fp);
/* Read data file */
fp = fopen(file, "rb");
if (fp == NULL)
err_syserr("failed to open file '%s' for reading\n", file);
enum { AMAP_SIZE = 256 };
uint32_t amap[AMAP_SIZE];
int32_t rc = read_arrbuff(fp, AMAP_SIZE, amap);
printf("rc = %" PRId32 "\n", rc);
for (int32_t i = 0; i < rc; i++)
printf("%3" PRId32 " = %3" PRId32 "\n", i, amap[i]);
fclose(fp);
return 0;
}
Diese Version Berichte ein bestimmter Fehler und gibt einen Fehler zurück. Es macht semi-angemessene Assertions bei der Eingabe (das 256-Limit ist nicht unbedingt angemessen, stimmt aber mit dem ursprünglichen Code überein).
Der Ausgang ist nicht aufregend (und dasselbe von beiden):
rc = 32
0 = 0
1 = 1
2 = 2
3 = 3
4 = 4
5 = 5
6 = 6
7 = 7
8 = 8
9 = 9
10 = 10
11 = 11
12 = 12
13 = 13
14 = 14
15 = 15
16 = 16
17 = 17
18 = 18
19 = 19
20 = 20
21 = 21
22 = 22
23 = 23
24 = 24
25 = 25
26 = 26
27 = 27
28 = 28
29 = 29
30 = 30
31 = 31
Angenommen, die erste int Lese einen negativen Wert hat ... Was denken Sie, die Funktion macht? –
Oder wenn 'arrmap' kleiner ist als die Anzahl der Bytes, die Sie lesen möchten. –
Dies ist eine seltsame Art, eine solche Funktion zu schreiben, um ehrlich zu sein. –