
Din hija t-tieni reviżjoni f'serje ta 'artikoli dwar l-ittestjar ta' programmi ta 'sors miftuħ biex jaħdmu mal-protokoll RDP. Fiha se nħarsu lejn il-klijent rdesktop u s-server xrdp.
Użat bħala għodda biex jiġu identifikati l-iżbalji Huwa analizzatur tal-kodiċi statiku għal C, C++, C# u Java, disponibbli fuq pjattaformi Windows, Linux и macOS.
L-artiklu jippreżenta biss dawk l-iżbalji li dehru interessanti għalija. Madankollu, il-proġetti huma żgħar, għalhekk kien hemm ftit żbalji :).
Innota. Jista 'jinstab artikolu preċedenti dwar il-verifika tal-proġett FreeRDP .
rdesktop
— свободная реализация клиента RDP для UNIX-based систем. Его также можно использовать и под Windows, если собирать проект под Cygwin. Лицензирован под GPLv3.
Dan il-klijent huwa popolari ħafna - huwa użat awtomatikament f'ReactOS, u tista 'ssib ukoll front-ends grafiċi ta' partijiet terzi għalih. Madankollu, huwa pjuttost antik: l-ewwel rilaxx tiegħu seħħ fl-4 ta 'April, 2001 - fil-ħin tal-kitba, huwa għandu 17-il sena.
Kif innutajt qabel, il-proġett huwa pjuttost żgħir. Fiha madwar 30 elf linja ta 'kodiċi, li hija daqsxejn stramba meta wieħed iqis l-età tiegħu. Għal tqabbil, FreeRDP fih 320 elf linja. Hawn hu l-output tal-programm Cloc:

Kodiċi li ma jintlaħaqx
Kodiċi mhux disponibbli misjuba. Huwa possibbli li jkun hemm żball. 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);
}L-iżball jiltaqa' magħna immedjatament fil-funzjoni prinċipali: naraw il-kodiċi ġej wara l-operatur ritorn — dan il-framment iwettaq tindif tal-memorja. Madankollu, l-iżball ma joħloqx theddida: il-memorja kollha allokata se titneħħa mis-sistema operattiva wara li joħroġ il-programm.
Ebda ġestjoni ta 'żbalji
Array underrun huwa possibbli. Il-valur tal-indiċi 'n' jista' jilħaq -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);
}
....
}Is-snippet tal-kodiċi f'dan il-każ jaqra mill-fajl għal buffer sakemm jintemm il-fajl. Madankollu, m'hemm l-ebda ġestjoni ta 'żbalji hawnhekk: jekk xi ħaġa tmur ħażin, allura taqra se jirritorna -1, u mbagħad il-firxa tinqabeż produzzjoni.
Uża EOF fit-tip char
EOF m'għandux jitqabbel ma' valur tat-tip 'char'. Il-'(c = fgetc(fp))' għandu jkun tat-tip '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++;
}
....
}Hawnhekk naraw immaniġġjar żbaljat li jintlaħaq it-tmiem tal-fajl: if fgetc jirritorna karattru li l-kodiċi tiegħu huwa 0xFF, se jiġi interpretat bħala t-tmiem tal-fajl (EOF).
EOF hija kostanti, normalment definita bħala -1. Pereżempju, fil-kodifikazzjoni CP1251, l-aħħar ittra tal-alfabett Russu għandha l-kodiċi 0xFF, li jikkorrispondi man-numru -1 jekk qed nitkellmu dwar varjabbli bħal chariot. Jirriżulta li s-simbolu 0xFF, bħal EOF (-1) huwa interpretat bħala t-tmiem tal-fajl. Biex jiġu evitati żbalji bħal dawn, ir-riżultat tal-funzjoni huwa fgetc għandhom ikunu maħżuna fil-varjabbli simili int.
Typos
Framment 1
L-espressjoni 'write_time' hija dejjem falza. 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; // <=
....
}Forsi l-awtur ta 'dan il-kodiċi ħa ħażin || и && f'kondizzjoni. Ejja nikkunsidraw l-għażliet possibbli għall-valuri write_time и change_time:
- Iż-żewġ varjabbli huma ugwali għal 0: f'dan il-każ nispiċċaw f'fergħa inkella: varjabbli mod_time dejjem ikun 0 irrispettivament mill-kundizzjoni sussegwenti.
- Waħda mill-varjabbli hija 0: mod_time se jkun ugwali għal 0 (sakemm il-varjabbli l-oħra jkollha valur mhux negattiv), għaliex MIN se tagħżel l-iżgħar miż-żewġ għażliet.
- Iż-żewġ varjabbli mhumiex ugwali għal 0: agħżel il-valur minimu.
Meta tissostitwixxi l-kundizzjoni bi write_time && change_time l-imġieba tidher korretta:
- Waħda jew iż-żewġ varjabbli mhumiex ugwali għal 0: agħżel valur mhux żero.
- Iż-żewġ varjabbli mhumiex ugwali għal 0: agħżel il-valur minimu.
Framment 2
L-espressjoni hija dejjem vera. Probabbilment l-operatur '&&' għandu jintuża hawn. 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;
....
}Milli jidher l-operaturi huma mħallta hawn ukoll || и &&Jew == и !=: Varjabbli ma jistax ikollha l-valur 20 u 9 fl-istess ħin.
Ikkopjar linja bla limitu
Sejħa tal-funzjoni 'sprintf' se twassal għal overflow tal-buffer '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);
....
}Meta tħares lejn il-funzjoni b'mod sħiħ, ikun ċar li dan il-kodiċi ma jikkawżax problemi. Madankollu, jistgħu jinqalgħu fil-futur: bidla waħda Ŝejjed u se jkollna buffer overflow - sprint mhix limitata minn xejn, għalhekk meta nikkonkatenaw mogħdijiet nistgħu mmorru lil hinn mill-konfini tal-firxa. Huwa rakkomandat li tinnota din is-sejħa fuq snprintf(fullpath, PATH_MAX, ….).
Kundizzjoni żejda
Parti mill-espressjoni kondizzjonali hija dejjem vera: żid > 0. scard.c 507
static void
inRepos(STREAM in, unsigned int read)
{
SERVER_DWORD add = 4 - read % 4;
if (add < 4 && add > 0)
{
....
}
}Проверка żid > 0 m'hemmx bżonn hawn: il-varjabbli dejjem se jkun akbar minn żero, għaliex aqra % 4 se jirritorna l-bqija tad-diviżjoni, iżda qatt mhu se jkun ugwali għal 4.
xrdp
— implimentazzjoni ta' server RDP b'kodiċi open source. Il-proġett huwa maqsum f'2 partijiet:
- xrdp - implimentazzjoni tal-protokoll. Imqassam taħt il-liċenzja Apache 2.0.
- xorgxrdp - Sett ta' sewwieqa Xorg għall-użu ma' xrdp. Liċenzja - X11 (bħal MIT, iżda tipprojbixxi l-użu fir-reklamar)
L-iżvilupp tal-proġett huwa bbażat fuq ir-riżultati ta 'rdesktop u FreeRDP. Inizjalment, biex taħdem bil-grafika, kellek tuża server VNC separat, jew server X11 speċjali b'appoġġ RDP - X11rdp, iżda bil-miġja ta 'xorgxrdp, il-ħtieġa għalihom sparixxa.
F'dan l-artikolu mhux se nkopru xorgxrdp.
Il-proġett xrdp, bħal dak preċedenti, huwa żgħir ħafna u fih madwar 80 elf linja.

