Tai antroji apžvalga iš straipsnių apie atvirojo kodo programų, skirtų darbui su KPP protokolu, testavimą. Jame apžvelgsime rdesktop klientą ir xrdp serverį.
Naudojamas kaip klaidų nustatymo įrankis . Tai statinis kodo analizatorius C, C++, C# ir Java kalboms, prieinamas Windows, Linux ir macOS platformose.
Straipsnyje pateikiamos tik tos klaidos, kurios man pasirodė įdomios. Tačiau projektai nedideli, tad klaidų buvo nedaug :).
Atkreipti dėmesį. Ankstesnį straipsnį apie FreeRDP projekto patikrinimą galite rasti .
rdesktop
— nemokamas KPP kliento diegimas UNIX sistemoms. Jis taip pat gali būti naudojamas sistemoje „Windows“, jei projektą kuriate naudodami „Cygwin“. Licencijuota pagal GPLv3.
Šis klientas yra labai populiarus – jis naudojamas pagal nutylėjimą ReactOS, taip pat galite rasti trečiųjų šalių grafinių sąsajų. Tačiau jis yra gana senas: pirmasis jo pasirodymas įvyko 4 m. balandžio 2001 d. – rašymo metu jam yra 17 metų.
Kaip minėjau anksčiau, projektas yra gana mažas. Jame yra maždaug 30 tūkstančių kodo eilučių, o tai, atsižvelgiant į jo amžių, yra šiek tiek keista. Palyginimui, FreeRDP yra 320 tūkstančių eilučių. Čia yra Cloc programos išvestis:
Nepasiekiamas kodas
Aptiktas nepasiekiamas kodas. Gali būti, kad yra klaida. 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);
}Klaida iš karto susiduria su funkcija pagrindinis: matome kodą, ateinantį po operatoriaus grįžti — šis fragmentas atlieka atminties valymą. Tačiau klaida nekelia grėsmės: programai išėjus, operacinei sistemai išvalys visą skirtą atmintį.
Jokio klaidų tvarkymo
Galimas masyvo paleidimas. „n“ indekso reikšmė gali siekti –1. rdesktop.c 1872 m
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);
}
....
}Šiuo atveju kodo fragmentas nuskaitomas iš failo į buferį, kol failas baigiasi. Tačiau čia nėra klaidų tvarkymo: jei kas nors negerai, tada skaityti grąžins -1, tada masyvas bus viršytas produkcija.
EOF naudojimas char tipo
EOF neturėtų būti lyginamas su „char“ tipo reikšme. „(c = fgetc(fp))“ turėtų būti „int“ tipo. 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++;
}
....
}Čia matome, kad neteisingai elgiamasi pasiekus failo pabaigą: if fgetc grąžina simbolį, kurio kodas yra 0xFF, jis bus interpretuojamas kaip failo pabaiga (EOF).
EOF tai konstanta, paprastai apibrėžiama kaip -1. Pavyzdžiui, CP1251 koduotėje paskutinė rusų abėcėlės raidė turi kodą 0xFF, kuris atitinka skaičių -1, jei kalbame apie kintamąjį, pvz. bakas. Pasirodo, simbolis 0xFF, kaip EOF (-1) interpretuojamas kaip bylos pabaiga. Norint išvengti tokių klaidų, funkcijos rezultatas yra fgetc turėtų būti saugomi kaip kintamasis int.
Rašybos klaidos
1 fragmentas
Išraiška „write_time“ visada yra klaidinga. diskas.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; // <=
....
}Galbūt šio kodo autorius suklydo || и && būklės. Panagrinėkime galimus vertybių variantus rašymo_laikas и keisti_laiką:
- Abu kintamieji lygūs 0: tokiu atveju atsidursime šakoje kitas: kintamasis mod_time visada bus 0, neatsižvelgiant į tolesnę sąlygą.
- Vienas iš kintamųjų yra 0: mod_time bus lygus 0 (su sąlyga, kad kitas kintamasis turi neneigiamą reikšmę), nes MIN pasirinks mažesnę iš dviejų variantų.
- Abu kintamieji nėra lygūs 0: pasirinkite mažiausią reikšmę.
Pakeitus sąlygą su rašymo laikas ir keitimo laikas elgesys atrodys teisingas:
- Vienas arba abu kintamieji nėra lygūs 0: pasirinkite reikšmę, kuri nėra nulis.
- Abu kintamieji nėra lygūs 0: pasirinkite mažiausią reikšmę.
2 fragmentas
Išraiška visada teisinga. Tikriausiai čia turėtų būti naudojamas „&&“ operatorius. diskas.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;
....
}Matyt, čia ir operatoriai susimaišę || и &&Arba == и !=: kintamasis vienu metu negali turėti 20 ir 9 reikšmių.
Neribotas eilučių kopijavimas
Funkcijos „sprintf“ iškvietimas sukels buferio „fullpath“ perpildymą. diskas.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);
....
}Kai pažvelgsite į funkciją iki galo, paaiškės, kad šis kodas nesukelia problemų. Tačiau jų gali atsirasti ateityje: vienas neatsargus pakeitimas ir sulauksime buferio perpildymo - sprintas nėra niekuo ribojamas, todėl sujungdami kelius galime peržengti masyvo ribas. Rekomenduojama atkreipti dėmesį į šį skambutį snprintf(visas kelias, PATH_MAX, ....).
Perteklinė būklė
Sąlyginės išraiškos dalis visada teisinga: pridėti > 0. scard.c 507
static void
inRepos(STREAM in, unsigned int read)
{
SERVER_DWORD add = 4 - read % 4;
if (add < 4 && add > 0)
{
....
}
}Проверка pridėti > 0 čia nereikia: kintamasis visada bus didesnis už nulį, nes skaityti % 4 grąžins likusią padalijimo dalį, bet ji niekada nebus lygi 4.
xrdp
— KPP serverio su atvirojo kodo įdiegimas. Projektas suskirstytas į 2 dalis:
- xrdp – protokolo įgyvendinimas. Platinama pagal Apache 2.0 licenciją.
- xorgxrdp – Xorg tvarkyklių rinkinys, skirtas naudoti su xrdp. Licencija – X11 (kaip MIT, bet draudžia naudoti reklamoje)
Kuriant projektą remiamasi rdesktop ir FreeRDP rezultatais. Iš pradžių, norint dirbti su grafika, reikėjo naudoti atskirą VNC serverį arba specialų X11 serverį su RDP palaikymu – X11rdp, tačiau atsiradus xorgxrdp, jų poreikis dingo.
Šiame straipsnyje neapžvelgsime xorgxrdp.
Xrdp projektas, kaip ir ankstesnis, yra labai mažas ir jame yra maždaug 80 tūkstančių eilučių.
Daugiau rašybos klaidų
Kode yra panašių blokų rinkinys. Pažymėkite elementus „r“, „g“, „r“ 87, 88, 89 eilutėse. 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++;
}
....
}
....
}Šis kodas buvo paimtas iš librfxcodec bibliotekos, kuri įgyvendina jpeg2000 kodeką, skirtą RemoteFX. Čia, matyt, sumaišyti grafiniai duomenų kanalai - vietoj „mėlynos“ spalvos įrašoma „raudona“. Ši klaida greičiausiai atsirado dėl kopijavimo ir įklijavimo.
Ta pati problema iškilo atliekant panašią funkciją rfx_encode_format_argb, kurį analizatorius mums taip pat pasakė:
Kode yra panašių blokų rinkinys. Pažymėkite elementus „a“, „r“, „g“, „r“ 260, 261, 262, 263 eilutėse. rfxencode_rgb_to_yuv.c 260
while (x < 64)
{
*la_buf++ = a;
*lr_buf++ = r;
*lg_buf++ = g;
*lb_buf++ = r;
x++;
}Masyvo deklaracija
Galimas masyvo viršijimas. Indekso „i — 8“ reikšmė gali siekti 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];
....
}
....
}Masyvo deklaravimas ir apibrėžimas šiuose dviejuose failuose yra nesuderinami – dydis skiriasi 1. Tačiau klaidų nebūna – teisingas dydis nurodytas faile evdev-map.c, todėl nėra ribų. Taigi tai tik klaida, kurią galima lengvai ištaisyti.
Neteisingas palyginimas
Sąlyginės išraiškos dalis visada yra klaidinga: (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 nuskaito tipo kintamąjį nepasirašytas trumpas į kintamąjį patinka int. Tikrinti čia nereikia, nes skaitome beženklį kintamąjį ir priskiriame rezultatą didesniam kintamajam, todėl kintamasis negali įgauti neigiamos reikšmės.
Nereikalingi patikrinimai
Sąlyginės išraiškos dalis visada teisinga: (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;
}
....
}Nelygybės patikrinimai čia neturi prasmės, nes mes jau turime palyginimą pradžioje. Tikėtina, kad tai yra rašybos klaida ir kūrėjas norėjo naudoti operatorių || išfiltruoti netinkamus argumentus.
išvada
Audito metu rimtų klaidų nenustatyta, tačiau rasta daug trūkumų. Tačiau šie dizainai naudojami daugelyje sistemų, nors ir nedidelės apimties. Nedidelis projektas nebūtinai turi daug klaidų, todėl nevertėtų vertinti analizatoriaus veikimo tik mažuose projektuose. Daugiau apie tai galite perskaityti straipsnyje ""
Iš mūsų galite atsisiųsti bandomąją PVS-Studio versiją .
Jei norite pasidalinti šiuo straipsniu su angliškai kalbančia auditorija, naudokite vertimo nuorodą: Sergejus Larinas.
Šaltinis: www.habr.com