Wir schießen in den Fuß und verarbeiten die Eingabedaten

Der Link im heutigen Artikel unterscheidet sich vom üblichen. Dies ist nicht ein Projekt, für das der Quellcode analysiert wurde, sondern eine Reihe von Antworten derselben Diagnoseregel in mehreren verschiedenen Projekten. Was ist das Interesse hier? Die Tatsache, dass einige der betrachteten Codefragmente Fehler enthalten, die bei der Arbeit mit der Anwendung reproduzierbar sind, während andere insgesamt Schwachstellen (CVE) enthalten. Darüber hinaus werden wir am Ende des Artikels ein wenig über das Thema Sicherheitsmängel diskutieren.

Kurzes Vorwort

Alle Fehler, die heute im Artikel berücksichtigt werden, haben ein ähnliches Muster:

• Das Programm empfängt Daten vom Standard- Stream.
• Es wird geprüft, ob Daten erfolgreich gelesen wurden.
• Wenn die Daten erfolgreich gelesen wurden, wird das Übertragszeichen aus der Zeile entfernt.

Alle Fragmente, die berücksichtigt werden, enthalten jedoch Fehler und sind anfällig für manipulierte Eingaben. Da Daten von einem Benutzer empfangen werden, der gegen die Logik der Anwendungsausführung verstoßen kann, bestand die große Versuchung, etwas zu beschädigen. Was ich getan habe.

Alle folgenden Probleme wurden vom statischen Analysator PVS-Studio entdeckt , der nicht nur nach C, C ++, sondern auch nach C #, Java nach Fehlern im Code sucht.

Natürlich ist es gut, ein Problem mit einem statischen Analysegerät zu finden, aber das Finden und Reproduzieren ist ein völlig anderes Vergnügen. :) :)

Freischalter

Das erste verdächtige Codefragment wurde im Code des Moduls fs_cli.exe gefunden , das Teil des FreeSWITCH-Distributionskits ist:

static const char *basic_gets(int *cnt) { .... int c = getchar(); if (c < 0) { if (fgets(command_buf, sizeof(command_buf) - 1, stdin) != command_buf) { break; } command_buf[strlen(command_buf)-1] = '\0'; /* remove endline */ break; } .... } 

PVS-Studio Warnung : V1010 CWE-20 Nicht aktivierte fehlerhafte Daten werden im Index 'strlen (command_buf)' verwendet.

Der Analysator warnt vor einem verdächtigen Aufruf des Arrays command_buf per Index. Es wird als verdächtig angesehen, da nicht überprüfte externe Daten als Index verwendet werden. Extern - weil sie über die Funktion fgets aus dem Standard- Stream abgerufen werden. Nicht überprüft - da vor der Verwendung keine Überprüfung durchgeführt wurde. Der Ausdruck fgets (command_buf, ....)! = Command_buf zählt nicht, da wir auf diese Weise nur die Tatsache überprüfen, dass die Daten empfangen werden, nicht aber deren Inhalt.

Das Problem mit diesem Code ist, dass unter bestimmten Bedingungen '\ 0' außerhalb des Arrays geschrieben wird, was zu undefiniertem Verhalten führt. Geben Sie dazu einfach eine Zeichenfolge mit der Länge Null ein (eine Zeichenfolge mit der Länge Null aus Sicht der C-Sprache, dh eine Zeichenfolge, in der das erste Zeichen '\ 0' lautet).

Lassen Sie uns abschätzen, was passiert, wenn Sie eine Zeichenfolge mit der Länge Null an die Eingabe übergeben:

• fgets (command_buf, ....) -> command_buf ;
• fgets (....)! = command_buf -> false ( dann wird der Zweig der if-Anweisung ignoriert);
• strlen (command_buf) -> 0 ;
• command_buf [strlen (command_buf) - 1] -> command_buf [-1] .

Ups!

Interessant ist hier, dass diese Analysatorwarnung "mit den Händen gefühlt" werden kann. Um das Problem zu wiederholen, benötigen Sie:

• Bringen Sie das Programm zu dieser Funktion.
• Passen Sie den Eingang so an, dass der Aufruf von getchar () einen negativen Wert zurückgibt.
• Übergeben Sie für die Funktion fgets eine Zeile mit einem Terminal Null am Anfang, die erfolgreich gelesen werden soll.

