Iċċekkja rdesktop u xrdp bl-użu tal-analizzatur PVS-Studio

Iċċekkja rdesktop u xrdp bl-użu tal-analizzatur PVS-Studio
Din hija t-tieni reviżjoni f'serje ta 'artikoli dwar l-ittestjar ta' programmi ta 'sors miftuħ biex jaħdmu mal-protokoll RDP. Fiha se nħarsu lejn il-klijent rdesktop u s-server xrdp.

Użat bħala għodda biex jiġu identifikati l-iżbalji PVS-IstudjoHuwa analizzatur tal-kodiċi statiku għal C, C++, C# u Java, disponibbli fuq pjattaformi Windows, Linux и macOS.

L-artiklu jippreżenta biss dawk l-iżbalji li dehru interessanti għalija. Madankollu, il-proġetti huma żgħar, għalhekk kien hemm ftit żbalji :).

Innota. Jista 'jinstab artikolu preċedenti dwar il-verifika tal-proġett FreeRDP hawn.

rdesktop

rdesktop — свободная реализация клиента RDP для UNIX-based систем. Его также можно использовать и под Windows, если собирать проект под Cygwin. Лицензирован под GPLv3.

Dan il-klijent huwa popolari ħafna - huwa użat awtomatikament f'ReactOS, u tista 'ssib ukoll front-ends grafiċi ta' partijiet terzi għalih. Madankollu, huwa pjuttost antik: l-ewwel rilaxx tiegħu seħħ fl-4 ta 'April, 2001 - fil-ħin tal-kitba, huwa għandu 17-il sena.

Kif innutajt qabel, il-proġett huwa pjuttost żgħir. Fiha madwar 30 elf linja ta 'kodiċi, li hija daqsxejn stramba meta wieħed iqis l-età tiegħu. Għal tqabbil, FreeRDP fih 320 elf linja. Hawn hu l-output tal-programm Cloc:

Iċċekkja rdesktop u xrdp bl-użu tal-analizzatur PVS-Studio

Kodiċi li ma jintlaħaqx

V779 Kodiċi mhux disponibbli misjuba. Huwa possibbli li jkun hemm żball. 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);
}

L-iżball jiltaqa' magħna immedjatament fil-funzjoni prinċipali: naraw il-kodiċi ġej wara l-operatur ritorn — dan il-framment iwettaq tindif tal-memorja. Madankollu, l-iżball ma joħloqx theddida: il-memorja kollha allokata se titneħħa mis-sistema operattiva wara li joħroġ il-programm.

Ebda ġestjoni ta 'żbalji

V557 Array underrun huwa possibbli. Il-valur tal-indiċi 'n' jista' jilħaq -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);
  }
  ....
}

Is-snippet tal-kodiċi f'dan il-każ jaqra mill-fajl għal buffer sakemm jintemm il-fajl. Madankollu, m'hemm l-ebda ġestjoni ta 'żbalji hawnhekk: jekk xi ħaġa tmur ħażin, allura taqra se jirritorna -1, u mbagħad il-firxa tinqabeż produzzjoni.

Uża EOF fit-tip char

V739 EOF m'għandux jitqabbel ma' valur tat-tip 'char'. Il-'(c = fgetc(fp))' għandu jkun tat-tip '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++;
  }
  ....
}

Hawnhekk naraw immaniġġjar żbaljat li jintlaħaq it-tmiem tal-fajl: if fgetc jirritorna karattru li l-kodiċi tiegħu huwa 0xFF, se jiġi interpretat bħala t-tmiem tal-fajl (EOF).

EOF hija kostanti, normalment definita bħala -1. Pereżempju, fil-kodifikazzjoni CP1251, l-aħħar ittra tal-alfabett Russu għandha l-kodiċi 0xFF, li jikkorrispondi man-numru -1 jekk qed nitkellmu dwar varjabbli bħal chariot. Jirriżulta li s-simbolu 0xFF, bħal EOF (-1) huwa interpretat bħala t-tmiem tal-fajl. Biex jiġu evitati żbalji bħal dawn, ir-riżultat tal-funzjoni huwa fgetc għandhom ikunu maħżuna fil-varjabbli simili int.

