Dëst ass déi zweet Iwwerpréiwung an enger Serie vun Artikelen iwwer Testen vun Open Source Programmer fir mam RDP Protokoll ze schaffen. An et wäerte mir den rdesktop Client an den xrdp Server kucken.
Benotzt als Tool fir Feeler z'identifizéieren . Et ass e statesche Code Analyser fir C, C++, C# a Java Sproochen, verfügbar op Windows, Linux a MacOS Plattformen.
Den Artikel presentéiert nëmmen déi Feeler, déi fir mech interessant ausgesinn. Allerdéngs sinn d'Projete kleng, sou datt et wéineg Feeler gouf :).
Remarque. E fréieren Artikel iwwer FreeRDP Projet Verifikatioun ka fonnt ginn .
rdesktop
- eng gratis Ëmsetzung vun engem RDP Client fir UNIX-baséiert Systemer. Et kann och ënner Windows benotzt ginn wann Dir de Projet ënner Cygwin baut. Lizenzéiert ënner GPLv3.
Dëse Client ass ganz populär - et gëtt als Standard an ReactOS benotzt, an Dir kënnt och Drëtt Partei grafesch Frontends dofir fannen. Wéi och ëmmer, hien ass zimlech al: seng éischt Verëffentlechung huet de 4. Abrëll 2001 stattfonnt - am Schreiwenzäit ass hien 17 Joer al.
Wéi ech virdru festgestallt hunn, ass de Projet relativ kleng. Et enthält ongeféier 30 Tausend Zeilen Code, wat e bësse komesch ass, wann Dir säin Alter berücksichtegt. Zum Verglach enthält FreeRDP 320 dausend Linnen. Hei ass d'Ausgab vum Cloc Programm:
Onerreechbar Code
Net verfügbare Code entdeckt. Et ass méiglech datt e Feeler präsent ass. 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);
}De Feeler trëfft eis direkt an der Funktioun Haaptstrooss: mir gesinn de Code kommen nom Bedreiwer zréck - dëst Fragment mécht Erënnerung Botzen. Wéi och ëmmer, de Feeler stellt keng Bedrohung aus: all zougewisen Erënnerung gëtt vum Betribssystem geläscht nodeems de Programm ofgeschloss ass.
Kee Feeler Ëmgank
Array Underrun ass méiglech. De Wäert vum 'n' Index kéint -1 erreechen. 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] = ' '; // <=
str_handle_lines(output, &rest, linehandler, data);
}
....
}De Code Snippet an dësem Fall liest aus der Datei an e Puffer bis d'Datei eriwwer ass. Et gëtt awer keng Fehlerhandhabung hei: wann eppes falsch leeft, dann gelies wäert zréck -1, an da gëtt d'Array iwwerrannt Ausgab.
Benotzt EOF am Char Typ
EOF soll net mat engem Wäert vum 'char' Typ verglach ginn. Den '(c = fgetc(fp))' soll vum 'int' Typ sinn. ctrl.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++;
}
....
}Hei gesi mir falsch Handhabung fir d'Enn vun der Datei z'erreechen: wann fgetc gëtt e Charakter zréck deem säi Code 0xFF ass, et gëtt als Enn vun der Datei interpretéiert (EOF).
EOF et ass eng konstant, normalerweis definéiert als -1. Zum Beispill, an der CP1251 Kodéierung, huet de leschte Buschtaf vum russesche Alphabet de Code 0xFF, deen der Nummer -1 entsprécht wa mir iwwer eng Variabel schwätzen wéi suz. Et stellt sech eraus datt d'Symbol 0xFF, wéi EOF (-1) gëtt als Enn vun der Datei interpretéiert. Fir esou Feeler ze vermeiden, ass d'Resultat vun der Funktioun fgetc soll an enger Variabel wéi gespäichert ginn INT.
Tippfehler
Fragment 1
Ausdrock 'write_time' ass ëmmer 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; // <=
....
}Vläicht huet den Auteur vun dësem Code et falsch || и && an Zoustand. Loosst eis méiglech Optiounen fir Wäerter betruechten schreiwen_Zäit и änneren_Zäit:
- Béid Variabelen sinn gläich 0: an dësem Fall komme mir an enger Branche aneren: Variabel mod_zeit wäert ëmmer 0 sinn onofhängeg vun der spéider Konditioun.
- Eng vun de Variabelen ass 0: mod_zeit wäert gläich 0 sinn (virausgesat datt déi aner Variabel en net-negative Wäert huet), well MIN wäert déi klengst vun deenen zwou Optiounen wielen.
- Béid Variabelen sinn net gläich 0: wielt de Minimum Wäert.
Wann d'Konditioun ersat mat schreiwen_Zäit && änneren_Zäit d'Verhalen wäert richteg ausgesinn:
- Eng oder béid Variabelen sinn net gläich wéi 0: Wielt en Net-Nullwäert.
- Béid Variabelen sinn net gläich 0: wielt de Minimum Wäert.
Fragment 2
Ausdrock ass ëmmer richteg. Wahrscheinlech soll den '&&' Bedreiwer hei benotzt ginn. 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 sinn d'Bedreiwer och hei vermëscht || и &&, oder == и !=: Eng Variabel kann net de Wäert 20 an 9 zur selwechter Zäit hunn.
Onlimitéiert Linn Kopie
En Uruff vun der 'sprintf' Funktioun féiert zum Iwwerfluss vum Puffer 'fullpath'. disk.c 1257
RD_NTSTATUS
disk_query_directory(....)
{
....
char *dirname, fullpath[PATH_MAX];
....
/* Get information for directory entry */
sprintf(fullpath, "%s/%s", dirname, pdirent->d_name);
....
}Wann Dir d'Funktioun voll kuckt, gëtt et kloer datt dëse Code keng Probleemer verursaacht. Wéi och ëmmer, se kënnen an Zukunft entstoen: eng suergfälteg Ännerung a mir kréien e Pufferiwwerfluss - sprint ass net vun näischt limitéiert, also wa mir d'Weeër verbannen, kënne mir iwwer d'Grenze vum Array goen. Et ass recommandéiert dësen Uruff unzepassen snprintf(fullpath, PATH_MAX, ….).
Redundant Zoustand
En Deel vum bedingten Ausdrock ass ëmmer wouer: add > 0. scard.c 507
static void
inRepos(STREAM in, unsigned int read)
{
SERVER_DWORD add = 4 - read % 4;
if (add < 4 && add > 0)
{
....
}
}Inspektioun derbäi > 0 et ass net néideg hei: d'Variabel wäert ëmmer méi grouss wéi null sinn, well liesen % 4 wäert de Rescht vun der Divisioun zréckginn, awer et wäert ni gläich wéi 4 sinn.
xrdp
- Ëmsetzung vun engem RDP Server mat Open Source Code. De Projet ass an 2 Deeler opgedeelt:
- xrdp - Protokoll Ëmsetzung. Verdeelt ënner der Apache 2.0 Lizenz.
- xorgxrdp - Eng Rei vun Xorg Treiber fir ze benotzen mat xrdp. Lizenz - X11 (wéi MIT, awer verbitt d'Benotzung an der Reklamm)
D'Entwécklung vum Projet baséiert op de Resultater vun rdesktop a FreeRDP. Am Ufank, fir mat Grafiken ze schaffen, musst Dir e separaten VNC-Server benotzen, oder e speziellen X11-Server mat RDP-Ënnerstëtzung - X11rdp, awer mat dem Advent vun xorgxrdp ass de Besoin fir si verschwonnen.
An dësem Artikel wäerte mir xorgxrdp net ofdecken.
Den xrdp Projet, wéi dee virdrun, ass ganz kleng an enthält ongeféier 80 dausend Linnen.
Méi Tippfehler
De Code enthält d'Sammlung vun ähnlechen Blocken. Kuckt d'Elementer 'r', 'g', 'r' an de Linnen 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; // <=
x++;
}
....
}
....
}Dëse Code gouf aus der librfxcodec Bibliothéik geholl, déi den jpeg2000 Codec fir RemoteFX implementéiert. Hei sinn anscheinend déi grafesch Datekanäl gemëscht - amplaz vun der "bloer" Faarf gëtt "rout" opgeholl. Dëse Feeler erschéngt héchstwahrscheinlech als Resultat vu Copy-Paste.
Dee selwechte Problem ass an enger ähnlecher Funktioun geschitt rfx_encode_format_argb, wat den Analyser eis och gesot huet:
De Code enthält d'Sammlung vun ähnlechen Blocken. Iwwerpréift Elementer 'a', 'r', 'g', 'r' an de Linnen 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 Deklaratioun
Array Iwwerschwemmung ass méiglech. De Wäert vum 'i — 8' Index kéint 129 erreechen. genkeymap.c 142
// evdev-map.c
int xfree86_to_evdev[137-8+1] = {
....
};
// genkeymap.c
extern int xfree86_to_evdev[137-8];
int main(int argc, char **argv)
{
....
for (i = 8; i <= 137; i++) /* Keycodes */
{
if (is_evdev)
e.keycode = xfree86_to_evdev[i-8];
....
}
....
}D'Deklaratioun an d'Definitioun vun der Array an dësen zwee Dateien sinn inkompatibel - d'Gréisst ënnerscheet sech ëm 1. Allerdéngs ginn et keng Feeler - déi richteg Gréisst gëtt an der evdev-map.c Datei spezifizéiert, sou datt et keen Out of bounds gëtt. Also dëst ass just e Feeler deen einfach fixéiert ka ginn.
Falsche Verglach
En Deel vum bedingten Ausdrock ass ëmmer falsch: (cap_len < 0). xrdp_caps.c 616
// common/parse.h
#if defined(B_ENDIAN) || defined(NEED_ALIGN)
#define in_uint16_le(s, v) do
....
#else
#define in_uint16_le(s, v) do
{
(v) = *((unsigned short*)((s)->p));
(s)->p += 2;
} while (0)
#endif
int
xrdp_caps_process_confirm_active(struct xrdp_rdp *self, struct stream *s)
{
int cap_len;
....
in_uint16_le(s, cap_len);
....
if ((cap_len < 0) || (cap_len > 1024 * 1024))
{
....
}
....
}D'Funktioun liest eng Zort Variabel net ënnerschriwwen kuerz an eng Variabel wéi INT. Iwwerpréift ass hei net néideg, well mir eng net ënnerschriwwe Variabel liesen an d'Resultat un eng méi grouss Variabel zouginn, sou datt d'Variabel net en negativen Wäert huelen kann.
Onnéideg Kontrollen
En Deel vum bedingten Ausdrock ass ëmmer wouer: (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;
}
....
}D'Ongläichheetskontrolle maachen hei kee Sënn, well mir am Ufank schonn e Verglach hunn. Et ass méiglech datt dëst en Tippfeeler ass an den Entwéckler wollt den Bedreiwer benotzen || ongëlteg Argumenter ze filteren.
Konklusioun
Beim Audit goufe keng sérieux Feeler identifizéiert, awer vill Mängel fonnt. Wéi och ëmmer, dës Designe ginn a ville Systemer benotzt, och wann et kleng ass. E klenge Projet huet net onbedéngt vill Feeler, also sollt Dir d'Leeschtung vum Analysator net nëmme bei klenge Projete beurteelen. Dir kënnt méi iwwer dëst am Artikel liesen "".
Dir kënnt eng Testversioun vum PVS-Studio vun eis eroflueden .
Wann Dir dësen Artikel mat engem engleschsproochege Publikum wëllt deelen, benotzt w.e.g. den Iwwersetzungslink: Sergey Larin.
Source: will.com