ProHoster > Blog > Administrimi > Kontrolli i rdesktop dhe xrdp me ndihmën e analizuesit PVS-Studio

Kontrolli i rdesktop dhe xrdp me ndihmën e analizuesit PVS-Studio

Kontrollimi i rdesktop dhe xrdp me ndihmën e analizatorit PVS-Studio
Ky është shqyrtimi i dytë nga seria e artikujve për kontrollin e programeve të hapura që punojnë me protokollin RDP. Në të do të shqyrtojmë klientin rdesktop dhe serverin xrdp.

Si mjet për identifikimin e gabimeve u përdor PVS-Studio. Ky është një analizues statik i kodit për gjuhët C, C++, C# dhe Java, i disponueshëm në platformat Windows, Linux dhe macOS.

Në artikull janë paraqitur vetëm ato gabime që më dukeshin interesante. Megjithatë, projektet janë të vogla, kështu që gabimet ishin të pakta :).

Shënim. Artikullin e mëparshëm për kontrollet mbi projektin FreeRDP mund ta gjeni këtu.

rdesktop

rdesktop — njĂ« zbatim i lirĂ« i klientit RDP pĂ«r sistemet UNIX-based. Ai gjithashtu mund tĂ« pĂ«rdoret nĂ«n Windows, duke e ndĂ«rtuar projektin nĂ«n Cygwin. Licenca Ă«shtĂ« GPLv3.

Ky klient ka popullaritet tĂ« madh — pĂ«rdoret si parazgjedhje nĂ« ReactOS, pĂ«r tĂ« cilin gjithashtu mund tĂ« gjeni front-end tĂ« tjerĂ« tĂ« jashtme. MegjithatĂ«, ai Ă«shtĂ« mjaft i vjetĂ«r: versioni i parĂ« u publikua mĂ« 4 prill 2001 — nĂ« momentin e shkrimit tĂ« kĂ«tij artikulli, mosha e tij Ă«shtĂ« 17 vjet.

Siç e kam theksuar më parë, projekti është shumë i vogël. Ai përmban rreth 30 mijë rreshta kode, gjë që duket disi e çuditshme, duke marrë parasysh moshën e tij. Për krahasim, FreeRDP ka 320 mijë rreshta. Këtu është rezultati i programit Cloc:

Kontrollimi i rdesktop dhe xrdp me ndihmën e analizatorit PVS-Studio

Kodi i papërmbushur

V779 Kodi i arritur nuk u zbulua. ËshtĂ« e mundur qĂ« njĂ« gabim Ă«shtĂ« prezent. rdesktop.c 1502

int
main(int argc, char *argv[])
{
  ....
  kthehu handle_disconnect_reason(deactivated, ext_disc_reason);

  nëse (g_redirect_username)
    xfree(g_redirect_username);

  xfree(g_username);
}

Gabimi na takon menjĂ«herĂ« nĂ« funksion main: ne shohim kodin qĂ« vjen pas operatorit kthehu — ky fragment kryen pastrimin e memories. MegjithatĂ«, gabimi nuk paraqet rrezik: tĂ« gjitha memoriet e alokuara do tĂ« pastrohen nga sistemi operativ pas pĂ«rfundimit tĂ« programit.

Mungesa e përpunimit të gabimeve

V557 Mund tĂ« ndodhĂ« njĂ« nĂ«nzhierje e Array. Vlera e indeksit ‘n’ mund tĂ« arrijĂ« -1. rdesktop.c 1872

RD_BOOL
subprocess(char *const argv[], str_handle_lines_t linehandler, void *data)
{
  int n = 1;
  char output[256];
  ....
  ndërsa (n > 0)
  {
    n = read(fd[0], output, 255);
    output[n] = ' '; // <=
    str_handle_lines(output, &rest, linehandler, data);
  }
  ....
}

Fragmenti i kodit në këtë rast lexon nga skedari në buferr derisa skedari të mbarojë. Megjithatë, përpunimi i gabimeve këtu mungon: nëse ndodhi diçka e gabuar, atëherë read do të kthejë -1, dhe atëherë do të ndodhi tejkalimi i kufijve të arrays output.

Përdorimi i EOF në llojin char

V739 EOF nuk duhet tĂ« krahasohet me njĂ« vlerĂ« tĂ« llojit ‘char’. ‘(c = fgetc(fp))’ duhet tĂ« jetĂ« i llojit ‘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++;
  }
  ....
}

Këtu shohim një trajtim jo korrekt për arritjen e fundit të skedarit: nëse fgetc kthen karakterin, kodi i të cilit është 0xFF, atëherë ai do të interpretohet si fund skedari (EOF).

