Rdesktop un xrdp pārbaude, izmantojot PVS-Studio analizatoru

ļ»æRdesktop un xrdp pārbaude, izmantojot PVS-Studio analizatoru
Å is ir otrais apskats rakstu sērijā par atvērtā pirmkoda programmu testÄ“Å”anu darbam ar RDP protokolu. Tajā mēs apskatÄ«sim rdesktop klientu un xrdp serveri.

Izmanto kā rīku kļūdu noteikŔanai PVS-studija. Tas ir statisks kodu analizators C, C++, C# un Java valodām, kas pieejams Windows, Linux un macOS platformās.

Rakstā ir parādÄ«tas tikai tās kļūdas, kuras man Ŕķita interesantas. Tomēr projekti ir mazi, tāpēc kļūdu bija maz :).

PiezÄ«me. IepriekŔējais raksts par FreeRDP projekta verifikāciju ir atrodams Å”eit.

rdesktop

rdesktop ā€” bezmaksas RDP klienta ievieÅ”ana sistēmām, kuru pamatā ir UNIX. To var izmantot arÄ« operētājsistēmā Windows, ja projektu veidojat programmā Cygwin. Licencēts saskaņā ar GPLv3.

Å is klients ir ļoti populārs ā€” tas tiek izmantots pēc noklusējuma ReactOS, un tam var atrast arÄ« treÅ”o puÅ”u grafiskos priekÅ”galus. Tomēr viņŔ ir diezgan vecs: viņa pirmā atbrÄ«voÅ”ana notika 4. gada 2001. aprÄ«lÄ« - rakstÄ«Å”anas laikā viņam ir 17 gadi.

Kā jau minēju iepriekÅ”, projekts ir diezgan mazs. Tajā ir aptuveni 30 tÅ«kstoÅ”i koda rindiņu, kas, ņemot vērā tā vecumu, ir nedaudz dÄ«vaini. SalÄ«dzinājumam, FreeRDP satur 320 tÅ«kstoÅ”us lÄ«niju. Å eit ir Cloc programmas izvade:

ļ»æRdesktop un xrdp pārbaude, izmantojot PVS-Studio analizatoru

Nesasniedzams kods

V779 Konstatēts nepieejams kods. Iespējams, ka ir radusies kļūda. 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);
}

Kļūda nekavējoties tiek parādÄ«ta funkcijā galvenais: mēs redzam, ka kods nāk aiz operatora atgrieÅ”anās ā€” Å”is fragments veic atmiņas tÄ«rÄ«Å”anu. Tomēr kļūda nerada draudus: visu pieŔķirto atmiņu operētājsistēma notÄ«rÄ«s pēc programmas izieÅ”anas.

Nav kļūdu apstrādes

V557 Ir iespējama masÄ«va nolaupÄ«Å”ana. Indeksa "n" vērtÄ«ba varētu sasniegt -1. rdesktop.c 1872. gads

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

Koda fragments Å”ajā gadÄ«jumā tiek nolasÄ«ts no faila buferÄ«, lÄ«dz fails beidzas. Tomēr Å”eit nav kļūdu apstrādes: ja kaut kas noiet greizi, tad lasÄ«t atgriezÄ«s -1, un tad masÄ«vs tiks pārsniegts produkcija.

EOF izmantoŔana char veidā

V739 EOF nevajadzētu salÄ«dzināt ar ā€œcharā€ tipa vērtÄ«bu. ā€œ(c = fgetc(fp))ā€ ir jābÅ«t ā€œintā€ veidam. 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++;
  }
  ....
}

Šeit mēs redzam nepareizu apstrādi, sasniedzot faila beigas: if fgetc atgriež rakstzīmi, kuras kods ir 0xFF, tas tiks interpretēts kā faila beigas (EOF).

EOF tā ir konstante, ko parasti definē kā -1. Piemēram, CP1251 kodējumā krievu alfabēta pēdējam burtam ir kods 0xFF, kas atbilst skaitlim -1, ja mēs runājam par mainÄ«go, piemēram, tvertne. Izrādās, ka simbols 0xFF, piemēram EOF (-1) tiek interpretēts kā faila beigas. Lai izvairÄ«tos no Ŕādām kļūdām, funkcijas rezultāts ir fgetc jāsaglabā tādā mainÄ«gā kā int.

