Ky është rishikimi i dytë në një seri artikujsh në lidhje me testimin e programeve me burim të hapur për të punuar me protokollin RDP. Në të do të shikojmë klientin rdesktop dhe serverin xrdp.
Përdoret si një mjet për të identifikuar gabimet . Është një analizues i kodit statik për gjuhët C, C++, C# dhe Java, i disponueshëm në platformat Windows, Linux dhe macOS.
Artikulli paraqet vetëm ato gabime që më dukeshin interesante. Megjithatë, projektet janë të vogla, kështu që ka pasur pak gabime :).
Shënim. Mund të gjendet një artikull i mëparshëm në lidhje me verifikimin e projektit FreeRDP .
desktop
— një implementim falas i një klienti RDP për sistemet e bazuara në UNIX. Mund të përdoret gjithashtu nën Windows nëse e ndërtoni projektin nën Cygwin. Licencuar sipas GPLv3.
Ky klient është shumë i popullarizuar - përdoret si parazgjedhje në ReactOS, dhe gjithashtu mund të gjeni pjesë të përparme grafike të palëve të treta për të. Sidoqoftë, ai është mjaft i vjetër: lirimi i tij i parë u bë më 4 prill 2001 - në kohën e shkrimit, ai është 17 vjeç.
Siç e përmenda më herët, projekti është shumë i vogël. Ai përmban afërsisht 30 mijë rreshta kodi, që është paksa e çuditshme duke marrë parasysh moshën e tij. Për krahasim, FreeRDP përmban 320 mijë rreshta. Këtu është rezultati i programit Cloc:
Kodi i paarritshëm
U zbulua kod i padisponueshëm. Është e mundur që të ketë një gabim. 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);
}Gabimi na has menjëherë në funksion kryesor: shohim kodin që vjen pas operatorit kthim — ky fragment kryen pastrimin e memories. Sidoqoftë, gabimi nuk përbën një kërcënim: e gjithë memoria e ndarë do të pastrohet nga sistemi operativ pas daljes së programit.
Asnjë trajtim i gabimeve
Mbyllja e grupit është e mundur. Vlera e indeksit 'n' mund të arrijë -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);
}
....
}Pjesa e kodit në këtë rast lexohet nga skedari në një buffer derisa skedari të përfundojë. Sidoqoftë, këtu nuk ka asnjë trajtim të gabimeve: nëse diçka shkon keq, atëherë lexoj do të kthehet -1, dhe më pas vargu do të tejkalohet prodhim.
Përdorimi i EOF në llojin char
EOF nuk duhet të krahasohet me një vlerë të tipit 'char'. '(c = fgetc(fp))' duhet të jetë e tipit '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++;
}
....
}Këtu shohim trajtimin e gabuar të arritjes në fund të skedarit: nëse fgetc kthen një karakter kodi i të cilit është 0xFF, ai do të interpretohet si fundi i skedarit (Eof).
Eof është një konstante, zakonisht e përcaktuar si -1. Për shembull, në kodimin CP1251, shkronja e fundit e alfabetit rus ka kodin 0xFF, i cili korrespondon me numrin -1 nëse po flasim për një ndryshore si p.sh. shkrumb. Rezulton se simboli 0xFF, si Eof (-1) interpretohet si fundi i skedarit. Për të shmangur gabime të tilla, rezultati i funksionit është fgetc duhet të ruhet në një variabël si int.
Gabimet e gabimit
Fragmenti 1
Shprehja 'write_time' është gjithmonë false. 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; // <=
....
}Ndoshta autori i këtij kodi e ka gabuar || и && ne gjendje. Le të shqyrtojmë opsionet e mundshme për vlerat koha_shkrimi и koha_ndryshimit:
- Të dy variablat janë të barabartë me 0: në këtë rast do të përfundojmë në një degë tjetër: variabël mod_koha do të jetë gjithmonë 0 pavarësisht nga kushti pasues.
- Një nga variablat është 0: mod_koha do të jetë e barabartë me 0 (me kusht që ndryshorja tjetër të ketë një vlerë jo negative), sepse MIN do të zgjedhë më të voglin nga dy opsionet.
- Të dy variablat nuk janë të barabartë me 0: zgjidhni vlerën minimale.
Kur e zëvendësoni gjendjen me koha_shkrimi dhe koha_ndryshimit sjellja do të duket e saktë:
- Njëra ose të dyja variablat nuk janë të barabarta me 0: zgjidhni një vlerë jo zero.
- Të dy variablat nuk janë të barabartë me 0: zgjidhni vlerën minimale.
Fragmenti 2
Shprehja është gjithmonë e vërtetë. Ndoshta operatori '&&' duhet të përdoret këtu. 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;
....
}Me sa duket edhe këtu operatorët janë të përzier || и &&Ose == и !=: Një ndryshore nuk mund të ketë vlerën 20 dhe 9 në të njëjtën kohë.
Kopjimi i pakufizuar i linjës
Një thirrje e funksionit 'sprintf' do të çojë në tejmbushje të bufferit '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);
....
}Kur shikoni funksionin plotësisht, do të bëhet e qartë se ky kod nuk shkakton probleme. Sidoqoftë, ato mund të lindin në të ardhmen: një ndryshim i pakujdesshëm dhe do të kemi një tejmbushje tampon - sprint nuk kufizohet nga asgjë, kështu që kur lidhim shtigje mund të shkojmë përtej kufijve të grupit. Rekomandohet të vini re këtë thirrje në snprintf (shtegu i plotë, PATH_MAX, ....).
Gjendje e tepërt
Një pjesë e shprehjes së kushtëzuar është gjithmonë e vërtetë: shtoni > 0. scard.c 507
static void
inRepos(STREAM in, unsigned int read)
{
SERVER_DWORD add = 4 - read % 4;
if (add < 4 && add > 0)
{
....
}
}Проверка shtoni > 0 këtu nuk ka nevojë: ndryshorja do të jetë gjithmonë më e madhe se zero, sepse lexo % 4 do të kthejë pjesën e mbetur të pjesëtimit, por nuk do të jetë kurrë e barabartë me 4.
xrdp
— implementimi i një serveri RDP me kod me burim të hapur. Projekti është i ndarë në 2 pjesë:
- xrdp - zbatimi i protokollit. Shpërndarë nën licencën Apache 2.0.
- xorgxrdp - Një grup drejtuesish Xorg për përdorim me xrdp. Licenca - X11 (si MIT, por ndalon përdorimin në reklama)
Zhvillimi i projektit bazohet në rezultatet e rdesktop dhe FreeRDP. Fillimisht, për të punuar me grafikë, ju duhej të përdorni një server të veçantë VNC, ose një server të veçantë X11 me mbështetje RDP - X11rdp, por me ardhjen e xorgxrdp, nevoja për to u zhduk.
Në këtë artikull ne nuk do të mbulojmë xorgxrdp.
Projekti xrdp, si ai i mëparshmi, është shumë i vogël dhe përmban afërsisht 80 mijë rreshta.
Më shumë gabime shtypi
Kodi përmban koleksionin e blloqeve të ngjashme. Kontrolloni artikujt 'r', 'g', 'r' në rreshtat 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++;
}
....
}
....
}Ky kod është marrë nga biblioteka librfxcodec, e cila zbaton kodekun jpeg2000 për RemoteFX. Këtu, me sa duket, kanalet grafike të të dhënave janë të përziera - në vend të ngjyrës "blu", regjistrohet "e kuqe". Ky gabim ka shumë të ngjarë të jetë shfaqur si rezultat i kopjimit-ngjitjes.
I njëjti problem ndodhi në një funksion të ngjashëm rfx_encode_format_argb, për të cilën analizuesi na tha gjithashtu:
Kodi përmban koleksionin e blloqeve të ngjashme. Kontrolloni artikujt 'a', 'r', 'g', 'r' në rreshtat 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++;
}Deklarata e vargut
Kapërcimi i grupit është i mundur. Vlera e indeksit 'i — 8' mund të arrijë në 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];
....
}
....
}Deklarimi dhe përkufizimi i grupit në këto dy skedarë janë të papajtueshëm - madhësia ndryshon me 1. Megjithatë, nuk ndodhin gabime - madhësia e saktë është specifikuar në skedarin evdev-map.c, kështu që nuk ka jashtë kufijve. Pra, ky është vetëm një gabim që mund të rregullohet lehtësisht.
Krahasim i gabuar
Një pjesë e shprehjes së kushtëzuar është gjithmonë false: (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))
{
....
}
....
}Funksioni lexon një variabël tipi e nënshkruar e shkurtër në një variabël si int. Kontrolli nuk është i nevojshëm këtu sepse ne po lexojmë një ndryshore të panënshkruar dhe po ia caktojmë rezultatin një ndryshoreje më të madhe, kështu që ndryshorja nuk mund të marrë një vlerë negative.
Kontrolle të panevojshme
Një pjesë e shprehjes kushtore është gjithmonë e vërtetë: (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;
}
....
}Kontrollet e pabarazisë nuk kanë kuptim këtu pasi ne kemi tashmë një krahasim në fillim. Ka të ngjarë që ky është një gabim shtypi dhe zhvilluesi ka dashur të përdorë operatorin || për të filtruar argumentet e pavlefshme.
Përfundim
Gjatë auditimit nuk janë konstatuar gabime serioze, por janë konstatuar shumë mangësi. Megjithatë, këto dizajne përdoren në shumë sisteme, megjithëse të vogla në shtrirje. Një projekt i vogël nuk ka domosdoshmërisht shumë gabime, kështu që nuk duhet të gjykoni performancën e analizuesit vetëm në projekte të vogla. Ju mund të lexoni më shumë rreth kësaj në artikullin "".
Ju mund të shkarkoni një version provë të PVS-Studio nga ne .
Nëse dëshironi ta ndani këtë artikull me një audiencë anglishtfolëse, ju lutemi përdorni lidhjen e përkthimit: Sergey Larin.
Burimi: www.habr.com