Thema: Absicherung gegen Buffer Overflow?

[ehenkes]

Wichtiges Thema, daher neuer Thread:
Bei einer Fehlersuche bin ich wieder einmal auf einen Buffer Overflow bzw. das Überschreiben von zulässigen Grenzen im Speicher gestoßen. Das sind üble Fehler, die hinter jedem memcpy, strcpy usw. lauern.

Wie sichert man das in einem OS sauber ab?

[kevin]

Klingt blöd, aber am effektivsten: Statt C eine Sprache benutzen, die Range Checks kann. Wobei die zusätzlichen Prüfungen natürlich zu Lasten der Performance gehen. Geschenkt bekommt man nichts.

Wenn es C sein soll, kann man versuchen, vor und nach Speicherbereichen, die durch malloc alloziert worden sind, eine magische Konstante zu speichern und z.B. beim free oder beim nächsten malloc zu prüfen, ob sie noch intakt ist. Allerdings ist das auch nur noch hinterher feststellen, dass etwas passiert ist - im Zweifelsfall also zu spät. Bei Arrays auf dem Stack bist du eher komplett machtlos.

[Jidder]

Absichern kannst du da nichts. Die Funktionen können den Daten nicht ansehen, dass sie fehlerhaft sind.

Du kannst deinen Code mit Assertions ausstatten, die bei Unstimmigkeiten möglichst früh Warnungen geben. Aber die schützen nicht gegen Fehler, und sie sagen dir nicht mal zuverlässig, was die Ursache war.

[ehenkes]

Ich habe jetzt ein k_strncpy anstelle eines k_strcpy verwendet und eine Kopieranzahl eingestellt, die sicher ist.

Man sollte also eher strncpy und strncat verwenden.
In OpenBSD gibt es noch strlcpy und strlcat ("The strlcpy and strlcat functions guarantee that they will always null terminate the destination string when given a non-zero length argument.").

[erik.vikinger]

Hallo,

Wie sichert man das in einem OS sauber ab?

Bearbeite niemals ein Array ohne zu wissen wie groß das ist! In allen Programmen und nicht nur in einem OS. Entweder das Array ist ein Object das seine Größe selber kennt und prüft oder Du musst die Größe immer als zusätzliche Variable/Parameter mitführen und in Deinem Code entsprechende Checks implementieren.

Das sind üble Fehler, die hinter jedem memcpy, strcpy usw. lauern.

Deswegen sind diese Funktionen "depricated" und sollten nicht verwendet werden. Besser die Varianten mit 'n' im Namen verwenden oder andere Funktionen denen man ebenfalls die Größe des Ziel-Arrays mitteilen muss.


Grüße
Erik

[bluecode]

Das sind üble Fehler, die hinter jedem memcpy, strcpy usw. lauern.

Deswegen sind diese Funktionen "depricated" und sollten nicht verwendet werden. Besser die Varianten mit 'n' im Namen verwenden oder andere Funktionen denen man ebenfalls die Größe des Ziel-Arrays mitteilen muss.

Die sind nicht 'deprecated' (zumindest nicht nach dem C Standard), aber ich glaube Visual C(++) beschwert sich wenn man die verwendet und schlägt irgendwas anderes vor.

Ansonsten kann ich zB C++ empfehlen, da kann man strings ja relativ nett kapseln. Man muss das natürlich auch selbst implementieren und dann debuggen, aber dann läuft es halt. Außerdem taugt C++ von der Compilerunterstützung her mindestens genauso gut wie C zum OSDev.

[ehenkes]

Du musst die Größe immer als zusätzliche Variable/Parameter mitführen und in Deinem Code entsprechende Checks implementieren.

Dise Idee kam mir auch schon. In einem initrd-Filesystem könnte man das z.B. zusätzlich realisieren für die Namenslänge des Files.

[bluecode]

Wie genau soll das helfen? Normalerweise würde ich da strlen auf den nullterminierten String machen, dann genug Speicher allozieren und dann kopieren, falls du das meinst. Ein statisches Array tut da halt nicht. Und eine Meldung a la "array zu kurz" nützt m.E. einem User überhaupt nichts.
Oder woher kam dein Buffer-Overflow genau?

[ehenkes]

