ProHoster > Blogi > Haldamine > Rdesktopi ja xrdp kontrollimine PVS-Studio analüsaatoriga

Rdesktopi ja xrdp kontrollimine PVS-Studio analüsaatoriga

rdesktopi ja xrdp kontrollimine PVS-Studio analüsaatori abil
See on teine ​​ülevaade artiklite seeriast, mis käsitleb avatud lähtekoodiga programmide testimist RDP-protokolliga töötamiseks. Selles vaatleme rdesktopi klienti ja xrdp-serverit.

Kasutatakse vigade tuvastamise vahendina PVS-stuudioSee on staatiline koodianalüsaator C, C++, C# ja Java jaoks, mis on saadaval platvormidel Windows, Linux и macOS.

Artiklis esitatakse ainult need vead, mis tundusid mulle huvitavad. Projektid on aga väikesed, nii et vigu oli vähe :).

Märkus. Eelmise artikli FreeRDP projekti kontrollimise kohta leiate siin.

rdesktop

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

See klient on väga populaarne – seda kasutatakse ReactOS-is vaikimisi ning sellele võib leida ka kolmanda osapoole graafilisi kasutajaliideseid. Siiski on ta üsna vana: tema esimene vabastamine toimus 4. aprillil 2001 - selle artikli kirjutamise ajal on ta 17-aastane.

Nagu ma varem märkisin, on projekt väga väike. See sisaldab ligikaudu 30 tuhat koodirida, mis on selle vanust arvestades pisut kummaline. Võrdluseks, FreeRDP sisaldab 320 tuhat rida. Siin on programmi Cloc väljund:

rdesktopi ja xrdp kontrollimine PVS-Studio analüsaatori abil

Kättesaamatu kood

V779 Tuvastati kättesaamatu kood. Võimalik, et tegemist on veaga. 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);
}

Funktsioonis ilmneb viga kohe põhiline: näeme, et kood tuleb operaatori järel tagasipöördumine — see fragment puhastab mälu. Viga aga ohtu ei kujuta: operatsioonisüsteem tühjendab pärast programmi väljumist kogu eraldatud mälu.

Vigade käsitlemine puudub

V557 Massiivi allasõit on võimalik. Indeksi "n" väärtus võib ulatuda -1-ni. 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);
  }
  ....
}

Koodilõik loeb sel juhul failist puhvrisse kuni faili lõpuni. Siin pole aga vigade käsitlemist: kui midagi läheb valesti, siis lugenud tagastab -1 ja siis aetakse massiiv üle väljund.

EOF-i kasutamine char-tüübis

V739 EOF-i ei tohiks võrrelda char-tüüpi väärtusega. "(c = fgetc(fp))" peaks olema "int" tüüpi. 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++;
  }
  ....
}

Siin näeme faili lõppu jõudmise ebaõiget käsitlemist: if fgetc tagastab märgi mille kood on 0xFF, tõlgendatakse seda faili lõpuna (EOF).

EOF see on konstant, mida tavaliselt defineeritakse kui -1. Näiteks CP1251 kodeeringus on vene tähestiku viimasel tähel kood 0xFF, mis vastab numbrile -1, kui me räägime muutujast nagu sõjavanker. Selgub, et sümbol 0xFF, nagu EOF (-1) tõlgendatakse faili lõpuna. Selliste vigade vältimiseks on funktsiooni tulemus fgetc tuleks salvestada muutujasse nagu int.

Kirjavead

1. fragment

V547 Avaldis 'write_time' on alati väär. 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; // <=
  ....
}

Võib-olla sai selle koodi autor valesti aru || и && seisukorras. Vaatleme väärtuste võimalikke variante kirjutamisaeg и muutmise_aeg:

  • Mõlemad muutujad on võrdsed 0-ga: sel juhul jõuame harusse teine: muutuv mod_time on alati 0, sõltumata järgnevast tingimusest.
  • Üks muutujatest on 0: mod_time võrdub 0-ga (eeldusel, et teisel muutujal on mittenegatiivne väärtus), sest MIN valib kahest valikust väiksema.
  • Mõlemad muutujad ei ole võrdsed 0-ga: valige minimaalne väärtus.

Tingimuse asendamisel kirjutamisaeg ja muutmisaeg käitumine näeb õige välja:

  • Üks või mõlemad muutujad ei võrdu nulliga: valige nullist erinev väärtus.
  • Mõlemad muutujad ei ole võrdsed 0-ga: valige minimaalne väärtus.

2. fragment

V547 Väljend on alati tõsi. Ilmselt tuleks siin kasutada operaatorit '&&'. 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;
  ....
}

Ilmselt on siingi operaatorid segamini || и &&Või == и !=: muutujal ei tohi olla korraga väärtusi 20 ja 9.

Piiramatu rea kopeerimine

V512 Funktsiooni "sprintf" väljakutse põhjustab puhvri "fullpath" ületäitumise. 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);
  ....
}

