Toto je druhá recenzia zo série článkov o testovaní open source programov pre prácu s protokolom RDP. V ňom sa pozrieme na klienta rdesktop a server xrdp.
Používa sa ako nástroj na identifikáciu chýb . Je to statický analyzátor kódu pre jazyky C, C++, C# a Java, dostupný na platformách Windows, Linux a macOS.
Článok uvádza len tie chyby, ktoré sa mi zdali zaujímavé. Projekty sú však malé, takže chýb bolo málo :).
Poznámka. Môžete nájsť predchádzajúci článok o overení projektu FreeRDP .
rddesktop
— bezplatná implementácia klienta RDP pre systémy založené na systéme UNIX. Dá sa použiť aj pod Windows, ak projekt postavíte pod Cygwin. Licencované pod GPLv3.
Tento klient je veľmi populárny – štandardne sa používa v ReactOS a nájdete k nemu aj grafické front-endy tretích strán. Je však dosť starý: jeho prvé vydanie sa uskutočnilo 4. apríla 2001 - v čase písania tohto článku má 17 rokov.
Ako som už uviedol, projekt je veľmi malý. Obsahuje približne 30 tisíc riadkov kódu, čo je vzhľadom na jeho vek trochu zvláštne. Pre porovnanie FreeRDP obsahuje 320 tisíc riadkov. Tu je výstup z programu Cloc:
Nedostupný kód
Zistil sa nedostupný kód. Je možné, že sa vyskytla chyba. 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);
}Chyba na nás narazí okamžite vo funkcii hlavné: vidíme, že kód prichádza za operátorom návrat — tento fragment vykonáva čistenie pamäte. Chyba však nepredstavuje hrozbu: po ukončení programu operačný systém vymaže všetku pridelenú pamäť.
Žiadne spracovanie chýb
Je možné podbehnutie poľa. Hodnota 'n' indexu môže dosiahnuť -1. 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);
}
....
}Útržok kódu sa v tomto prípade číta zo súboru do vyrovnávacej pamäte, kým sa súbor neskončí. Nedochádza tu však k žiadnej chybe: ak sa niečo pokazí, potom čítať vráti -1 a pole bude pretečené výkon.
Použitie EOF v type znaku
EOF by sa nemalo porovnávať s hodnotou typu „char“. '(c = fgetc(fp))' by malo byť typu 'int'. 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++;
}
....
}Tu vidíme nesprávne zaobchádzanie s dosiahnutím konca súboru: if fgetc vráti znak, ktorého kód je 0xFF, bude interpretovaný ako koniec súboru (EOF).
EOF je to konštanta, zvyčajne definovaná ako -1. Napríklad v kódovaní CP1251 má posledné písmeno ruskej abecedy kód 0xFF, čo zodpovedá číslu -1, ak hovoríme o premennej ako spáliť. Ukazuje sa, že symbol 0xFF, ako EOF (-1) sa interpretuje ako koniec súboru. Aby sa predišlo takýmto chybám, výsledkom funkcie je fgetc by mali byť uložené v premennej ako int.
Preklepy
Fragment 1
Výraz 'write_time' je vždy nepravdivý. 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; // <=
....
}Možno sa autor tohto kódu pomýlil || и && v stave. Zvážme možné možnosti hodnôt write_time и change_time:
- Obe premenné sa rovnajú 0: v tomto prípade skončíme vo vetve inak: variabilný mod_time bude vždy 0 bez ohľadu na nasledujúcu podmienku.
- Jedna z premenných je 0: mod_time sa bude rovnať 0 (za predpokladu, že druhá premenná má nezápornú hodnotu), pretože MIN vyberie menšiu z dvoch možností.
- Obe premenné sa nerovnajú 0: vyberte minimálnu hodnotu.
Pri výmene podmienky s write_time && change_time správanie bude vyzerať správne:
- Jedna alebo obe premenné sa nerovnajú 0: vyberte nenulovú hodnotu.
- Obe premenné sa nerovnajú 0: vyberte minimálnu hodnotu.
Fragment 2
Výraz je vždy pravdivý. Pravdepodobne by sa tu mal použiť operátor '&&'. 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;
....
}Zrejme sa tu pomiešali aj operátori || и &&Alebo == и !=: Premenná nemôže mať súčasne hodnotu 20 a 9.
Neobmedzené kopírovanie riadkov
Volanie funkcie 'sprintf' povedie k pretečeniu 'plnej cesty' vyrovnávacej pamäte. 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);
....
}Keď sa pozriete na funkciu v plnom rozsahu, bude jasné, že tento kód nespôsobuje problémy. Môžu sa však objaviť v budúcnosti: jedna neopatrná zmena a dostaneme pretečenie vyrovnávacej pamäte - šprint nie je ničím obmedzený, takže pri spájaní ciest môžeme ísť za hranice poľa. Odporúča sa spozorovať toto volanie snprintf(celá cesta, PATH_MAX, ….).
Nadbytočný stav
Časť podmieneného výrazu je vždy pravdivá: add > 0. scard.c 507
static void
inRepos(STREAM in, unsigned int read)
{
SERVER_DWORD add = 4 - read % 4;
if (add < 4 && add > 0)
{
....
}
}Проверка pridať > 0 tu nie je potrebné: premenná bude vždy väčšia ako nula, pretože prečítajte si % 4 vráti zvyšok delenia, ale nikdy sa nebude rovnať 4.
XRDP
— implementácia servera RDP s otvoreným zdrojovým kódom. Projekt je rozdelený na 2 časti:
- xrdp - implementácia protokolu. Distribuované pod licenciou Apache 2.0.
- xorgxrdp – Sada ovládačov Xorg na použitie s xrdp. Licencia - X11 (ako MIT, ale zakazuje použitie v reklame)
Vývoj projektu je založený na výsledkoch rdesktop a FreeRDP. Spočiatku ste na prácu s grafikou museli použiť samostatný server VNC alebo špeciálny server X11 s podporou RDP - X11rdp, ale s príchodom xorgxrdp ich potreba zmizla.
V tomto článku sa nebudeme zaoberať xorgxrdp.
Projekt xrdp, rovnako ako predchádzajúci, je veľmi malý a obsahuje približne 80 tisíc riadkov.
Viac preklepov
Kód obsahuje kolekciu podobných blokov. Skontrolujte položky „r“, „g“, „r“ v riadkoch 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++;
}
....
}
....
}Tento kód bol prevzatý z knižnice librfxcodec, ktorá implementuje kodek jpeg2000 pre RemoteFX. Tu sú zjavne zmiešané grafické dátové kanály - namiesto „modrej“ farby sa zaznamenáva „červená“. Táto chyba sa s najväčšou pravdepodobnosťou objavila v dôsledku kopírovania a vkladania.
Rovnaký problém sa vyskytol v podobnej funkcii rfx_encode_format_argb, čo nám analyzátor tiež povedal:
Kód obsahuje kolekciu podobných blokov. Skontrolujte položky „a“, „r“, „g“, „r“ v riadkoch 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++;
}Vyhlásenie poľa
Prekročenie poľa je možné. Hodnota indexu 'i — 8' mohla dosiahnuť 129. 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];
....
}
....
}Deklarácia a definícia poľa v týchto dvoch súboroch sú nekompatibilné - veľkosť sa líši o 1. Nedochádza však k žiadnym chybám - správna veľkosť je uvedená v súbore evdev-map.c, takže nie je prekročenie hraníc. Ide teda len o chybu, ktorá sa dá ľahko opraviť.
Nesprávne porovnanie
Časť podmieneného výrazu je vždy nepravdivá: (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))
{
....
}
....
}Funkcia číta typovú premennú nepodpísané krátke do premennej ako int. Kontrola tu nie je potrebná, pretože čítame premennú bez znamienka a výsledok priraďujeme k väčšej premennej, takže premenná nemôže mať zápornú hodnotu.
Zbytočné kontroly
Časť podmieneného výrazu je vždy pravdivá: (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;
}
....
}Kontroly nerovností tu nemajú zmysel, keďže porovnanie už máme na začiatku. Je pravdepodobné, že ide o preklep a vývojár chcel využiť operátora || na odfiltrovanie neplatných argumentov.
Záver
Počas auditu neboli zistené žiadne závažné chyby, ale bolo zistených veľa nedostatkov. Tieto konštrukcie sa však používajú v mnohých systémoch, aj keď v malom rozsahu. Malý projekt nemusí mať nevyhnutne veľa chýb, preto by ste výkon analyzátora nemali posudzovať len na malých projektoch. Viac si o tom môžete prečítať v článku „".
Môžete si od nás stiahnuť skúšobnú verziu PVS-Studio .
Ak chcete tento článok zdieľať s anglicky hovoriacim publikom, použite odkaz na preklad: Sergey Larin.
Zdroj: hab.com