Typos

Framment 1

V547 L-espressjoni 'write_time' hija dejjem falza. 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; // <=
  ....
}

Forsi l-awtur ta 'dan il-kodiċi ħa ħażin || и && f'kondizzjoni. Ejja nikkunsidraw l-għażliet possibbli għall-valuri write_time и change_time:

  • Iż-żewġ varjabbli huma ugwali għal 0: f'dan il-każ nispiċċaw f'fergħa inkella: varjabbli mod_time dejjem ikun 0 irrispettivament mill-kundizzjoni sussegwenti.
  • Waħda mill-varjabbli hija 0: mod_time se jkun ugwali għal 0 (sakemm il-varjabbli l-oħra jkollha valur mhux negattiv), għaliex MIN se tagħżel l-iżgħar miż-żewġ għażliet.
  • Iż-żewġ varjabbli mhumiex ugwali għal 0: agħżel il-valur minimu.

Meta tissostitwixxi l-kundizzjoni bi write_time && change_time l-imġieba tidher korretta:

  • Waħda jew iż-żewġ varjabbli mhumiex ugwali għal 0: agħżel valur mhux żero.
  • Iż-żewġ varjabbli mhumiex ugwali għal 0: agħżel il-valur minimu.

Framment 2

V547 L-espressjoni hija dejjem vera. Probabbilment l-operatur '&&' għandu jintuża hawn. 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;
  ....
}

Milli jidher l-operaturi huma mħallta hawn ukoll || и &&Jew == и !=: Varjabbli ma jistax ikollha l-valur 20 u 9 fl-istess ħin.

Ikkopjar linja bla limitu

V512 Sejħa tal-funzjoni 'sprintf' se twassal għal overflow tal-buffer '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);
  ....
}

Meta tħares lejn il-funzjoni b'mod sħiħ, ikun ċar li dan il-kodiċi ma jikkawżax problemi. Madankollu, jistgħu jinqalgħu fil-futur: bidla waħda Ŝejjed u se jkollna buffer overflow - sprint mhix limitata minn xejn, għalhekk meta nikkonkatenaw mogħdijiet nistgħu mmorru lil hinn mill-konfini tal-firxa. Huwa rakkomandat li tinnota din is-sejħa fuq snprintf(fullpath, PATH_MAX, ….).

Kundizzjoni żejda

V560 Parti mill-espressjoni kondizzjonali hija dejjem vera: żid > 0. scard.c 507

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

Проверка żid > 0 m'hemmx bżonn hawn: il-varjabbli dejjem se jkun akbar minn żero, għaliex aqra % 4 se jirritorna l-bqija tad-diviżjoni, iżda qatt mhu se jkun ugwali għal 4.

xrdp

xrdp — implimentazzjoni ta' server RDP b'kodiċi open source. Il-proġett huwa maqsum f'2 partijiet:

  • xrdp - implimentazzjoni tal-protokoll. Imqassam taħt il-liċenzja Apache 2.0.
  • xorgxrdp - Sett ta' sewwieqa Xorg għall-użu ma' xrdp. Liċenzja - X11 (bħal MIT, iżda tipprojbixxi l-użu fir-reklamar)

L-iżvilupp tal-proġett huwa bbażat fuq ir-riżultati ta 'rdesktop u FreeRDP. Inizjalment, biex taħdem bil-grafika, kellek tuża server VNC separat, jew server X11 speċjali b'appoġġ RDP - X11rdp, iżda bil-miġja ta 'xorgxrdp, il-ħtieġa għalihom sparixxa.

F'dan l-artikolu mhux se nkopru xorgxrdp.

