🥇Provjera rdesktop i xrdp pomoću PVS-Studio analizatora

Ovo je drugi pregled u nizu članaka o testiranju programa otvorenog koda za rad s RDP protokolom. U njemu ćemo pogledati rdesktop klijent i xrdp poslužitelj.

Koristi se kao alat za prepoznavanje grešaka PVS-Studio. To je statički analizator koda za jezike C, C++, C# i Java, dostupan na Windows, Linux i macOS platformama.

U članku su prikazane samo one pogreške koje su mi se činile zanimljivima. Ipak, projekti su mali, pa je bilo malo grešaka :).

Primijetiti. Prethodni članak o provjeri FreeRDP projekta može se pronaći здесь.

rdesktop

rdesktop — besplatna implementacija RDP klijenta za sustave temeljene na UNIX-u. Također se može koristiti pod Windowsima ako gradite projekt pod Cygwinom. Licencirano pod GPLv3.

Ovaj je klijent vrlo popularan - koristi se prema zadanim postavkama u ReactOS-u, a za njega možete pronaći i grafička sučelja trećih strana. Međutim, on je prilično star: njegovo prvo izdanje dogodilo se 4. travnja 2001. - u vrijeme pisanja, imao je 17 godina.

Kao što sam ranije napomenuo, projekt je vrlo mali. Sadrži otprilike 30 tisuća redaka koda, što je malo čudno s obzirom na njegovu starost. Usporedbe radi, FreeRDP sadrži 320 tisuća linija. Ovo je rezultat programa Cloc:

Nedostupan kod

V779 Otkriven nedostupan kod. Moguće je da postoji greška. 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);
}

Greška nam se javlja odmah u funkciji glavni: vidimo kod koji dolazi nakon operatora povratak — ovaj fragment izvodi čišćenje memorije. Međutim, pogreška ne predstavlja prijetnju: operativni sustav će izbrisati svu dodijeljenu memoriju nakon što program izađe iz programa.

Nema obrade grešaka

V557 Moguće je spuštanje niza. Vrijednost 'n' indeksa može doseći -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);
  }
  ....
}

Isječak koda u ovom slučaju čita iz datoteke u međuspremnik dok datoteka ne završi. Međutim, ovdje nema obrade grešaka: ako nešto pođe po zlu, onda čitati vratit će -1, a zatim će niz biti prekoračen izlaz.

Korištenje EOF-a u tipu char

V739 EOF se ne smije uspoređivati s vrijednošću tipa 'char'. '(c = fgetc(fp))' treba biti tipa '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++;
  }
  ....
}

Ovdje vidimo neispravno rukovanje dosezanjem kraja datoteke: if fgetc vraća znak čiji je kod 0xFF, to će se protumačiti kao kraj datoteke (EOF).

EOF to je konstanta, obično definirana kao -1. Na primjer, u CP1251 kodiranju, posljednje slovo ruske abecede ima kod 0xFF, što odgovara broju -1 ako govorimo o varijabli poput čađ. Ispada da simbol 0xFF, kao EOF (-1) se tumači kao kraj datoteke. Da bi se izbjegle takve pogreške, rezultat funkcije je fgetc treba pohraniti u varijablu poput int.

Pravopisne pogreške

Fragment 1

V547 Izraz 'write_time' je uvijek 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; // <=
  ....
}

Možda je autor ovog koda pogriješio || и && u stanju. Razmotrimo moguće opcije za vrijednosti vrijeme_pisanja и promjena_vremena:

Obje varijable su jednake 0: u ovom slučaju ćemo završiti u grani drugo: varijabla mod_vrijeme uvijek će biti 0 bez obzira na naknadni uvjet.
Jedna od varijabli je 0: mod_vrijeme bit će jednaka 0 (pod uvjetom da druga varijabla ima nenegativnu vrijednost), jer MIN će odabrati manju od dvije mogućnosti.
Obje varijable nisu jednake 0: odaberite minimalnu vrijednost.

Prilikom zamjene stanja sa vrijeme_pisanja && vrijeme_promjene ponašanje će izgledati ispravno:

Jedna ili obje varijable nisu jednake 0: odaberite vrijednost koja nije nula.
Obje varijable nisu jednake 0: odaberite minimalnu vrijednost.

Fragment 2

V547 Izraz je uvijek istinit. Vjerojatno bi se ovdje trebao koristiti 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čito su se i tu operateri pomiješali || и &&Ili == и !=: Varijabla ne može imati vrijednost 20 i 9 u isto vrijeme.