EOF kjo është një konstante, e cila zakonisht përcaktohet si -1. Për shembull, në kodimin CP1251 letra e fundit e alfabetit rus ka kodin 0xFF, që korrespondon me numrin -1, nëse flasim për një variabël të tipit char. Kështu që, karakteri 0xFF, si dhe EOF (-1) perceptohet si fund skedari. Për të shmangur gabime të tilla, rezultati i funksionit fgetc duhet të ruhet në një variabël të tipit int.

Gabime shkrimi

Fragmenti 1

V547 Shprehja ‘write_time’ Ă«shtĂ« gjithmonĂ« 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; // <=
  ....
}

Ndoshta autori i këtij kodi e ka ngatërruar || dhe && në kusht. Le të shqyrtojmë mundësitë për vlerat write_time dhe change_time:

  • TĂ« dy variablat janĂ« tĂ« barabarta me 0: nĂ« kĂ«tĂ« rast ne do tĂ« shkojmĂ« nĂ« degĂ«n else: variabla mod_time do tĂ« jetĂ« gjithmonĂ« e barabartĂ« me 0, pavarĂ«sisht nga kushti i mĂ«passhĂ«m.
  • NjĂ« nga variablat Ă«shtĂ« e barabartĂ« me 0: mod_time do tĂ« jetĂ« e barabartĂ« me 0 (me kusht qĂ« variabla tjetĂ«r tĂ« ketĂ« vlerĂ« jo negative), sepse MIN do tĂ« zgjedhĂ« mĂ« tĂ« voglin nga dy mundĂ«sitĂ«.
  • TĂ« dy variablat nuk janĂ« tĂ« barabartĂ« me 0: zgjedhim vlerĂ«n minimale.

Kur e zëvendësojmë kushtin me write_time && change_time sjellja do të duket e saktë:

  • NjĂ« ose tĂ« dy variablat nuk janĂ« tĂ« barabartĂ« me 0: zgjedhim vlerĂ«n e ndryshme nga 0.
  • TĂ« dy variablat nuk janĂ« tĂ« barabartĂ« me 0: zgjedhim vlerĂ«n minimale.

Fragmenti 2

V547 Shprehja Ă«shtĂ« gjithmonĂ« e vĂ«rtetĂ«. Ndoshta operatori ‘&&’ duhet tĂ« pĂ«rdoret kĂ«tu. 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;
  ....
}

Duket se edhe këtu janë ngatërruar operatorët || dhe &&, ose == dhe !=: variabla nuk mund të marrë njëkohësisht vlerën 20 dhe 9.

Kopjimi i pakufizuar i vargut

V512 NjĂ« thirrje e funksionit ‘sprintf’ do tĂ« çojĂ« nĂ« mbushjen e tepruar tĂ« buffer-it ‘fullpath’. disk.c 1257

RD_NTSTATUS
disk_query_directory(....)
{
  ....
  char *dirname, fullpath[PATH_MAX];
  ....
  /* Merrni informacion për hyrjen e direktorisë */
  sprintf(fullpath, "%s/%s", dirname, pdirent->d_name);
  ....
}

Duke shqyrtuar funksionin, do tĂ« kuptohet plotĂ«sisht se ky kod nuk shkakton probleme. MegjithatĂ«, ato mund tĂ« shfaqen nĂ« tĂ« ardhmen: njĂ« ndryshim i padijshĂ«m dhe do tĂ« kemi mbushje tĂ« tepruar tĂ« buffer-it — sprintf nuk Ă«shtĂ« i kufizuar, kĂ«shtu qĂ« gjatĂ« konkatenimit tĂ« rrugĂ«ve mund tĂ« dalim jashtĂ« kufijve tĂ« vargut. Rekomandohet tĂ« shĂ«nohet ky thirrje nĂ« snprintf(fullpath, PATH_MAX, 
.).

Kusht i tepruar

V560 Një pjesë e shprehjes kushtore është gjithmonë e vërtetë: shtoni > 0. scard.c 507

static void
inRepos(STREAM in, unsigned int read)
{
  SERVER_DWORD add = 4 - read % 4;
  if (add  0)
  {
    ....
  }
}

Kontroll add > 0 këtu nuk ka kuptim: variabla gjithmonë do të jetë më e madhe se zero, pasi read % 4 do të kthejë mbetjen e ndarjes, dhe ajo nuk do të jetë kurrë e barabartë me 4.

xrdp

xrdp — implementimi i serverit RDP me kod tĂ« hapur. Projekti ndahen nĂ« 2 pjesĂ«:

  • xrdp — implementimi i protokollit. ShpĂ«rndahet nĂ«n licencĂ«n Apache 2.0.
  • xorgxrdp — njĂ« grup drejtuesish Xorg pĂ«r pĂ«rdorim me xrdp. Licenca — X11 (si MIT, por ndalon pĂ«rdorimin nĂ« reklama)

