ProHoster > Blog > Administrim > Kontrolli i rdesktop dhe xrdp me analizuesin PVS-Studio

Kontrolli i rdesktop dhe xrdp me analizuesin PVS-Studio

Shqyrtimi i rdesktop dhe xrdp me ndihmën e analizuesit PVS-Studio
Ky është shqyrtimi i dytë nga cikli i artikujve për kontrollimin e programeve të hapura për të punuar me protokollin RDP. Në të do të shqyrtojmë klientin rdesktop dhe serverin xrdp.

Si një 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ë këtë artikull janë të paraqitura vetëm ato gabime që më dukeshin interesante. Megjithatë, projektet janë të vogla, prandaj edhe gabimet ishin të pakta : ).

Shënim. Artikulli i mëparshëm për shqyrtimin e projektit FreeRDP mund të gjendet këtu.

rdesktop

rdesktop — njĂ« implementim i lirĂ« i klientit RDP pĂ«r sistemet UNIX-based. Ai gjithashtu mund tĂ« pĂ«rdoret edhe nĂ«n Windows, nĂ«se projekti ndĂ«rtohet nĂ«n Cygwin. Licencuar nĂ«n GPLv3.

Ky klient ka popullaritet tĂ« madh — ai pĂ«rdoret si parazgjedhje nĂ« ReactOS, gjithashtu pĂ«r tĂ« mund tĂ« gjenden front-end tĂ« tretĂ«. MegjithatĂ«, ai Ă«shtĂ« mjaft i vjetĂ«r: lĂ«shimi i parĂ« ndodhi mĂ« 4 prill 2001 — nĂ« momentin e shkrimit tĂ« artikullit, mosha e tij Ă«shtĂ« 17 vjet.

Siç e kam përmendur më parë, projekti është shumë i vogël. Ai përmban rreth 30 mijë rreshta kodi, që është pak e çuditshme, duke e marrë parasysh moshën e tij. Për krahasim, FreeRDP përmban 320 mijë rreshta. Ja rezultati i programit Cloc:

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

Kodi i paarrijshëm

V779 Kodi i paarrijshĂ«m Ă«shtĂ« zbuluar. ËshtĂ« e mundur qĂ« tĂ« jetĂ« njĂ« gabim. 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);
}

Gabimi na takon menjĂ«herĂ« nĂ« funksionin main: ne shohim kodin qĂ« vjen pas operatorit return — ky fragment kryen pastrimin e memories. MegjithatĂ«, gabimi nuk paraqet ndonjĂ« kĂ«rcĂ«nim: tĂ« gjitha memoriet e ndara do tĂ« pastrohen nga sistemi operativ pas pĂ«rfundimit tĂ« programit.

Mungesa e përpunimit të gabimeve

V557 Nënçeshtja e array është e mundur. 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];
  ....
  while (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ë buffer derisa skedari të përfundojë. Megjithatë, përpunimi i gabimeve mungon këtu: nëse ndodhi ndonjë gjë e gabuar, read do të kthejë -1, dhe atëherë do të ndodhë tejkalimi i kufijve të array 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ë e 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 trajtimin e papërshtatshëm të arrijmë fundin e skedarit: nëse fgetc kthen një karakter, kodi i të cilit është 0xFF, ai do të perceptohet si fund skedari (EOF).

EOF kjo është një konstantë, e cila zakonisht përcaktohet si -1. Për shembull, në kodimin CP1251, shkronja e fundit e alfabetit rus ka kodin 0xFF, i cili korrespondon me numrin -1, nëse flasim për një variabël të tipit char. Kështu që, karakteri 0xFF, ashtu si 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.

Gabimet tipografike

Framenti 1

V547 Shprehja ‘write_time’ gjithmonĂ« Ă«shtĂ« e pavĂ«rtetĂ«. disk.c 805

RD_NTSTATUS
disk_set_information(....)
{
  time_t write_time, change_time, access_time, mod_time;
  ....
  nëse (write_time || change_time)
    mod_time = MIN(write_time, change_time);
  përndryshe
    mod_time = write_time ? write_time : change_time; \/\/ <=
  ....
}

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

  • TĂ« dy variablat janĂ« tĂ« barabartĂ« me 0: nĂ« kĂ«tĂ« rast ne do tĂ« shkojmĂ« nĂ« degen 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Ă« 0 (me kusht qĂ« variabla tjetĂ«r tĂ« ketĂ« njĂ« vlerĂ« tĂ« paarritshme), pasi MIN do tĂ« zgjedhĂ« mĂ« tĂ« voglin nga dy mundĂ«sitĂ«.
  • TĂ« dy variablat nuk janĂ« tĂ« barabarta me 0: zgjidhni vlerĂ«n minimale.

Duke zëvendësuar kushtin me write_time && change_time sjellja do të duket e saktë:

  • NjĂ« ose tĂ« dy variablat nuk janĂ« tĂ« barabarta me 0: zgjidhni njĂ« vlerĂ« tĂ« ndryshme nga 0.
  • TĂ« dy variablat nuk janĂ« tĂ« barabarta me 0: zgjidhni vlerĂ«n minimale.

Framenti 2

V547 Shprehja gjithmonĂ« Ă«shtĂ« 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)
{
  ....
  nëse (((request >> 16) != 20) || ((request >> 16) != 9))
    kthe RD_STATUS_INVALID_PARAMETER;
  ....
}