Alles ok: Im Filesystem initrd.img steht File1'\0' als Name. 5 Characters werden kopiert.
Problem: Hinter File1 ist das '\0' weg (Ursache momentan unklar, kommt vom PC, da es von gleicher Disk bei einem PC klappt, beim anderen nicht). Dadurch wird solange kopiert, bis der statische Buffer des Ziels überschrieben wird (im konkreten fall 128 Byte). Mit strncpy kann man das auf 64 (höchste zulässige Zahl im initrd-Filesystem fest begrenzen. Man kann beim Erstellen des Filesystems aber auch 5 mit rein schreiben, also nicht nur filename, offset, size, sondern filename, filename-length, offset, size. Dann kann man strncpy(dest, filename-length, src) machen, nachdem man filename-length vorher auf <=64 abgefragt hat. Dann werden sinnlose Zeichen abgeschnitten.

Dir Ursache für das Fehlverhalten ist mir leider noch unbekannt. Ich weiß durch Prints (auch in strcpy) bisher nur die Stelle, an der es sich auswirkt, und kann das dort nun bekämpfen. Bisher kam es zum Anhalten durch unverständliche #PF. 
 

[ChristianF]

Kommen der #PF beim durchsuchen der InitRD oder beim Ausführen von einem Programm?

[kevin]

Problem: Hinter File1 ist das '\0' weg (Ursache momentan unklar, kommt vom PC, da es von gleicher Disk bei einem PC klappt, beim anderen nicht). Dadurch wird solange kopiert, bis der statische Buffer des Ziels überschrieben wird (im konkreten fall 128 Byte). Mit strncpy kann man das auf 64 (höchste zulässige Zahl im initrd-Filesystem fest begrenzen. Man kann beim Erstellen des Filesystems aber auch 5 mit rein schreiben, also nicht nur filename, offset, size, sondern filename, filename-length, offset, size. Dann kann man strncpy(dest, filename-length, src) machen, nachdem man filename-length vorher auf <=64 abgefragt hat. Dann werden sinnlose Zeichen abgeschnitten.

Außer die 5 ist überschrieben. Wenn dein Puffer 64 Bytes groß ist, dann übergibst du am besten auch die 64 an strncpy, und nichts anderes.

[ehenkes]

Kommen der #PF beim durchsuchen der InitRD oder beim Ausführen von einem Programm?

Eindeutig beim Durchsuchen der InitRD. Die Stelle habe ich ja gefunden, indem ich ein print in strcpy eingebaut habe:

Code: [Auswählen]

        // Create a new file node.
        k_strcpy(root_nodes[i].name, file_headers[i].name);
Nun steht da:

Code: [Auswählen]

k_strncpy(root_nodes[i].name, file_headers[i].name, 64); ... wie von taljeth vorgeschlagen.

[ChristianF]

Ist er dadurch behoben, oder tritt dieser noch immer auf?

[Jidder]

Wenn der Puffer nicht nullterminiert wird (root_nodes.name[63] = 0;) dann ja.

[ehenkes]

Code: [Auswählen]

k_strncpy(root_nodes[i].name, file_headers[i].name, 64); /// critical !!!
root_nodes[i].name[64] = 0;
root_nodes.name hat 128 Character, nur das Filesystem ist momentan beschränkt auf 64. 

[Jidder]

Wenn du das Byte bei root_nodes.name[64] auf 0 setzt, hast du zwar ein paar der Probleme bei strlen(file_headers.name) >= 64 ausgeschlossen, aber die maximal korrekte Terminierung des Puffers wäre bei root_nodes.name[63], weil auch spätestens file_headers.name[63] = 0 sein müsste.

Nur mal so als Kommentar um Eins(TM).

[ehenkes]

Danke für den Hinweis. Bei mir tritt dieser merkwürdige PC-abhängige "Fehler" momentan nicht auf. Bin auf Tests auf gewissen PCs an anderer Stelle gespannt (muss urlaubsbedingt noch etwas warten). Wichtig ist, dass das OS gegen Absturz und bewusste "hacks" gesichert ist.

[ehenkes]

Alles soweit o.k., Tests waren erfolgreich. Ich habe inzwischen bezüglich des Bootvorgangs auch auf Nutzung von FAT12 umgerüstet, so dass der Dateiverwaltungsaufbau der Diskette transparenter und nützlicher wird:
http://www.henkessoft.de/OS_Dev/OS_Dev3.htm#mozTocId188479
http://www.henkessoft.de/OS_Dev/Downloads/97.zip