To je drugi pregled v seriji člankov o testiranju odprtokodnih programov za delo s protokolom RDP. V njem si bomo ogledali odjemalca rdesktop in strežnik xrdp.
Uporablja se kot orodje za prepoznavanje napak . Je statični analizator kode za jezike C, C++, C# in Java, ki je na voljo na platformah Windows, Linux in macOS.
Članek predstavlja samo tiste napake, ki so se mi zdele zanimive. So pa projekti majhni, tako da je bilo malo napak :).
Obvestilo. Najdete lahko prejšnji članek o preverjanju projekta FreeRDP .
rdesktop
— brezplačna izvedba odjemalca RDP za sisteme, ki temeljijo na Unixu. Lahko se uporablja tudi v sistemu Windows, če zgradite projekt pod Cygwin. Licencirano pod GPLv3.
Ta odjemalec je zelo priljubljen - privzeto se uporablja v ReactOS, zanj pa lahko najdete tudi grafične vmesnike drugih proizvajalcev. Vendar je precej star: njegova prva izdaja se je zgodila 4. aprila 2001 - v času pisanja je star 17 let.
Kot sem že omenil, je projekt precej majhen. Vsebuje približno 30 tisoč vrstic kode, kar je glede na njegovo starost nekoliko čudno. Za primerjavo, FreeRDP vsebuje 320 tisoč vrstic. Tukaj je rezultat programa Cloc:
Nedosegljiva koda
Zaznana nerazpoložljiva koda. Možno je, da je prisotna napaka. 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);
}Na napako se takoj srečamo v funkciji Glavni: kodo vidimo za operaterjem vrnitev — ta fragment izvaja čiščenje pomnilnika. Vendar pa napaka ne predstavlja grožnje: ves dodeljeni pomnilnik bo operacijski sistem počistil po izhodu programa.
Brez obravnavanja napak
Možno je podleganje polja. Vrednost indeksa 'n' lahko doseže -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);
}
....
}Delček kode v tem primeru bere iz datoteke v medpomnilnik, dokler se datoteka ne konča. Vendar tukaj ni obravnavanja napak: če gre kaj narobe, potem preberite vrne -1 in nato bo matrika prekoračena izhod.
Uporaba EOF v vrsti char
EOF se ne sme primerjati z vrednostjo tipa 'char'. '(c = fgetc(fp))' mora biti vrste '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++;
}
....
}Tukaj vidimo nepravilno obravnavanje doseganja konca datoteke: if fgetc vrne znak, katerega koda je 0xFF, bo razložen kot konec datoteke (EOF).
EOF je konstanta, običajno definirana kot -1. Na primer, v kodiranju CP1251 ima zadnja črka ruske abecede kodo 0xFF, ki ustreza številki -1, če govorimo o spremenljivki, kot je kočija. Izkazalo se je, da je simbol 0xFF, kot je EOF (-1) se interpretira kot konec datoteke. Da bi se izognili takim napakam, je rezultat funkcije fgetc mora biti shranjen v spremenljivki, kot je int.
Pravopisne napake
Fragment 1
Izraz 'write_time' je vedno napačen. 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; // <=
....
}Morda se je avtor te kode zmotil || и && v stanju. Razmislimo o možnih možnostih vrednosti pisni_čas и spremeni_čas:
- Obe spremenljivki sta enaki 0: v tem primeru bomo končali v veji ostalo: spremenljivka mod_time bo vedno 0, ne glede na naslednji pogoj.
- Ena od spremenljivk je 0: mod_time bo enako 0 (pod pogojem, da ima druga spremenljivka nenegativno vrednost), ker MIN bo izbral manjšo od obeh možnosti.
- Obe spremenljivki nista enaki 0: izberite najmanjšo vrednost.
Pri zamenjavi pogoja z pisni_čas && spremeni_čas vedenje bo videti pravilno:
- Ena ali obe spremenljivki nista enaki 0: izberite vrednost, ki ni enaka nič.
- Obe spremenljivki nista enaki 0: izberite najmanjšo vrednost.
Fragment 2
Izraz je vedno resničen. Tukaj bi verjetno morali uporabiti 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čitno so tudi tu operaterji pomešani || и &&Ali == и !=: Spremenljivka ne more imeti vrednosti 20 in 9 hkrati.
Neomejeno kopiranje vrstic
Klic funkcije 'sprintf' bo povzročil prepolnitev medpomnilnika '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);
....
}Ko si funkcijo ogledate v celoti, bo jasno, da ta koda ne povzroča težav. Vendar pa se lahko pojavijo v prihodnosti: ena neprevidna sprememba in dobili bomo prekoračitev medpomnilnika - sprint ni z ničemer omejen, zato lahko pri veriženju poti presežemo meje polja. Priporočljivo je, da ta klic opazite na snprintf(polna pot, PATH_MAX, ….).
Odvečno stanje
Del pogojnega izraza je vedno resničen: add > 0. scard.c 507
static void
inRepos(STREAM in, unsigned int read)
{
SERVER_DWORD add = 4 - read % 4;
if (add < 4 && add > 0)
{
....
}
}Проверка dodaj > 0 tukaj ni potrebe: spremenljivka bo vedno večja od nič, ker preberi % 4 vrne preostanek deljenja, vendar nikoli ne bo enak 4.
xrdp
— izvedba strežnika RDP z odprtokodno kodo. Projekt je razdeljen na 2 dela:
- xrdp - implementacija protokola. Distribuirano pod licenco Apache 2.0.
- xorgxrdp - niz gonilnikov Xorg za uporabo z xrdp. Licenca - X11 (kot MIT, vendar prepoveduje uporabo v oglaševanju)
Razvoj projekta temelji na rezultatih rdesktop in FreeRDP. Sprva ste za delo z grafiko morali uporabiti ločen strežnik VNC ali poseben strežnik X11 s podporo za RDP - X11rdp, vendar je s prihodom xorgxrdp potreba po njih izginila.
V tem članku ne bomo obravnavali xorgxrdp.
Projekt xrdp je tako kot prejšnji zelo majhen in vsebuje približno 80 tisoč vrstic.
Več tipkarskih napak
Koda vsebuje zbirko podobnih blokov. Preverite elemente 'r', 'g', 'r' v vrsticah 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++;
}
....
}
....
}Ta koda je bila vzeta iz knjižnice librfxcodec, ki implementira kodek jpeg2000 za RemoteFX. Tu so očitno pomešani grafični podatkovni kanali - namesto "modre" barve je posneta "rdeča". Ta napaka se je najverjetneje pojavila kot posledica kopiranja in lepljenja.
Ista težava se je pojavila pri podobni funkciji rfx_encode_format_argb, kar nam je analizator tudi povedal:
Koda vsebuje zbirko podobnih blokov. Preverite postavke 'a', 'r', 'g', 'r' v vrsticah 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 polja
Možna je prekoračitev polja. Vrednost indeksa 'i — 8' lahko doseže 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 in definicija matrike v teh dveh datotekah sta nezdružljivi - velikost se razlikuje za 1. Vendar pa ne pride do napak - pravilna velikost je podana v datoteki evdev-map.c, tako da ni izven meja. To je torej samo napaka, ki jo je mogoče enostavno popraviti.
Napačna primerjava
Del pogojnega izraza je vedno napačen: (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 prebere spremenljivko tipa nepodpisana kratka v spremenljivko, kot je int. Preverjanje tukaj ni potrebno, ker beremo nepredznačeno spremenljivko in rezultat pripisujemo večji spremenljivki, zato spremenljivka ne more imeti negativne vrednosti.
Nepotrebni pregledi
Del pogojnega izraza je vedno resničen: (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;
}
....
}Preverjanje neenakosti tukaj ni smiselno, saj imamo primerjavo že na začetku. Verjetno je to tipkarska napaka in je razvijalec želel uporabiti operater || za filtriranje neveljavnih argumentov.
Zaključek
Med revizijo resnih napak ni bilo ugotovljenih, so pa bile ugotovljene številne pomanjkljivosti. Vendar pa se te zasnove uporabljajo v številnih sistemih, čeprav v majhnem obsegu. Ni nujno, da ima majhen projekt veliko napak, zato ne smete ocenjevati delovanja analizatorja le pri majhnih projektih. Več o tem si lahko preberete v članku “".
Pri nas lahko prenesete preizkusno različico PVS-Studio .
Če želite ta članek deliti z angleško govorečim občinstvom, uporabite povezavo za prevod: Sergey Larin.
Vir: www.habr.com