Duket se 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Ă« tejkalimin e tamponit ‘fullpath’. disk.c 1257

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

Duke shqyrtuar funksionin plotĂ«sisht, do tĂ« jetĂ« e qartĂ« se ky kod nuk shkakton probleme. MegjithatĂ«, ato mund tĂ« shfaqen nĂ« tĂ« ardhmen: njĂ« ndryshim i pakujdesshĂ«m, dhe ne do tĂ« kemi tejkalim tamponi — sprintf nuk Ă«shtĂ« i kufizuar, prandaj gjatĂ« bashkimit tĂ« rrugĂ«ve ne mund tĂ« dalim jashtĂ« kufijve tĂ« array-t. Rekomandohet tĂ« vĂ«zhgohet kjo 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)
  {
    ....
  }
}

Kontrolli add > 0 nuk është ashtu: variabli do të jetë gjithmonë më i madh se zero, sepse read % 4 do të kthejë mbetjen e ndarjes, dhe ajo kurrë nuk do të jetë e barabartë me 4.

xrdp

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

  • xrdp — implementimi i protokollit. Distribuohet nĂ«n licencĂ«n Apache 2.0.
  • xorgxrdp — njĂ« grup motorista Xorg pĂ«r t'u pĂ«rdorur 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Ă«n ishte e nevojshme tĂ« pĂ«rdorej njĂ« server i veçantĂ« VNC, ose njĂ« server i veçantĂ« X11 me mbĂ«shtetje pĂ«r RDP — X11rdp, megjithatĂ« me daljen e xorgxrdp nevoja pĂ«r to ra.

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

Projekti xrdp, si edhe ai i mëparshmi, është krejt i vogël dhe përmban rreth 80 mijë rreshta.

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

Ende gabime shtypi

V525 Kodi përmban koleksionin e blloqeve të ngjashme. Kontrolloni artikujt 'r', 'g', 'r' në rreshtat 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, e cila implementon kodekun 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' po regjistrohet 'e kuqe'. NjĂ« gabim i tillĂ«, me sa duket, ka lindur nga kopjimi-ngjitja.

Kjo problematikë ka prekur edhe një funksion të ngjashëm rfx_encode_format_argb, për të cilën na informoi gjithashtu analizeri:

V525 Kodi përmban koleksionin e blloqeve të ngjashme. Kontrolloni artikujt 'a', 'r', 'g', 'r' në rreshtat 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 matrixit

V557 Kryesisht ka mundësi për tejkalim të array-t. Vlera e indekset '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];
    ....
  }
  ....
}

Deklarimi dhe pĂ«rcaktimi i array-it nĂ« kĂ«to dy skedarĂ« Ă«shtĂ« i papĂ«rshtatshĂ«m — madhĂ«sia ndryshon me 1. MegjithatĂ« nuk ndodhin gabime — nĂ« skedarin evdev-map.c Ă«shtĂ« caktuar madhĂ«sia e saktĂ«, kĂ«shtu qĂ« nuk ka kalime pĂ«rtej kufijve. Pra, kjo Ă«shtĂ« thjesht njĂ« gabim qĂ« Ă«shtĂ« lehtĂ«sisht i zgjidhshĂ«m.

Krahasimi i papërshtatshëm

V560 Një pjesë e shprehjes kushtore gjithmonë është e pavë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 ndodhi leximi i variants tipit unsigned short në një variant të tipit intKëtu nuk është e nevojshme të bëni kontrollin, pasi ne lexojmë variablën e tipit të padëshiruar dhe i japim rezultatit një variabël më të madhe, ndaj variabla nuk mund të marrë një vlerë negative.

Kontrolla të panevojshme

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;
  }
  ....
}

Kontrollat për mospërputhje këtu nuk kanë kuptim, pasi ne tashmë kemi një krahasim në fillim. E mundshme është që kjo të jetë një gabim shtypi dhe zhvilluesi donte të përdorte operatorin || për të filtruar argumentet e pasakta.

PĂ«rfundim

Gjatë verifikimit, nuk u gjetën gabime serioze, por u identifikuan shumë parregullsi. Megjithatë, këto projekte përdoren në shumë sisteme, pavarësisht se janë të vogla në përmasë. Në një projekt të vogël, nuk është e nevojshme të ketë shumë gabime, prandaj nuk duhet të gjykohet puna e analizuesit vetëm mbi projekte të vogla. Më shumë rreth kësaj mund të lexoni në artikullin "Ndjenjat që u konfirmuan nga numrat«.

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

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

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

Burimi: habr.com

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