Funktsiooni täies mahus vaadates selgub, et see kood probleeme ei tekita. Tulevikus võivad need siiski tekkida: üks hooletu muudatus ja saame puhvri ülevoolu - sprint ei ole millegagi piiratud, nii et teede ühendamisel saame massiivi piiridest kaugemale minna. Soovitatav on seda üleskutset tähele panna snprintf(täistee, PATH_MAX, ….).

Üleliigne seisund

V560 Osa tingimusavaldisest on alati tõene: add > 0. scard.c 507

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

Проверка lisa > 0 siin pole vaja: muutuja on alati suurem kui null, sest loe % 4 tagastab ülejäänud jaotuse, kuid see ei ole kunagi võrdne 4-ga.

xrdp

xrdp — avatud lähtekoodiga RDP-serveri rakendamine. Projekt on jagatud 2 osaks:

  • xrdp - protokolli rakendamine. Levitatakse Apache 2.0 litsentsi alusel.
  • xorgxrdp – Xorgi draiverite komplekt kasutamiseks koos xrdp-ga. Litsents – X11 (nagu MIT, kuid keelab kasutada reklaamides)

Projekti arendus põhineb rdesktopi ja FreeRDP tulemustel. Algselt tuli graafikaga töötamiseks kasutada eraldi VNC serverit või spetsiaalset RDP toega X11 serverit - X11rdp, kuid xorgxrdp tulekuga kadus vajadus nende järele.

Selles artiklis me ei käsitle xorgxrdp-d.

Xrdp projekt, nagu ka eelmine, on väga väike ja sisaldab ligikaudu 80 tuhat rida.

rdesktopi ja xrdp kontrollimine PVS-Studio analüsaatori abil

Veel kirjavigu

V525 Kood sisaldab sarnaste plokkide kogumit. Märkige üksused "r", "g", "r" ridadel 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++;
      }
      ....
  }
  ....
}

See kood võeti librfxcodeci teegist, mis rakendab RemoteFX-i jaoks kodekit jpeg2000. Ilmselt on siin graafilised andmekanalid segamini - "sinise" värvi asemel salvestatakse "punane". Tõenäoliselt ilmnes see viga kopeerimise ja kleepimise tulemusena.

Sama probleem ilmnes sarnase funktsiooniga rfx_encode_format_argb, mida analüsaator meile ka ütles:

V525 Kood sisaldab sarnaste plokkide kogumit. Märkige üksused "a", "r", "g", "r" ridadel 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++;
}

Massiivi deklaratsioon

V557 Massiivi ületamine on võimalik. Indeksi "i — 8" väärtus võib ulatuda 129-ni. 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];
    ....
  }
  ....
}

Massiivi deklaratsioon ja määratlus nendes kahes failis ei ühildu – suurus erineb 1 võrra. Siiski vigu ei esine – õige suurus on määratud failis evdev-map.c, seega pole piiridest väljumist. Nii et see on lihtsalt viga, mida saab kergesti parandada.

Vale võrdlus

V560 Osa tingimusavaldisest on alati väär: (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))
  {
    ....
  }
  ....
}

Funktsioon loeb tüübimuutujat allkirjastamata lühike muutujaks nagu int. Kontrollimine pole siin vajalik, kuna loeme märgita muutujat ja omistame tulemuse suuremale muutujale, mistõttu muutuja ei saa võtta negatiivset väärtust.

Tarbetud kontrollid

V560 Osa tingimusavaldisest on alati tõene: (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;
  }
  ....
}

Ebavõrdsuse kontrollimisel pole siin mõtet, kuna meil on juba alguses võrdlus olemas. Tõenäoliselt on tegemist kirjaveaga ja arendaja soovis kasutada operaatorit || kehtetute argumentide välja filtreerimiseks.

Järeldus

Auditi käigus tõsiseid vigu ei tuvastatud, küll aga leiti palju puudusi. Neid kujundusi kasutatakse aga paljudes süsteemides, kuigi nende ulatus on väike. Väikeses projektis ei pruugi olla palju vigu, seega ei tohiks analüsaatori jõudlust hinnata ainult väikeste projektide puhul. Lisateavet selle kohta saate lugeda artiklist "Tunded, mida kinnitasid numbrid"

Meilt saate alla laadida PVS-Studio prooviversiooni veebisait.

rdesktopi ja xrdp kontrollimine PVS-Studio analüsaatori abil

Kui soovite seda artiklit inglise keelt kõneleva vaatajaskonnaga jagada, kasutage tõlkelinki: Sergey Larin. Rdesktopi ja xrdp kontrollimine PVS-Studioga

Allikas: www.habr.com

Ostke DDoS-kaitsega saitide jaoks usaldusväärne hostimine, VPS VDS-serverid 🔥 Osta usaldusväärne veebimajutus DDoS-kaitsega, VPS VDS serverid | ProHoster