Drukas kļūdas

1. fragments

V547 Izteiksme ā€œwrite_timeā€ vienmēr ir nepatiesa. disks.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; // <=
  ....
}

Iespējams, Ŕī koda autors ir kļūdÄ«jies || Šø && stāvoklÄ«. ApskatÄ«sim iespējamos vērtÄ«bu variantus rakstÄ«Å”anas_laiks Šø maiņas_laiks:

  • Abi mainÄ«gie ir vienādi ar 0: Å”ajā gadÄ«jumā mēs nonāksim zarā cits: mainÄ«gs mod_time vienmēr bÅ«s 0 neatkarÄ«gi no turpmākā nosacÄ«juma.
  • Viens no mainÄ«gajiem ir 0: mod_time bÅ«s vienāds ar 0 (ar nosacÄ«jumu, ka otram mainÄ«gajam ir nenegatÄ«va vērtÄ«ba), jo MIN izvēlēsies mazāko no divām iespējām.
  • Abi mainÄ«gie nav vienādi ar 0: izvēlieties minimālo vērtÄ«bu.

Nomainot nosacÄ«jumu ar rakstÄ«Å”anas_laiks un maiņas_laiks uzvedÄ«ba izskatÄ«sies pareizi:

  • Viens vai abi mainÄ«gie nav vienādi ar 0: izvēlieties vērtÄ«bu, kas nav nulle.
  • Abi mainÄ«gie nav vienādi ar 0: izvēlieties minimālo vērtÄ«bu.

2. fragments

V547 Izteiksme vienmēr ir patiesa. DroÅ”i vien Å”eit ir jāizmanto operators '&&'. disks.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;
  ....
}

AcÄ«mredzot arÄ« Å”eit ir sajaukti operatori || Šø &&Vai == Šø !=: mainÄ«gajam nevar vienlaikus bÅ«t vērtÄ«ba 20 un 9.

Neierobežota rindu kopÄ“Å”ana

V512 Funkcijas "sprintf" izsaukŔana novedīs pie bufera "fullpath" pārpildes. disks.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);
  ....
}

Apskatot funkciju pilnÄ«bā, kļūs skaidrs, ka Å”is kods problēmas nerada. Tomēr tie var rasties nākotnē: viena neuzmanÄ«ga maiņa, un mēs iegÅ«sim bufera pārplÅ«di - sprints nekas neierobežo, tāpēc, savienojot ceļus, mēs varam iziet ārpus masÄ«va robežām. Å o aicinājumu ieteicams pamanÄ«t snprintf(pilns ceļŔ, PATH_MAX,ā€¦.).

Lieks stāvoklis

V560 Nosacītā izteiksmes daļa vienmēr ir patiesa: add > 0. scard.c 507

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

ŠŸŃ€Š¾Š²ŠµŃ€ŠŗŠ° pievienot > 0 Å”eit nav vajadzÄ«bas: mainÄ«gais vienmēr bÅ«s lielāks par nulli, jo lasÄ«t % 4 atgriezÄ«s atlikuÅ”o dalÄ«juma daļu, bet tas nekad nebÅ«s vienāds ar 4.

xrdp

xrdp ā€” LAP servera ar atvērtā pirmkoda ievieÅ”ana. Projekts ir sadalÄ«ts 2 daļās:

  • xrdp - protokola ievieÅ”ana. IzplatÄ«ts saskaņā ar Apache 2.0 licenci.
  • xorgxrdp ā€” Xorg draiveru komplekts lietoÅ”anai ar xrdp. Licence ā€” X11 (piemēram, MIT, bet aizliedz izmantot reklāmā)

Projekta izstrādes pamatā ir rdesktop un FreeRDP rezultāti. Sākotnēji, lai strādātu ar grafiku, bija jāizmanto atseviŔķs VNC serveris vai Ä«paÅ”s X11 serveris ar RDP atbalstu - X11rdp, taču lÄ«dz ar xorgxrdp parādÄ«Å”anos nepiecieÅ”amÄ«ba pēc tiem zuda.

Šajā rakstā mēs neaplūkosim xorgxrdp.