Aktar typos
Il-kodiċi fih il-ġbir ta 'blokki simili. Iċċekkja l-oġġetti 'r', 'g', 'r' fil-linji 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++;
}
....
}
....
}Dan il-kodiċi ttieħed mil-librerija librfxcodec, li timplimenta l-codec jpeg2000 għal RemoteFX. Hawnhekk, apparentement, il-kanali tad-dejta grafika huma mħallta - minflok il-kulur "blu", "aħmar" huwa rreġistrat. Dan l-iżball x'aktarx deher bħala riżultat ta 'copy-paste.
L-istess problema seħħet f'funzjoni simili rfx_encode_format_argb, li l-analizzatur qalilna wkoll:
Il-kodiċi fih il-ġbir ta 'blokki simili. Iċċekkja l-oġġetti 'a', 'r', 'g', 'r' fil-linji 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++;
}Dikjarazzjoni ta' Array
Array qbiż huwa possibbli. Il-valur tal-indiċi 'i — 8' jista' jilħaq 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];
....
}
....
}Id-dikjarazzjoni u d-definizzjoni tal-firxa f'dawn iż-żewġ fajls huma inkompatibbli - id-daqs ivarja b'1. Madankollu, ma jseħħu l-ebda żball - id-daqs korrett huwa speċifikat fil-fajl evdev-map.c, għalhekk m'hemm l-ebda barra mill-limiti. Allura dan huwa biss bug li jista 'jiġi ffissat faċilment.
Tqabbil skorrett
Parti mill-espressjoni kondizzjonali hija dejjem falza: (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))
{
....
}
....
}Il-funzjoni taqra varjabbli tat-tip qasir mhux iffirmat fi varjabbli simili int. L-iċċekkjar mhuwiex meħtieġ hawnhekk għaliex qed naqraw varjabbli mhux iffirmat u nassenjaw ir-riżultat għal varjabbli akbar, għalhekk il-varjabbli ma tistax tieħu valur negattiv.
Kontrolli bla bżonn
Parti mill-espressjoni kondizzjonali hija dejjem vera: (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;
}
....
}Il-kontrolli tal-inugwaljanza ma jagħmlux sens hawnhekk peress li diġà għandna paragun fil-bidu. Huwa probabbli li dan huwa typo u l-iżviluppatur ried juża l-operatur || biex tiffiltra argumenti invalidi.
Konklużjoni
Matul il-verifika, ma ġew identifikati l-ebda żbalji serji, iżda nstabu ħafna nuqqasijiet. Madankollu, dawn id-disinji jintużaw f'ħafna sistemi, għalkemm żgħar fl-ambitu. Proġett żgħir mhux bilfors ikollu ħafna żbalji, għalhekk m'għandekx tiġġudika l-prestazzjoni tal-analizzatur biss fuq proġetti żgħar. Tista’ taqra aktar dwar dan fl-artiklu “".
Tista 'tniżżel verżjoni ta' prova ta 'PVS-Studio mingħandna .
Jekk trid taqsam dan l-artiklu ma’ udjenza li titkellem bl-Ingliż, jekk jogħġbok uża l-link tat-traduzzjoni: Sergey Larin.
Sors: www.habr.com