Neograničeno kopiranje linija

V512 Poziv funkcije 'sprintf' dovest će do prekoračenja međuspremnika '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);
  ....
}

Kada pogledate funkciju u cijelosti, bit će jasno da ovaj kod ne uzrokuje probleme. Međutim, mogu se pojaviti u budućnosti: jedna nepažljiva promjena i dobit ćemo prekoračenje međuspremnika - sprint nije ničim ograničen, pa kod ulančavanja staza možemo izaći izvan granica niza. Preporuča se primijetiti ovaj poziv na snprintf(puni put, PATH_MAX, ….).

Suvišno stanje

V560 Dio uvjetnog izraza uvijek je istinit: 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 ovdje nema potrebe: varijabla će uvijek biti veća od nule, jer čitaj % 4 vratit će ostatak dijeljenja, ali nikada neće biti jednak 4.

xrdp

xrdp — implementacija RDP poslužitelja s otvorenim kodom. Projekt je podijeljen u 2 dijela:

xrdp - implementacija protokola. Distribuira se pod licencom Apache 2.0.
xorgxrdp - Skup Xorg upravljačkih programa za korištenje s xrdp-om. Licenca - X11 (kao MIT, ali zabranjuje korištenje u oglašavanju)

Razvoj projekta temelji se na rezultatima rdesktopa i FreeRDP-a. U početku, za rad s grafikom, morali ste koristiti zasebni VNC poslužitelj ili poseban X11 poslužitelj s podrškom za RDP - X11rdp, ali s dolaskom xorgxrdp-a, potreba za njima je nestala.

U ovom članku nećemo obraditi xorgxrdp.

Projekt xrdp, kao i prethodni, vrlo je malen i sadrži otprilike 80 tisuća redaka.

Više tipfelera

V525 Kod sadrži kolekciju sličnih blokova. Provjerite stavke 'r', 'g', 'r' u redovima 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++;
      }
      ....
  }
  ....
}

Ovaj kod je preuzet iz biblioteke librfxcodec, koja implementira kodek jpeg2000 za RemoteFX. Ovdje su, očito, kanali grafičkih podataka pomiješani - umjesto "plave" boje snima se "crvena". Ova se pogreška najvjerojatnije pojavila kao rezultat kopiranja i lijepljenja.

Isti se problem pojavio u sličnoj funkciji rfx_encode_format_argb, što nam je analizator također rekao:

V525 Kod sadrži kolekciju sličnih blokova. Provjerite stavke 'a', 'r', 'g', 'r' u redovima 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

V557 Moguće je prekoračenje polja. Vrijednost indeksa 'i — 8' mogla bi doseći 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 i definicija niza u ove dvije datoteke su nekompatibilne - veličina se razlikuje za 1. Međutim, ne dolazi do pogrešaka - točna veličina navedena je u datoteci evdev-map.c, tako da nema izvan granica. Dakle, ovo je samo greška koja se lako može popraviti.

Netočna usporedba

V560 Dio uvjetnog izraza uvijek je lažan: (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 čita varijablu tipa nepotpisani kratki u varijablu poput int. Provjera ovdje nije potrebna jer čitamo nepredznačenu varijablu i pridjeljujemo rezultat većoj varijabli, tako da varijabla ne može imati negativnu vrijednost.

Nepotrebne provjere

V560 Dio uvjetnog izraza uvijek je istinit: (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;
  }
  ....
}

Provjere nejednakosti ovdje nemaju smisla jer već imamo usporedbu na početku. Vjerojatno je ovo pogreška pri upisu i programer je želio upotrijebiti operator || za filtriranje nevažećih argumenata.

Zaključak

Tijekom revizije nisu utvrđene ozbiljne pogreške, ali su pronađeni mnogi nedostaci. Međutim, ovi dizajni se koriste u mnogim sustavima, iako malog opsega. Mali projekt ne mora nužno imati mnogo pogrešaka, stoga ne biste trebali prosuđivati rad analizatora samo na malim projektima. Više o tome možete pročitati u članku “Osjećaji koji su bili potvrđeni brojkama”.

Kod nas možete preuzeti probnu verziju PVS-Studio Online.

Ako želite podijeliti ovaj članak s publikom koja govori engleski, upotrijebite vezu za prijevod: Sergey Larin. Provjera rdesktop i xrdp s PVS-Studio

Izvor: www.habr.com

Provjera rdesktop i xrdp pomoću analizatora PVS-Studio