Tämä on toinen katsaus artikkelisarjassa, jossa käsitellään avoimen lähdekoodin ohjelmien testaamista RDP-protokollan kanssa työskennellä. Siinä tarkastelemme rdesktop-asiakasta ja xrdp-palvelinta.
Käytetään työkaluna virheiden tunnistamiseen . Se on staattinen koodianalysaattori C-, C++-, C#- ja Java-kielille, saatavilla Windows-, Linux- ja macOS-alustoilla.
Artikkelissa esitetään vain ne virheet, jotka tuntuivat kiinnostavilta. Projektit ovat kuitenkin pieniä, joten virheitä oli vähän :).
Huomata. Edellinen artikkeli FreeRDP-projektin todentamisesta löytyy .
rdesktop
— RDP-asiakkaan ilmainen toteutus UNIX-pohjaisiin järjestelmiin. Sitä voidaan käyttää myös Windowsissa, jos rakennat projektin Cygwinillä. Lisensoitu GPLv3:lla.
Tämä asiakas on erittäin suosittu - sitä käytetään oletuksena ReactOS: ssä, ja voit löytää sille myös kolmannen osapuolen graafiset käyttöliittymät. Hän on kuitenkin melko vanha: hänen ensimmäinen julkaisunsa tapahtui 4. huhtikuuta 2001 - kirjoittamishetkellä hän on 17-vuotias.
Kuten aiemmin totesin, projekti on hyvin pieni. Se sisältää noin 30 tuhatta koodiriviä, mikä on hieman outoa sen ikään nähden. Vertailun vuoksi FreeRDP sisältää 320 tuhatta riviä. Tässä on Cloc-ohjelman tulos:
Koodi, jota ei tavoiteta
Koodi, joka ei ole käytettävissä. On mahdollista, että kyseessä on virhe. 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);
}Virhe havaitsee meidät välittömästi toiminnossa tärkein: näemme koodin tulevan operaattorin jälkeen palata — tämä fragmentti puhdistaa muistia. Virhe ei kuitenkaan aiheuta uhkaa: käyttöjärjestelmä tyhjentää kaiken varatun muistin ohjelman lopettamisen jälkeen.
Ei virheenkäsittelyä
Ryhmän alleajossa on mahdollista. "n"-indeksin arvo voi olla -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);
}
....
}Tässä tapauksessa koodinpätkä lukee tiedostosta puskuriin, kunnes tiedosto loppuu. Tässä ei kuitenkaan ole virheenkäsittelyä: jos jokin menee pieleen, niin sitten luettu palauttaa -1, ja sitten taulukko ylittyy ulostulo.
EOF:n käyttö char-tyypissä
EOF:ää ei pitäisi verrata 'char'-tyyppiseen arvoon. '(c = fgetc(fp))':n tulee olla 'int'-tyyppiä. 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++;
}
....
}Tässä näemme tiedoston loppuun pääsemisen virheellisen käsittelyn: if fgetc palauttaa merkin, jonka koodi on 0xFF, se tulkitaan tiedoston lopuksi (EOF).
EOF se on vakio, joka yleensä määritellään -1. Esimerkiksi CP1251-koodauksessa venäjän aakkosten viimeisellä kirjaimella on koodi 0xFF, joka vastaa numeroa -1, jos puhumme muuttujasta, kuten sotavaunut. Osoittautuu, että symboli 0xFF, kuten EOF (-1) tulkitaan tiedoston lopuksi. Tällaisten virheiden välttämiseksi funktion tulos on fgetc tulee tallentaa muuttujaan kuten int.
Kirjoitusvirheet
Fragmentti 1
Lauseke 'write_time' on aina epätosi. 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; // <=
....
}Ehkä tämän koodin kirjoittaja on ymmärtänyt väärin || и && kunnossa. Pohditaan mahdollisia arvovaihtoehtoja kirjoitusaika и muutos_aika:
- Molemmat muuttujat ovat yhtä suuria kuin 0: tässä tapauksessa päädymme haaraan muu: muuttuva mod_time on aina 0 seuraavasta ehdosta riippumatta.
- Yksi muuttujista on 0: mod_time on yhtä suuri kuin 0 (edellyttäen, että toisella muuttujalla on ei-negatiivinen arvo), koska MIN valitsee kahdesta vaihtoehdosta pienemmän.
- Molemmat muuttujat eivät ole yhtä suuria kuin 0: valitse minimiarvo.
Kun tila korvataan kirjoitusaika && muutosaika käyttäytyminen näyttää oikealta:
- Yksi tai molemmat muuttujat eivät ole yhtä suuria kuin 0: valitse nollasta poikkeava arvo.
- Molemmat muuttujat eivät ole yhtä suuria kuin 0: valitse minimiarvo.
Fragmentti 2
Ilmaisu on aina totta. Tässä pitäisi luultavasti käyttää '&&'-operaattoria. 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;
....
}Ilmeisesti tässäkin on operaattorit sekaisin || и &&Tai == и !=: Muuttujalla ei voi olla arvoja 20 ja 9 samanaikaisesti.
Rajoittamaton rivikopiointi
"sprintf"-funktion kutsu johtaa puskurin "fullpath" ylivuotoon. 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);
....
}Kun tarkastelet toimintoa kokonaan, käy selväksi, että tämä koodi ei aiheuta ongelmia. Niitä voi kuitenkin syntyä tulevaisuudessa: yksi huolimaton muutos ja saamme puskurin ylivuodon - sprintti sitä ei rajoita mikään, joten polkuja ketjutettaessa voimme mennä taulukon rajojen ulkopuolelle. On suositeltavaa huomata tämä kutsu snprintf(fullpath, PATH_MAX, ….).
Ylimääräinen kunto
Ehdollisen lausekkeen osa on aina tosi: add > 0. scard.c 507
static void
inRepos(STREAM in, unsigned int read)
{
SERVER_DWORD add = 4 - read % 4;
if (add < 4 && add > 0)
{
....
}
}Проверка lisää > 0 tässä ei ole tarvetta: muuttuja on aina suurempi kuin nolla, koska lue % 4 palauttaa jaon loppuosan, mutta se ei koskaan ole yhtä suuri kuin 4.
xrdp
— RDP-palvelimen käyttöönotto avoimella lähdekoodilla. Projekti on jaettu 2 osaan:
- xrdp - protokollan toteutus. Jaettu Apache 2.0 -lisenssillä.
- xorgxrdp - Joukko Xorg-ajureita käytettäväksi xrdp:n kanssa. Lisenssi - X11 (kuten MIT, mutta kieltää käytön mainonnassa)
Projektin kehitys perustuu rdesktopin ja FreeRDP:n tuloksiin. Aluksi grafiikan kanssa työskentelyyn piti käyttää erillistä VNC-palvelinta tai erityistä X11-palvelinta RDP-tuella - X11rdp, mutta xorgxrdp:n myötä niiden tarve katosi.
Tässä artikkelissa emme käsittele xorgxrdp:tä.
Xrdp-projekti, kuten edellinenkin, on hyvin pieni ja sisältää noin 80 tuhatta riviä.
Lisää kirjoitusvirheitä
Koodi sisältää kokoelman samanlaisia lohkoja. Tarkista kohteet "r", "g", "r" riveillä 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++;
}
....
}
....
}Tämä koodi on otettu librfxcodec-kirjastosta, joka toteuttaa RemoteFX:n jpeg2000-koodekin. Tässä ilmeisesti graafiset tietokanavat ovat sekoittuneet - "sinisen" värin sijaan tallennetaan "punainen". Tämä virhe ilmeni todennäköisesti copy-paste-toiminnon seurauksena.
Sama ongelma ilmeni samanlaisessa toiminnossa rfx_encode_format_argb, jonka analysaattori myös kertoi meille:
Koodi sisältää kokoelman samanlaisia lohkoja. Tarkista kohteet "a", "r", "g", "r" riveillä 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++;
}Array-ilmoitus
Ryhmän ylitys on mahdollista. Indeksin 'i — 8' arvo voi olla 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];
....
}
....
}Näiden kahden tiedoston taulukon ilmoitus ja määritelmä eivät ole yhteensopivia - koko eroaa yhdellä. Virheitä ei kuitenkaan tapahdu - oikea koko on määritetty tiedostossa evdev-map.c, joten rajojen ulkopuolella ei ole. Tämä on siis vain bugi, joka voidaan helposti korjata.
Väärä vertailu
Ehdollisen lausekkeen osa on aina epätosi: (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))
{
....
}
....
}Funktio lukee tyyppimuuttujan allekirjoittamaton lyhyt muuttujaksi kuten int. Tässä ei tarvitse tarkistaa, koska luemme etumerkitöntä muuttujaa ja kohdistamme tuloksen suurempaan muuttujaan, joten muuttuja ei voi saada negatiivista arvoa.
Tarpeettomia tarkastuksia
Ehdollisen lausekkeen osa on aina tosi: (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;
}
....
}Epätasa-arvotarkistuksissa ei ole tässä järkeä, koska meillä on jo vertailu alussa. On todennäköistä, että tämä on kirjoitusvirhe ja kehittäjä halusi käyttää operaattoria || suodattaaksesi virheelliset argumentit.
Johtopäätös
Tarkastuksessa ei havaittu vakavia virheitä, mutta havaittiin monia puutteita. Näitä malleja käytetään kuitenkin monissa järjestelmissä, vaikkakin laajuudeltaan pieni. Pienessä projektissa ei välttämättä ole paljon virheitä, joten sinun ei pidä arvioida analysaattorin suorituskykyä vain pienissä projekteissa. Voit lukea tästä lisää artikkelista "".
Voit ladata meiltä PVS-Studion kokeiluversion .
Jos haluat jakaa tämän artikkelin englanninkielisen yleisön kanssa, käytä käännöslinkkiä: Sergey Larin.
Lähde: will.com