Toto je druhá recenze ze série článků o testování open source programů pro práci s protokolem RDP. V něm se podíváme na klienta rdesktop a server xrdp.
Používá se jako nástroj k identifikaci chyb Jedná se o statický analyzátor kódu pro C, C++, C# a Javu, dostupný na platformách Windows, Linux и macOS.
Článek uvádí pouze ty chyby, které se mi zdály zajímavé. Projekty jsou však malé, takže bylo málo chyb :).
Poznámka. Předchozí článek o ověřování projektu FreeRDP naleznete .
rddesktop
— bezplatná implementace RDP klienta pro systémy založené na UNIXu. Lze ji také použít pod Windows, pokud projekt sestavujete pod Cygwinem. Licencováno pod GPLv3.
Tento klient je velmi oblíbený – standardně se používá v ReactOS a můžete pro něj najít i grafické front-endy třetích stran. Je však poměrně starý: jeho první vydání proběhlo 4. dubna 2001 – v době psaní tohoto článku mu bylo 17 let.
Jak jsem již uvedl, projekt je poměrně malý. Obsahuje přibližně 30 tisíc řádků kódu, což je vzhledem k jeho stáří trochu zvláštní. Pro srovnání FreeRDP obsahuje 320 tisíc řádků. Zde je výstup z programu Cloc:
Nedosažitelný kód
Byl zjištěn nedostupný kód. Je možné, že došlo k chybě. 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žitě ve funkci hlavní: vidíme kód, který přichází za operátorem zpáteční — tento fragment provádí čištění paměti. Chyba však nepředstavuje hrozbu: veškerá přidělená paměť bude po ukončení programu operačním systémem vymazána.
Žádné zpracování chyb
Je možné podběhnutí pole. Hodnota indexu 'n' by mohla dosáhnout -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);
}
....
}Fragment kódu se v tomto případě čte ze souboru do vyrovnávací paměti, dokud soubor neskončí. Zde však nedochází k žádnému zpracování chyb: pokud se něco pokazí, pak číst vrátí -1 a pole bude přetečeno výstup.
Použití EOF v typu char
EOF by neměl být porovnáván s hodnotou typu 'char'. '(c = fgetc(fp))' by mělo být 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++;
}
....
}Zde vidíme nesprávné zacházení s dosažením konce souboru: if fgetc vrátí znak, jehož kód je 0xFF, bude interpretován jako konec souboru (EOF).
EOF je to konstanta, obvykle definovaná jako -1. Například v kódování CP1251 má poslední písmeno ruské abecedy kód 0xFF, což odpovídá číslu -1, pokud mluvíme o proměnné jako spálit. Ukazuje se, že symbol 0xFF, jako EOF (-1) se interpretuje jako konec souboru. Aby se předešlo takovým chybám, výsledek funkce je fgetc by měly být uloženy v proměnné jako int.
Překlepy
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žná se autor tohoto kódu spletl || и && ve stavu. Zvažme možné možnosti hodnot čas_zápisu и change_time:
- Obě proměnné se rovnají 0: v tomto případě skončíme ve větvi jiný: variabilní mod_time bude vždy 0 bez ohledu na následující podmínku.
- Jedna z proměnných je 0: mod_time se bude rovnat 0 (za předpokladu, že druhá proměnná má nezápornou hodnotu), protože MIN vybere menší ze dvou možností.
- Obě proměnné se nerovnají 0: zvolte minimální hodnotu.
Při výměně podmínky za write_time && change_time chování bude vypadat správně:
- Jedna nebo obě proměnné se nerovnají 0: zvolte nenulovou hodnotu.
- Obě proměnné se nerovnají 0: zvolte minimální hodnotu.
Fragment 2
Výraz je vždy pravdivý. Pravděpodobně by zde měl být použit 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;
....
}Operátoři se zde zřejmě také pomíchali || и &&Nebo == и !=: Proměnná nemůže mít současně hodnotu 20 a 9.
Neomezené kopírování řádků
Volání funkce 'sprintf' povede k přetečení 'plné cesty' bufferu. 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);
....
}Když se podíváte na funkci v plném rozsahu, bude jasné, že tento kód nezpůsobuje problémy. Mohou se však objevit v budoucnu: jedna neopatrná změna a přetečení zásobníku - sprint není ničím omezen, takže při zřetězení cest můžeme jít za hranice pole. Doporučuje se zaznamenat toto volání snprintf(celá cesta, PATH_MAX, ….).
Nadbytečný stav
Část podmíněné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)
{
....
}
}Проверка přidat > 0 zde není potřeba: proměnná bude vždy větší než nula, protože přečtěte si % 4 vrátí zbytek dělení, ale nikdy se nebude rovnat 4.
xrdp
— implementace serveru RDP s otevřeným zdrojovým kódem. Projekt je rozdělen na 2 části:
- xrdp - implementace protokolu. Distribuováno pod licencí Apache 2.0.
- xorgxrdp – Sada ovladačů Xorg pro použití s xrdp. Licence – X11 (jako MIT, ale zakazuje použití v reklamě)
Vývoj projektu je založen na výsledcích rdesktop a FreeRDP. Zpočátku jste pro práci s grafikou museli používat samostatný VNC server nebo speciální X11 server s podporou RDP - X11rdp, ale s příchodem xorgxrdp jejich potřeba zmizela.
V tomto článku se nebudeme zabývat xorgxrdp.
Projekt xrdp je stejně jako předchozí velmi malý a obsahuje přibližně 80 tisíc řádků.
Další překlepy
Kód obsahuje kolekci podobných bloků. Zkontrolujte položky 'r', 'g', 'r' v řádcích 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 byl převzat z knihovny librfxcodec, která implementuje kodek jpeg2000 pro RemoteFX. Zde se zdá, že grafické datové kanály jsou smíšené - místo „modré“ barvy se zaznamenává „červená“. Tato chyba se pravděpodobně objevila v důsledku kopírování a vkládání.
Stejný problém nastal v podobné funkci rfx_encode_format_argb, což nám analyzátor také řekl:
Kód obsahuje kolekci podobných bloků. Zkontrolujte položky 'a', 'r', 'g', 'r' v řádcích 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++;
}Prohlášení pole
Překročení pole je možné. Hodnota indexu 'i — 8' by mohla dosáhnout 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];
....
}
....
}Deklarace a definice pole v těchto dvou souborech jsou nekompatibilní - velikost se liší o 1. Nedochází však k žádným chybám - správná velikost je uvedena v souboru evdev-map.c, takže není mimo hranice. Jedná se tedy pouze o chybu, kterou lze snadno opravit.
Nesprávné srovnání
Část podmíněné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))
{
....
}
....
}Funkce čte proměnnou typu bez znaménka krátký do proměnné jako int. Kontrola zde není nutná, protože čteme proměnnou bez znaménka a výsledek přiřazujeme větší proměnné, takže proměnná nemůže nabývat záporné hodnoty.
Zbytečné kontroly
Část podmíněné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í zde nedávají smysl, protože srovnání již máme na začátku. Je pravděpodobné, že se jedná o překlep a vývojář chtěl využít operátora || k odfiltrování neplatných argumentů.
Závěr
Při kontrole nebyly zjištěny žádné závažné chyby, ale bylo zjištěno mnoho nedostatků. Tyto konstrukce se však používají v mnoha systémech, i když v malém rozsahu. Malý projekt nemusí mít nutně mnoho chyb, takže byste neměli posuzovat výkon analyzátoru pouze na malých projektech. Více si o tom můžete přečíst v článku „".
Můžete si od nás stáhnout zkušební verzi PVS-Studio .
Pokud chcete tento článek sdílet s anglicky mluvícím publikem, použijte odkaz na překlad: Sergey Larin.
Zdroj: www.habr.com
