Dies ist die zweite Überprüfung in einer Reihe von Artikeln zur Überprüfung von Open Source-Programmen auf die Arbeit mit dem RDP-Protokoll. Darin werden wir den rdesktop-Client und den xrdp-Server betrachten.
PVS-Studio wurde als Tool zur Fehlererkennung verwendet. Es ist ein statischer Code-Analysator für C, C ++, C # und Java, der unter Windows, Linux und macOS verfügbar ist.
Der Artikel präsentiert nur die Fehler, die mir interessant erschienen. Die Projekte sind jedoch klein, so dass es nur wenige Fehler gab :).
Hinweis Den vorherigen Artikel zum Überprüfen des FreeRDP-Projekts finden Sie
hier .
rdesktop
rdesktop ist eine kostenlose Implementierung des RDP-Clients für UNIX-basierte Systeme. Es kann auch unter Windows verwendet werden, wenn Sie ein Projekt unter Cygwin erstellen. Lizenziert unter GPLv3.
Dieser Client ist sehr beliebt - er wird standardmäßig in ReactOS verwendet. Sie können auch grafische Frontends von Drittanbietern dafür finden. Trotzdem ist er ziemlich alt: Die erste Veröffentlichung fand am 4. April 2001 statt - zum Zeitpunkt des Schreibens ist er 17 Jahre alt.
Wie ich bereits erwähnt habe, ist das Projekt sehr klein. Es enthält ungefähr 30.000 Codezeilen, was angesichts seines Alters etwas seltsam ist. Zum Vergleich enthält FreeRDP 320.000 Zeilen. Hier ist die Ausgabe des Cloc-Programms:
Nicht erreichbarer Code
V779 Nicht erreichbarer Code erkannt. Möglicherweise liegt ein Fehler vor. rdesktop.c 1502
int main(int argc, char *argv[]) { .... return handle_disconnect_reason(deactivated, ext_disc_reason); if (g_redirect_username) xfree(g_redirect_username); xfree(g_username); }
Der Fehler trifft uns sofort in der Hauptfunktion: Wir sehen den Code, der nach der
return-Anweisung kommt - dieses Fragment bereinigt den Speicher. Trotzdem stellt der Fehler keine Bedrohung dar: Der gesamte zugewiesene Speicher wird vom Betriebssystem nach Beendigung des Programms bereinigt.
Fehlende Fehlerbehandlung
V557 Array-Unterlauf ist möglich. Der Wert des 'n'-Index könnte -1 erreichen. rdesktop.c 1872
RD_BOOL subprocess(char *const argv[], str_handle_lines_t linehandler, void *data) { int n = 1; char output[256]; .... while (n > 0) { n = read(fd[0], output, 255); output[n] = '\0';
Das Code-Snippet liest in diesem Fall von der Datei in den Puffer, bis die Datei endet. Hier gibt es jedoch keine Fehlerbehandlung: Wenn etwas schief geht, gibt
read -1 zurück, und das Ausgabearray überschreitet die Grenzen des Arrays.
Verwenden von EOF in char
V739 EOF sollte nicht mit einem Wert vom Typ 'char' verglichen werden. Das '(c = fgetc (fp))' sollte vom Typ 'int' sein. Strg.c 500
int ctrl_send_command(const char *cmd, const char *arg) { char result[CTRL_RESULT_SIZE], c, *escaped; .... while ((c = fgetc(fp)) != EOF && index < CTRL_RESULT_SIZE && c != '\n') { result[index] = c; index++; } .... }
Hier sehen wir eine falsche Verarbeitung beim Erreichen des Dateiende: Wenn
fgetc ein Zeichen mit dem Code 0xFF zurückgibt, wird es als Dateiende (
EOF ) interpretiert.
EOF ist eine Konstante, die normalerweise als -1 definiert wird. Bei der Codierung von CP1251 hat beispielsweise der letzte Buchstabe des russischen Alphabets den Code 0xFF, der der Zahl -1 entspricht, wenn es sich um eine Variable vom Typ
char handelt . Es stellt sich heraus, dass das Zeichen 0xFF wie
EOF (-1) als das Ende der Datei wahrgenommen wird. Um solche Fehler zu vermeiden, sollte das Ergebnis der Funktion
fgetc in einer
int- Variablen gespeichert werden.
Tippfehler
Fragment 1V547 Der Ausdruck 'write_time' ist immer falsch. disk.c 805
RD_NTSTATUS disk_set_information(....) { time_t write_time, change_time, access_time, mod_time; .... if (write_time || change_time) mod_time = MIN(write_time, change_time); else mod_time = write_time ? write_time : change_time;
Vielleicht hat der Autor dieses Codes
|| verwirrt und
&& zur Verfügung gestellt. Berücksichtigen Sie die möglichen Optionen für
write_time und
change_time :
- Beide Variablen sind 0: In diesem Fall gelangen wir zum Zweig else : Die Variable mod_time ist unabhängig von der folgenden Bedingung immer 0.
- Eine der Variablen ist 0: mod_time ist 0 (vorausgesetzt, die andere Variable hat einen nicht negativen Wert), da MIN die kleinste der beiden Optionen auswählt.
- Beide Variablen sind ungleich 0: Wir wählen den Minimalwert.
Wenn Sie eine Bedingung durch
write_time && change_time ersetzen, sieht das Verhalten korrekt aus:
- Eine oder beide Variablen sind ungleich 0: Wählen Sie einen Wert ungleich Null.
- Beide Variablen sind ungleich 0: Wir wählen den Minimalwert.
Fragment 2V547 Ausdruck ist immer wahr. Wahrscheinlich sollte hier der Operator '&&' verwendet werden. disk.c 1419
static RD_NTSTATUS disk_device_control(RD_NTHANDLE handle, uint32 request, STREAM in, STREAM out) { .... if (((request >> 16) != 20) || ((request >> 16) != 9)) return RD_STATUS_INVALID_PARAMETER; .... }
Anscheinend sind die Betreiber
|| und
&& , entweder
== und
! = : Die Variable kann nicht gleichzeitig die Werte 20 und 9 annehmen.
Unbegrenztes Kopieren von Zeilen
V512 Ein Aufruf der Funktion 'sprintf' führt zum Überlaufen des Puffers 'fullpath'. disk.c 1257
RD_NTSTATUS disk_query_directory(....) { .... char *dirname, fullpath[PATH_MAX]; .... sprintf(fullpath, "%s/%s", dirname, pdirent->d_name); .... }
Bei Betrachtung der Funktion wird völlig klar, dass dieser Code keine Probleme verursacht. Sie können jedoch in Zukunft auftreten: Eine unachtsame Änderung, und wir erhalten einen Pufferüberlauf -
sprintf ist durch nichts begrenzt. Wenn wir also Pfade verketten, können wir die Grenzen des Arrays überschreiten. Es wird empfohlen, diesen Aufruf auf
snprintf (fullpath, PATH_MAX, ....) zu beachten .
Redundanter Zustand
V560 Ein Teil des bedingten Ausdrucks ist immer wahr: addiere> 0. scard.c 507
static void inRepos(STREAM in, unsigned int read) { SERVER_DWORD add = 4 - read % 4; if (add < 4 && add > 0) { .... } }
Das Aktivieren von
add> 0 ist nutzlos: Die Variable ist immer größer als Null, da
read% 4 den Rest der Division zurückgibt und niemals 4 ist.
xrdp
xrdp ist eine Open Source-RDP-
Serverimplementierung . Das Projekt ist in 2 Teile gegliedert:
- xrdp - Protokollimplementierung. Unter der Apache 2.0-Lizenz vertrieben.
- xorgxrdp - Eine Reihe von Xorg-Treibern zur Verwendung mit xrdp. Lizenz - X11 (als MIT, verbietet jedoch die Verwendung in der Werbung)
Die Projektentwicklung basiert auf den Ergebnissen von rdesktop und FreeRDP. Anfangs musste ich einen separaten VNC-Server verwenden, um mit Grafiken zu arbeiten, oder einen speziellen X11-Server mit RDP-Unterstützung - X11rdp, aber mit dem Aufkommen von xorgxrdp wurden sie nicht mehr benötigt.
In diesem Artikel werden wir nicht auf xorgxrdp eingehen.
Das xrdp-Projekt ist wie das vorherige sehr klein und enthält ungefähr 80.000 Zeilen.
Weitere Tippfehler
V525 Der Code enthält die Sammlung ähnlicher Blöcke. Überprüfen Sie die Elemente 'r', 'g', 'r' in den Zeilen 87, 88, 89. rfxencode_rgb_to_yuv.c 87
static int rfx_encode_format_rgb(const char *rgb_data, int width, int height, int stride_bytes, int pixel_format, uint8 *r_buf, uint8 *g_buf, uint8 *b_buf) { .... switch (pixel_format) { case RFX_FORMAT_BGRA: .... while (x < 64) { *lr_buf++ = r; *lg_buf++ = g; *lb_buf++ = r;
Dieser Code wurde aus der Bibliothek librfxcodec übernommen, die den jpeg2000-Codec implementiert, damit RemoteFX funktioniert. Hier werden anscheinend die grafischen Datenkanäle verwechselt - anstelle der Farbe „blau“ wird „rot“ geschrieben. Ein solcher Fehler ist höchstwahrscheinlich auf das Kopieren und Einfügen zurückzuführen.
Das gleiche Problem fiel in eine ähnliche Funktion
rfx_encode_format_argb , von der uns der Analysator auch erzählte:
V525 Der Code enthält die Sammlung ähnlicher Blöcke. Überprüfen Sie die Elemente 'a', 'r', 'g', 'r' in den Zeilen 260, 261, 262, 263. rfxencode_rgb_to_yuv.c 260
while (x < 64) { *la_buf++ = a; *lr_buf++ = r; *lg_buf++ = g; *lb_buf++ = r; x++; }
Array-Deklaration
V557 Array-Überlauf ist möglich. Der Wert des Index 'i - 8' könnte 129 erreichen. Genkeymap.c 142
Die Deklaration und Definition des Arrays in diesen beiden Dateien ist nicht kompatibel - die Größe unterscheidet sich um 1. Es treten jedoch keine Fehler auf - die richtige Größe wird in der Datei evdev-map.c angegeben, sodass kein Ausweg aus den Grenzen besteht. Dies ist also nur ein Fehler, der leicht zu beheben ist.
Ungültiger Vergleich
V560 Ein Teil des bedingten Ausdrucks ist immer falsch: (cap_len <0). xrdp_caps.c 616
In einer Funktion wird eine Variable vom Typ
unsigned short in eine Variable vom Typ
int eingelesen. Die Prüfung ist hier nicht erforderlich, da wir eine Variable eines vorzeichenlosen Typs lesen und das Ergebnis einer größeren Variablen zuweisen, sodass die Variable keinen negativen Wert annehmen kann.
Unnötige Kontrollen
V560 Ein Teil des bedingten Ausdrucks ist immer wahr: (bpp! = 16). libxrdp.c 704
int EXPORT_CC libxrdp_send_pointer(struct xrdp_session *session, int cache_idx, char *data, char *mask, int x, int y, int bpp) { .... if ((bpp == 15) && (bpp != 16) && (bpp != 24) && (bpp != 32)) { g_writeln("libxrdp_send_pointer: error"); return 1; } .... }
Ungleichheitsprüfungen sind hier nicht sinnvoll, da wir bereits zu Beginn einen Vergleich haben. Es ist wahrscheinlich, dass dies ein Tippfehler ist und der Entwickler das
|| verwenden wollte ungültige Argumente herauszufiltern.
Fazit
Während der Inspektion wurden keine schwerwiegenden Fehler festgestellt, es wurden jedoch viele Mängel festgestellt. Diese Projekte werden jedoch in vielen Systemen verwendet, wenn auch in geringem Umfang. Ein kleines Projekt muss nicht viele Fehler enthalten, daher sollten Sie die Arbeit des Analysators nicht nur an kleinen Projekten beurteilen. Mehr dazu lesen Sie im Artikel "
Empfindungen, die durch Zahlen bestätigt werden ."
Sie können eine Testversion von PVS-Studio auf unserer
Website herunterladen .
Wenn Sie diesen Artikel einem englischsprachigen Publikum zugänglich machen möchten, verwenden Sie bitte den Link zur Übersetzung: Sergey Larin.
Überprüfen von rdesktop und xrdp mit PVS-Studio