Zhvillimi i projektit bazohet nĂ« rezultatet e rdesktop dhe FreeRDP. Fillimisht pĂ«r tĂ« punuar me grafikĂ« ishte e nevojshme tĂ« pĂ«rdoret njĂ« server VNC tĂ« veçantĂ«, ose njĂ« server i veçantĂ« X11 me mbĂ«shtetje RDP — X11rdp, megjithatĂ« me shfaqjen e xorgxrdp nevoja pĂ«r to Ă«shtĂ« eliminuar.

Në këtë artikull ne nuk do të trajtojmë xorgxrdp.

Projekti xrdp, ashtu si i mëparshmi, është mjaft i vogël dhe përmban rreth 80 mijë rreshta.

Kontrollimi i rdesktop dhe xrdp me ndihmën e analizatorit PVS-Studio

Akoma gabime të shtypura

V525 Kodi përmban një koleksion blokesh të ngjashme. Kontrolloni elementet 'r', 'g', 'r' në linjat 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++;
      }
      ....
  }
  ....
}

Ky kod Ă«shtĂ« marrĂ« nga biblioteka librfxcodec, qĂ« implementon koduesin jpeg2000 pĂ«r tĂ« punuar me RemoteFX. KĂ«tu, duket se janĂ« ngatĂ«rruar kanalet e tĂ« dhĂ«nave grafike — nĂ« vend tĂ« ngjyrĂ«s "blu" regjistrohet "e kuqe". NjĂ« gabim i tillĂ«, me siguri, ka lindur si rezultat i kopjimit dhe ngjitjes.

Kjo problematikë ka ndodhur edhe në një funksion të ngjashëm rfx_encode_format_argb, për të cilën na njoftoi gjithashtu analizatori:

V525 Kodi përmban një koleksion blokesh të ngjashme. Kontrolloni elementet 'a', 'r', 'g', 'r' në linjat 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++;
}

Deklarimi i array-it

V557 Kohëzgjatja e array-it është e mundshme. Vlera e indeksit 'i - 8' mund të arrijë 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];
    ....
  }
  ....
}

Deklarata dhe përcaktimi i arrays në këta dy skeda nuk janë të përshtatshme - madhësia ndryshon me 1. Megjithatë, nuk ka ndodhur ndonjë gabim - në skedën evdev-map.c është shënuar madhësia e saktë, kështu që nuk ka tejkalim. Kështu që kjo është vetëm një mangësi që është e lehtë për t'u korrigjuar.

Krahasim i pasaktë

V560 Një pjesë e shprehjes kushtore është gjithmonë e vërtetë: (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))
  {
    ....
  }
  ....
}

Në funksion ndodh leximi i variablës të tipit unsigned short në variablën e tipit int. Kontrolli këtu nuk është i nevojshëm, sepse ne lexojmë një variabël me format të paqartë dhe i caktojmë rezultatin një variabli më të madh, kështu që variabla nuk mund të marrë një vlerë negative.

Kontrollime të tepruara

V560 Një pjesë e shprehjes kushtore është gjithmonë e vërtetë: (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: gabim");
      return 1;
  }
  ....
}

Kontrollimet pĂ«r pa-barazi kĂ«tu nuk kanĂ« kuptim, sepse ne tashmĂ« kemi njĂ« krahasim nĂ« fillim. ËshtĂ« shumĂ« e mundshme qĂ« kjo Ă«shtĂ« njĂ« gabim shkrimi dhe zhvilluesi dĂ«shiron tĂ« pĂ«rdorĂ« operatorin || pĂ«r tĂ« filtruar argumentet e gabuara.

PĂ«rfundimi

Gjatë kontrollit nuk u gjetën gabime serioze, por u gjetën shumë shtrembërime. Megjithatë, këto projekte përdoren në shumë sisteme, edhe pse janë të vogla në përmasat e tyre. Në një projekt të vogël nuk është e nevojshme të ketë shumë gabime, prandaj nuk duhet të gjykoni punën e analizatorit vetëm në projekte të vogla. Më shumë rreth kësaj mund të lexoni në artikullin "Ndjenja që u konfirmuan me numra«.

Mund të shkarkoni versionin provues të PVS-Studio nga ne në të internetit.

Kontrollimi i rdesktop dhe xrdp me ndihmën e analizatorit PVS-Studio

Nëse dëshironi ta ndani këtë artikull me audiencën anglishtfolëse, ju lutem përdorni lidhjen për përkthimin: Sergey Larin. Kontrollimi i rdesktop dhe xrdp me PVS-Studio

Burimi: habr.com

Bleni hostim tĂ« besueshĂ«m pĂ«r faqe me mbrojtje nga DDoS, serverĂ« VPS VDS đŸ”„ Bleni hostim tĂ« besueshĂ«m pĂ«r faqe me mbrojtje nga DDoS, serverĂ« VPS VDS | ProHoster