Il-proġett xrdp, bħal dak preċedenti, huwa żgħir ħafna u fih madwar 80 elf linja.

Iċċekkja rdesktop u xrdp bl-użu tal-analizzatur PVS-Studio

Aktar typos

V525 Il-kodiċi fih il-ġbir ta 'blokki simili. Iċċekkja l-oġġetti 'r', 'g', 'r' fil-linji 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++;
      }
      ....
  }
  ....
}

Dan il-kodiċi ttieħed mil-librerija librfxcodec, li timplimenta l-codec jpeg2000 għal RemoteFX. Hawnhekk, apparentement, il-kanali tad-dejta grafika huma mħallta - minflok il-kulur "blu", "aħmar" huwa rreġistrat. Dan l-iżball x'aktarx deher bħala riżultat ta 'copy-paste.

L-istess problema seħħet f'funzjoni simili rfx_encode_format_argb, li l-analizzatur qalilna wkoll:

V525 Il-kodiċi fih il-ġbir ta 'blokki simili. Iċċekkja l-oġġetti 'a', 'r', 'g', 'r' fil-linji 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++;
}

Dikjarazzjoni ta' Array

V557 Array qbiż huwa possibbli. Il-valur tal-indiċi 'i — 8' jista' jilħaq 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];
    ....
  }
  ....
}

Id-dikjarazzjoni u d-definizzjoni tal-firxa f'dawn iż-żewġ fajls huma inkompatibbli - id-daqs ivarja b'1. Madankollu, ma jseħħu l-ebda żball - id-daqs korrett huwa speċifikat fil-fajl evdev-map.c, għalhekk m'hemm l-ebda barra mill-limiti. Allura dan huwa biss bug li jista 'jiġi ffissat faċilment.

Tqabbil skorrett

V560 Parti mill-espressjoni kondizzjonali hija dejjem falza: (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))
  {
    ....
  }
  ....
}

Il-funzjoni taqra varjabbli tat-tip qasir mhux iffirmat fi varjabbli simili int. L-iċċekkjar mhuwiex meħtieġ hawnhekk għaliex qed naqraw varjabbli mhux iffirmat u nassenjaw ir-riżultat għal varjabbli akbar, għalhekk il-varjabbli ma tistax tieħu valur negattiv.

Kontrolli bla bżonn

V560 Parti mill-espressjoni kondizzjonali hija dejjem vera: (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;
  }
  ....
}

Il-kontrolli tal-inugwaljanza ma jagħmlux sens hawnhekk peress li diġà għandna paragun fil-bidu. Huwa probabbli li dan huwa typo u l-iżviluppatur ried juża l-operatur || biex tiffiltra argumenti invalidi.

Konklużjoni

Matul il-verifika, ma ġew identifikati l-ebda żbalji serji, iżda nstabu ħafna nuqqasijiet. Madankollu, dawn id-disinji jintużaw f'ħafna sistemi, għalkemm żgħar fl-ambitu. Proġett żgħir mhux bilfors ikollu ħafna żbalji, għalhekk m'għandekx tiġġudika l-prestazzjoni tal-analizzatur biss fuq proġetti żgħar. Tista’ taqra aktar dwar dan fl-artiklu “Sentimenti li ġew ikkonfermati bin-numri".

Tista 'tniżżel verżjoni ta' prova ta 'PVS-Studio mingħandna Online.

Iċċekkja rdesktop u xrdp bl-użu tal-analizzatur PVS-Studio

Jekk trid taqsam dan l-artiklu ma’ udjenza li titkellem bl-Ingliż, jekk jogħġbok uża l-link tat-traduzzjoni: Sergey Larin. Iċċekkja rdesktop u xrdp ma PVS-Studio

Sors: www.habr.com

Ixtri hosting affidabbli għal siti bi protezzjoni DDoS, servers VPS VDS 🔥 Ixtri hosting ta' websajts affidabbli bi protezzjoni DDoS, servers VPS VDS | ProHoster