Ich stöberte ein wenig in der Quelle und stellte eine bestimmte Sequenz für die Reproduktion des Problems zusammen:

• Führen Sie fs_cli.exe im Stapelmodus aus ( fs_cli.exe -b ). Ich stelle fest, dass die Verbindung fs_cli.exe zum Server erfolgreich sein muss, um weitere Schritte ausführen zu können. Dazu reicht es beispielsweise aus, FreeSwitchConsole.exe lokal als Administrator auszuführen .
• Wir führen die Eingabe so aus, dass der Aufruf von getchar () einen negativen Wert zurückgibt.
• Geben Sie am Anfang eine Zeile mit einem Terminal Null ein (z. B. '\ 0Oooops').
• ....
• GEWINN!

Das Folgende ist eine Videowiedergabe des Problems:

Ncftp

Ein ähnliches Problem wurde im NcFTP-Projekt entdeckt, das jedoch bereits an zwei Stellen aufgetreten ist. Da der Code ähnlich aussieht, sollten Sie nur einen problematischen Ort berücksichtigen:

 static int NcFTPConfirmResumeDownloadProc(....) { .... if (fgets(newname, sizeof(newname) - 1, stdin) == NULL) newname[0] = '\0'; newname[strlen(newname) - 1] = '\0'; .... } 

PVS-Studio Warnung : V1010 CWE-20 Nicht aktivierte fehlerhafte Daten werden im Index 'strlen (neuer Name)' verwendet.

Im Gegensatz zum Beispiel von FreeSWITCH ist der Code hier schlechter geschrieben und anfälliger für Probleme. Das Schreiben von '\ 0' erfolgt beispielsweise unabhängig davon, ob der Lesevorgang mit fgets erfolgreich war oder nicht. Das heißt, es gibt noch mehr Möglichkeiten, die normale Ausführungslogik zu brechen. Gehen wir auf bewährte Weise durch Linien mit der Länge Null.

Das reproduzierte Problem ist etwas komplizierter als bei FreeSWITCH. Die Reihenfolge der Schritte wird nachfolgend beschrieben:

• Starten und Herstellen einer Verbindung mit dem Server, von dem Sie die Datei herunterladen können. Zum Beispiel habe ich speedtest.tele2.net verwendet (am Ende sieht der Befehl zum Starten der Anwendung folgendermaßen aus: ncftp.exe speedtest.tele2.net );
• Herunterladen einer Datei vom Server. Lokal sollte bereits eine Datei mit demselben Namen, aber unterschiedlichen Eigenschaften vorhanden sein. Sie können beispielsweise eine Datei vom Server herunterladen, ändern und versuchen, den Download-Befehl erneut auszuführen (z. B. 512 KB. Zip ).
• Beantworten Sie die Frage zur Auswahl einer Aktion mit einer Zeile, die mit dem Zeichen 'N' beginnt (z. B. Jetzt haben wir Spaß ).
• Geben Sie '\ 0' (oder etwas interessanteres) ein.
• ....
• GEWINN!

Die Reproduktion des Problems wird auch auf Video aufgezeichnet:

Openldap

Im OpenLDAP-Projekt (genauer gesagt in einem der zugehörigen Dienstprogramme) traten sie auf den gleichen Rechen wie in FreeSWITCH. Ein Versuch, ein Zeilenumbruchzeichen zu löschen, erfolgt nur, wenn die Zeile erfolgreich gelesen wurde, es besteht jedoch auch kein Schutz gegen Zeilen mit der Länge Null.

Code-Snippet:

 int main( int argc, char **argv ) { char buf[ 4096 ]; FILE *fp = NULL; .... if (....) { fp = stdin; } .... if ( fp == NULL ) { .... } else { while ((rc == 0 || contoper) && fgets(buf, sizeof(buf), fp) != NULL) { buf[ strlen( buf ) - 1 ] = '\0'; /* remove trailing newline */ if ( *buf != '\0' ) { rc = dodelete( ld, buf ); if ( rc != 0 ) retval = rc; } } } .... } 

PVS-Studio Warnung : V1010 CWE-20 Ungeprüfte fehlerhafte Daten werden im Index 'strlen (buf)' verwendet.

Wir werfen den Überschuss weg, damit die Essenz des Problems offensichtlicher wird:

 while (.... && fgets(buf, sizeof(buf), fp) != NULL) { buf[ strlen( buf ) - 1 ] = '\0'; .... } 

