Ovo je drugi pregled u nizu članaka o testiranju programa otvorenog koda za rad sa RDP protokolom. U njemu ćemo pogledati rdesktop klijent i xrdp server.
Koristi se kao alat za identifikaciju grešaka . To je statički analizator koda za C, C++, C# i Java jezike, dostupan na Windows, Linux i macOS platformama.
U članku su prikazane samo one greške koje su mi se učinile interesantnim. Međutim, projekti su mali, tako da je bilo malo grešaka :).
primjedba. Prethodni članak o verifikaciji FreeRDP projekta možete pronaći .
rdesktop
— besplatna implementacija RDP klijenta za UNIX-bazirane sisteme. Takođe se može koristiti pod Windows-om ako gradite projekat pod Cygwin-om. Licencirano pod GPLv3.
Ovaj klijent je veoma popularan - koristi se podrazumevano u ReactOS-u, a za njega možete pronaći i grafičke front-endove treće strane. Međutim, on je prilično star: njegovo prvo izdanje održano je 4. aprila 2001. - u vrijeme pisanja, ima 17 godina.
Kao što sam ranije napomenuo, projekat je prilično mali. Sadrži otprilike 30 hiljada linija koda, što je malo čudno s obzirom na njegovu starost. Poređenja radi, FreeRDP sadrži 320 hiljada linija. Evo izlaza Cloc programa:
Nedostupan kod
Otkriven je nedostupan kod. Moguće je da postoji greška. 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);
}Greška nas nailazi odmah u funkciji glavni: vidimo kod koji dolazi iza operatora povratak — ovaj fragment vrši čišćenje memorije. Međutim, greška ne predstavlja prijetnju: operativni sistem će obrisati svu dodijeljenu memoriju nakon što program izađe.
Nema rukovanja greškama
Moguće je potkopavanje niza. Vrijednost 'n' indeksa može doseći -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);
}
....
}Isječak koda u ovom slučaju čita iz datoteke u međuspremnik dok se datoteka ne završi. Međutim, ovdje nema rukovanja greškama: ako nešto krene po zlu, onda čitati će vratiti -1, a zatim će niz biti pregažen Izlaz.
Korištenje EOF u tipu char
EOF ne treba porediti sa vrednošću tipa 'char'. '(c = fgetc(fp))' bi trebao biti tipa '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++;
}
....
}Ovdje vidimo pogrešno rukovanje dolaskom do kraja datoteke: if fgetc vraća znak čiji je kod 0xFF, on će se tumačiti kao kraj datoteke (EOF).
EOF to je konstanta, obično definisana kao -1. Na primjer, u kodiranju CP1251 posljednje slovo ruske abecede ima kod 0xFF, što odgovara broju -1 ako govorimo o varijabli kao što je znakova. Ispostavilo se da je simbol 0xFF, kao EOF (-1) se tumači kao kraj datoteke. Da bi se izbjegle takve greške, rezultat funkcije je fgetc treba pohraniti u varijablu kao što je Int.
Greške
Fragment 1
Izraz 'write_time' je uvijek lažan. 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žda je autor ovog koda pogriješio || и && u stanju. Razmotrimo moguće opcije za vrijednosti write_time и change_time:
- Obje varijable su jednake 0: u ovom slučaju ćemo završiti u grani drugo: varijabilna mod_time uvijek će biti 0 bez obzira na sljedeći uvjet.
- Jedna od varijabli je 0: mod_time će biti jednako 0 (pod uslovom da druga varijabla ima nenegativnu vrijednost), jer MIN će izabrati manju od dvije opcije.
- Obje varijable nisu jednake 0: odaberite minimalnu vrijednost.
Prilikom zamjene stanja sa vrijeme_pisanja && vrijeme_promjene ponašanje će izgledati ispravno:
- Jedna ili obje varijable nisu jednake 0: odaberite vrijednost koja nije nula.
- Obje varijable nisu jednake 0: odaberite minimalnu vrijednost.
Fragment 2
Izraz je uvek istinit. Vjerovatno ovdje treba koristiti operator '&&'. 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;
....
}Očigledno su i ovdje operateri pomiješani || и &&, ili == и !=: Varijabla ne može imati vrijednost 20 i 9 u isto vrijeme.
Neograničeno kopiranje linija
Poziv funkcije 'sprintf' će dovesti do prelijevanja 'fullpath' bafera. 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);
....
}Kada pogledate funkciju u cijelosti, bit će vam jasno da ovaj kod ne uzrokuje probleme. Međutim, oni se mogu pojaviti u budućnosti: jedna neoprezna promjena i dobićemo prelivanje bafera - sprint nije ograničen ničim, tako da kada spajamo staze možemo ići izvan granica niza. Preporučljivo je primijetiti ovaj poziv snprintf(puna putanja, PATH_MAX, ….).
Redundantno stanje
Deo uslovnog izraza je uvek tačan: dodaj > 0. scard.c 507
static void
inRepos(STREAM in, unsigned int read)
{
SERVER_DWORD add = 4 - read % 4;
if (add < 4 && add > 0)
{
....
}
}inspekcija dodaj > 0 tu nema potrebe: varijabla će uvijek biti veća od nule, jer pročitaj % 4 će vratiti ostatak dijeljenja, ali nikada neće biti jednak 4.
xrdp
— implementacija RDP servera sa otvorenim izvornim kodom. Projekat je podijeljen u 2 dijela:
- xrdp - implementacija protokola. Distribuirano pod licencom Apache 2.0.
- xorgxrdp - Skup Xorg drajvera za upotrebu sa xrdp-om. Licenca - X11 (kao MIT, ali zabranjuje korištenje u oglašavanju)
Razvoj projekta je baziran na rezultatima rdesktop-a i FreeRDP-a. U početku, za rad sa grafikom, morali ste koristiti poseban VNC server ili poseban X11 server sa RDP podrškom - X11rdp, ali s pojavom xorgxrdp, potreba za njima je nestala.
U ovom članku nećemo pokrivati xorgxrdp.
Projekat xrdp, kao i prethodni, vrlo je mali i sadrži oko 80 hiljada linija.
Više grešaka u kucanju
Kod sadrži kolekciju sličnih blokova. Označite stavke 'r', 'g', 'r' u redovima 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++;
}
....
}
....
}Ovaj kod je preuzet iz biblioteke librfxcodec, koja implementira jpeg2000 kodek za RemoteFX. Ovdje su, očigledno, pomiješani kanali grafičkih podataka - umjesto "plave" boje, snima se "crvena". Ova greška se najvjerovatnije pojavila kao rezultat copy-paste-a.
Isti problem se pojavio u sličnoj funkciji rfx_encode_format_argb, što nam je analizator takođe rekao:
Kod sadrži kolekciju sličnih blokova. Označite stavke 'a', 'r', 'g', 'r' u redovima 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++;
}Deklaracija niza
Moguće je prekoračenje niza. Vrijednost indeksa 'i — 8' mogla bi dostići 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];
....
}
....
}Deklaracija i definicija niza u ova dva fajla su nekompatibilne - veličina se razlikuje za 1. Međutim, nema grešaka - tačna veličina je navedena u datoteci evdev-map.c, tako da nema van granica. Dakle, ovo je samo greška koja se lako može popraviti.
Netačno poređenje
Dio uslovnog izraza je uvijek lažan: (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))
{
....
}
....
}Funkcija čita varijablu tipa nepotpisano kratko u varijablu poput Int. Provjera ovdje nije potrebna jer čitamo neoznačenu varijablu i dodjeljujemo rezultat većoj varijabli, tako da varijabla ne može uzeti negativnu vrijednost.
Nepotrebne provjere
Deo uslovnog izraza je uvek tačan: (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;
}
....
}Provjere nejednakosti ovdje nemaju smisla jer već imamo poređenje na početku. Vjerovatno je ovo greška u kucanju i programer je želio koristiti operatera || za filtriranje nevažećih argumenata.
zaključak
Tokom revizije nisu identifikovane ozbiljne greške, ali su utvrđeni mnogi nedostaci. Međutim, ovi dizajni se koriste u mnogim sistemima, iako malog obima. Mali projekat ne mora nužno imati mnogo grešaka, tako da ne biste trebali suditi o performansama analizatora samo na malim projektima. Više o tome možete pročitati u članku “".
Možete preuzeti probnu verziju PVS-Studio kod nas .
Ako želite da podijelite ovaj članak sa publikom koja govori engleski, koristite link za prijevod: Sergey Larin.
izvor: www.habr.com