Xrdp projekts, tāpat kā iepriekŔējais, ir ļoti mazs un satur aptuveni 80 tÅ«kstoÅ”us rindu.

ļ»æRdesktop un xrdp pārbaude, izmantojot PVS-Studio analizatoru

Vairāk drukas kļūdu

V525 Kods satur lÄ«dzÄ«gu bloku kolekciju. AtzÄ«mējiet vienumus ā€œrā€, ā€œgā€, ā€œrā€ 87., 88., 89. rindā. 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++;
      }
      ....
  }
  ....
}

Å is kods tika ņemts no librfxcodec bibliotēkas, kas ievieÅ” jpeg2000 kodeku RemoteFX. Å eit acÄ«mredzot ir sajaukti grafiskie datu kanāli - ā€œzilāsā€ krāsas vietā tiek ierakstÄ«ts ā€œsarkansā€. Å Ä« kļūda, visticamāk, radās kopÄ“Å”anas un ielÄ«mÄ“Å”anas rezultātā.

Tāda pati problēma radās līdzīgā funkcijā rfx_encode_format_argb, ko analizators mums arī teica:

V525 Kods satur lÄ«dzÄ«gu bloku kolekciju. AtzÄ«mējiet vienumus ā€œaā€, ā€œrā€, ā€œgā€, ā€œrā€ 260., 261., 262., 263. rindā. rfxencode_rgb_to_yuv.c 260

while (x < 64)
{
    *la_buf++ = a;
    *lr_buf++ = r;
    *lg_buf++ = g;
    *lb_buf++ = r;
    x++;
}

Masīva deklarācija

V557 Ir iespējama masÄ«va pārtēriņa. Indeksa 'i ā€” 8' vērtÄ«ba varētu sasniegt 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];
    ....
  }
  ....
}

MasÄ«va deklarācija un definÄ«cija Å”ajos divos failos nav savietojama - izmērs atŔķiras par 1. Tomēr kļūdas nenotiek - pareizais izmērs ir norādÄ«ts failā evdev-map.c, tāpēc nav robežu. Tātad Ŕī ir tikai kļūda, kuru var viegli novērst.

Nepareizs salīdzinājums

V560 Nosacītā izteiksmes daļa vienmēr ir nepatiesa: (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 nolasa tipa mainÄ«go neparakstÄ«ts Ä«ss mainÄ«gajā kā int. Pārbaude Å”eit nav nepiecieÅ”ama, jo mēs lasām neparakstÄ«tu mainÄ«go un pieŔķiram rezultātu lielākam mainÄ«gajam, tāpēc mainÄ«gais nevar iegÅ«t negatÄ«vu vērtÄ«bu.

Nevajadzīgas pārbaudes

V560 Nosacītā izteiksmes daļa vienmēr ir patiesa: (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;
  }
  ....
}

NevienlÄ«dzÄ«bas pārbaudēm Å”eit nav jēgas, jo mums jau ir salÄ«dzinājums sākumā. Iespējams, ka tā ir drukas kļūda, un izstrādātājs vēlējās izmantot operatoru || lai filtrētu nederÄ«gos argumentus.

Secinājums

RevÄ«zijas laikā nopietnas kļūdas netika konstatētas, taču tika konstatētas daudzas nepilnÄ«bas. Tomēr Ŕīs konstrukcijas tiek izmantotas daudzās sistēmās, lai gan tās ir nelielas. Nelielam projektam ne vienmēr ir daudz kļūdu, tāpēc nevajadzētu spriest par analizatora veiktspēju tikai mazos projektos. Vairāk par to varat lasÄ«t rakstā "SajÅ«tas, kuras apstiprināja cipari".

No mums varat lejupielādēt PVS-Studio izmēģinājuma versiju TieÅ”saistē.

ļ»æRdesktop un xrdp pārbaude, izmantojot PVS-Studio analizatoru

Ja vēlaties dalÄ«ties ar Å”o rakstu ar angliski runājoÅ”u auditoriju, lÅ«dzu, izmantojiet tulkoÅ”anas saiti: Sergejs Larins. Rdesktop un xrdp pārbaude, izmantojot PVS-Studio

Avots: www.habr.com

Pievieno komentāru