Dieser Code ist besser als NcFTP, aber immer noch anfällig. Wenn auf Anforderung eine Zeichenfolge mit der Länge Null an die Eingabe übergeben werden soll:

• fgets (buf, ....) -> buf ;
• fgets (....)! = NULL -> true (der Körper der while-Schleife beginnt auszuführen);
• strlen (buf) - 1 -> 0 - 1 -> -1 ;
• buf [-1] = '\ 0' .

libidn

Trotz der Tatsache, dass die oben diskutierten Fehler sehr interessant sind (sie werden stabil reproduziert und können "berührt" werden (außer dass ich das OpenLDAP-Problem nicht erreichen konnte)), können sie nicht als Schwachstellen bezeichnet werden, schon allein deshalb Problemen werden keine CVE-Kennungen zugewiesen.

Einige echte Sicherheitslücken weisen jedoch das gleiche Problemmuster auf. Die beiden folgenden Codefragmente gelten für das libidn-Projekt.

Code-Snippet:

 int main (int argc, char *argv[]) { .... else if (fgets (readbuf, BUFSIZ, stdin) == NULL) { if (feof (stdin)) break; error (EXIT_FAILURE, errno, _("input error")); } if (readbuf[strlen (readbuf) - 1] == '\n') readbuf[strlen (readbuf) - 1] = '\0'; .... } 

PVS-Studio Warnung : V1010 CWE-20 Nicht aktivierte fehlerhafte Daten werden im Index 'strlen (readbuf)' verwendet.

Die Situation ist ähnlich, mit der Ausnahme, dass im Gegensatz zu den vorherigen Beispielen, in denen die Aufzeichnung mit dem Index -1 durchgeführt wurde , hier gelesen wird. Dies ist jedoch immer noch undefiniertes Verhalten. Diesem Fehler wurde eine eigene CVE-Kennung zugewiesen ( CVE-2015-8948 ).

Nachdem ein Problem festgestellt wurde, wurde dieser Code wie folgt geändert:

 int main (int argc, char *argv[]) { .... else if (getline (&line, &linelen, stdin) == -1) { if (feof (stdin)) break; error (EXIT_FAILURE, errno, _("input error")); } if (line[strlen (line) - 1] == '\n') line[strlen (line) - 1] = '\0'; .... } 

Ein bisschen überrascht? Es passiert. Neue Sicherheitsanfälligkeit, entsprechende CVE-Kennung: CVE-2016-6262 .

PVS-Studio Warnung : V1010 CWE-20 Nicht aktivierte fehlerhafte Daten werden im Index 'strlen (line)' verwendet.

Bei einem anderen Versuch wurde das Problem behoben, indem die Länge der Eingabezeichenfolge überprüft wurde:

 if (strlen (line) > 0) if (line[strlen (line) - 1] == '\n') line[strlen (line) - 1] = '\0'; 

Werfen wir einen Blick auf die Daten. Das Commit 'Closing' CVE-2015-8948 - 08/10/2015. Commit Closing CVE-2016-62-62 - 14.01.2016. Das heißt, der Unterschied zwischen den oben genannten Korrekturen beträgt 5 Monate ! Hier erinnern Sie sich an einen solchen Vorteil der statischen Analyse wie das Erkennen von Fehlern in den frühen Phasen des Codeschreibens ...

Statische Analyse und Sicherheit

Es wird keine weiteren Codebeispiele geben, sondern Statistiken und Argumente. In diesem Abschnitt stimmt die Meinung des Autors möglicherweise nicht viel mehr mit der Meinung des Lesers überein als zuvor in diesem Artikel.

Hinweis Ich empfehle Ihnen, einen weiteren Artikel zu einem ähnlichen Thema zu lesen: " Wie kann PVS-Studio beim Auffinden von Sicherheitslücken helfen? ". Es gibt interessante Beispiele für Sicherheitslücken, die wie einfache Fehler aussehen. Darüber hinaus habe ich in diesem Artikel ein wenig über die Terminologie gesprochen und warum statische Analysen ein Muss sind, wenn Sie sich Gedanken über das Sicherheitsthema machen.

Schauen wir uns die Statistiken zur Anzahl der in den letzten 10 Jahren entdeckten Sicherheitslücken an, um die Situation zu bewerten. Ich habe die Daten von der CVE Details- Website übernommen.

Eine interessante Situation zeichnet sich ab. Bis 2014 hat die Anzahl der erfassten CVEs die Marke von 6.000 Einheiten nicht überschritten und ist ab - sie ist nicht darunter gefallen. Am interessantesten ist hier natürlich die Statistik für 2017 - der absolute Spitzenreiter (14.714 Einheiten). Das laufende Jahr 2018 ist noch nicht zu Ende, bricht aber bereits Rekorde - 15.310 Einheiten.

Bedeutet dies, dass jede neue Software wie ein Sieb voller Löcher ist? Ich glaube nicht und hier ist der Grund:

• Erhöhtes Interesse am Thema Schwachstellen. Selbst wenn Sie dem Thema Sicherheit nicht sehr nahe stehen, sind Sie sicherlich wiederholt auf Artikel, Notizen, Berichte und Videos zum Thema Sicherheit gestoßen. Mit anderen Worten, es entstand eine Art „Hype“. Ist es schlecht Wahrscheinlich nicht. Am Ende kommt es darauf an, dass Entwickler sich mehr um die Anwendungssicherheit sorgen, was gut ist.
• Eine Zunahme der Anzahl der entwickelten Anwendungen. Mehr Code - es ist wahrscheinlicher, dass eine Sicherheitsanfälligkeit auftritt, durch die die Statistiken wieder aufgefüllt werden.
• Verbesserte Tools zur Suche nach Sicherheitslücken und zur Sicherung der Codequalität. Mehr Nachfrage -> mehr Angebot. Analysatoren, Fuzzers und andere Tools werden immer weiter entwickelt, was denjenigen in die Hände spielt, die nach Schwachstellen suchen möchten (unabhängig davon, auf welcher Seite der Barrikaden sie sich befinden).

Der aufkommende Trend kann daher nicht ausschließlich als negativ bezeichnet werden. Die Verlage sind mehr besorgt über die Informationssicherheit, die Tools zur Problemfindung werden verbessert, und all dies ist zweifellos positiv.

Bedeutet dies, dass Sie sich entspannen und nicht „baden“ können? Ich denke nicht. Wenn Sie sich Gedanken über das Sicherheitsthema Ihrer Anwendungen machen, sollten Sie so viele Sicherheitsmaßnahmen wie möglich ergreifen. Dies gilt insbesondere dann, wenn der Quellcode gemeinfrei ist, da:

• anfälliger für die Einbettung externer Schwachstellen;
• anfälliger für "Nachforschungen" durch jene "Herren", die an Lücken in Ihrer Anwendung interessiert sind, um sie auszunutzen. Obwohl Gratulanten in diesem Fall in der Lage sein werden, Ihnen mehr zu helfen.

Ich möchte nicht sagen, dass Sie Ihre Projekte nicht unter Open Source übersetzen müssen. Beachten Sie einfach die entsprechenden Qualitäts- / Sicherheitskontrollen.

Ist die statische Analyse eine zusätzliche Maßnahme? Ja Die statische Analyse erkennt potenzielle Schwachstellen, die in Zukunft möglicherweise auftreten werden.

Es scheint mir (ich gebe zu, dass ich mich irre), dass viele Schwachstellen als ein ziemlich hochrangiges Phänomen betrachten. Ja und nein. Codeprobleme, die wie einfache Programmierfehler erscheinen, können ebenfalls schwerwiegende Sicherheitslücken sein. Wiederum finden Sie einige Beispiele für solche Sicherheitslücken im zuvor erwähnten Artikel . Unterschätzen Sie nicht die "einfachen" Fehler.

Fazit

Vergessen Sie nicht, dass die Eingabedaten eine Länge von Null haben können, und dies muss ebenfalls berücksichtigt werden.

Schlussfolgerungen darüber, ob der ganze Hype mit Schwachstellen nur ein Hype ist oder ob das Problem besteht, machen Sie es selbst.

Ich für meinen Teil, es sei denn, ich schlage vor, Ihr Projekt PVS-Studio anzuprobieren , falls Sie dies noch nicht getan haben.

Alles Gute!

Wenn Sie diesen Artikel einem englischsprachigen Publikum zugänglich machen möchten, verwenden Sie bitte den Link zur Übersetzung: Sergey Vasiliev. Schießen Sie sich beim Umgang mit Eingabedaten in den Fuß