ProHoster > Blog > Ma'muriyat > PVS-Studio analizatori yordamida rdesktop va xrdp tekshirilmoqda

PVS-Studio analizatori yordamida rdesktop va xrdp tekshirilmoqda

PVS-Studio analizatori yordamida rdesktop va xrdp ni tekshirish
Bu RDP protokoli bilan ishlash uchun ochiq kodli dasturlarni sinovdan o'tkazish bo'yicha bir qator maqolalardagi ikkinchi sharh. Unda biz rdesktop mijozi va xrdp serverini ko'rib chiqamiz.

Xatolarni aniqlash uchun vosita sifatida ishlatiladi PVS-studiyasi. Bu Windows, Linux va macOS platformalarida mavjud boʻlgan C, C++, C# va Java tillari uchun statik kod analizatoridir.

Maqolada faqat men uchun qiziqarli bo'lib tuyulgan xatolar keltirilgan. Biroq, loyihalar kichik, shuning uchun kam xatolar bor edi :).

nota. FreeRDP loyihasini tekshirish haqidagi oldingi maqolani topish mumkin shu yerda.

ish stoli

ish stoli — UNIX asosidagi tizimlar uchun RDP mijozining bepul amalga oshirilishi. Agar siz loyihani Cygwin ostida qursangiz, undan Windows ostida ham foydalanish mumkin. GPLv3 ostida litsenziyalangan.

Ushbu mijoz juda mashhur - u sukut bo'yicha ReactOS-da qo'llaniladi va siz uning uchun uchinchi tomon grafik old qismlarini ham topishingiz mumkin. Biroq, u ancha keksa: uning birinchi chiqishi 4 yil 2001 aprelda bo'lib o'tdi - yozish paytida u 17 yoshda.

Yuqorida aytib o'tganimdek, loyiha juda kichik. U taxminan 30 ming qator kodni o'z ichiga oladi, bu uning yoshini hisobga olgan holda biroz g'alati. Taqqoslash uchun, FreeRDP 320 ming qatorni o'z ichiga oladi. Mana Cloc dasturining chiqishi:

PVS-Studio analizatori yordamida rdesktop va xrdp ni tekshirish

Urib bo'lmaydigan kod

V779 Kod mavjud emasligi aniqlandi. Ehtimol, xatolik mavjud. 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);
}

Xato bizni funksiyada darhol uchratadi asosiy: operatordan keyin keladigan kodni ko'ramiz Qaytish — bu fragment xotirani tozalashni amalga oshiradi. Biroq, xato xavf tug'dirmaydi: dastur tugagandan so'ng barcha ajratilgan xotira operatsion tizim tomonidan o'chiriladi.

Hech qanday xatolik yo'q

V557 Massivning kamayishi mumkin. "n" indeksining qiymati -1 ga yetishi mumkin. 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);
  }
  ....
}

Bu holda kod parchasi fayldan buferga fayl tugaguncha o'qiydi. Biroq, bu erda hech qanday xatolik yo'q: agar biror narsa noto'g'ri bo'lsa, unda o'qib -1 ni qaytaradi va keyin massiv to'ldiriladi Chiqdi.

Belgilar turida EOF dan foydalanish

V739 EOF ni "char" turidagi qiymat bilan solishtirmaslik kerak. "(c = fgetc(fp))" "int" turida bo'lishi kerak. 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++;
  }
  ....
}

Bu erda faylning oxirigacha noto'g'ri ishlov berishni ko'ramiz: if fgetc kodi 0xFF bo'lgan belgini qaytaradi, u faylning oxiri sifatida talqin qilinadi (EOF).

EOF u doimiy, odatda -1 sifatida aniqlanadi. Masalan, CP1251 kodlashda rus alifbosining oxirgi harfida 0xFF kodi mavjud bo'lib, agar biz o'zgaruvchi haqida gapiradigan bo'lsak -1 raqamiga to'g'ri keladi. char. Ma'lum bo'lishicha, 0xFF belgisi kabi EOF (-1) faylning oxiri sifatida talqin qilinadi. Bunday xatolardan qochish uchun funktsiyaning natijasi fgetc kabi o'zgaruvchida saqlanishi kerak int.

Yozuvlar

1-qism

V547 "Write_time" iborasi har doim noto'g'ri. 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; // <=
  ....
}

Ehtimol, ushbu kod muallifi noto'g'ri tushungan || и && holatda. Keling, qiymatlar uchun mumkin bo'lgan variantlarni ko'rib chiqaylik yozish_vaqti и vaqtni o'zgartirish:

  • Ikkala o'zgaruvchi ham 0 ga teng: bu holda biz filialga tushamiz yana boshqa: o'zgaruvchan mod_time keyingi shartdan qat'iy nazar har doim 0 bo'ladi.
  • O'zgaruvchilardan biri 0: mod_time 0 ga teng bo'ladi (boshqa o'zgaruvchining manfiy bo'lmagan qiymatga ega bo'lishi sharti bilan), chunki MIN ikkita variantdan kichigini tanlaydi.
  • Ikkala o'zgaruvchi ham 0 ga teng emas: minimal qiymatni tanlang.

Shartni bilan almashtirganda yozish_vaqt && vaqtni o'zgartirish xatti-harakatlar to'g'ri ko'rinadi:

  • Bir yoki ikkala o'zgaruvchi 0 ga teng emas: nolga teng bo'lmagan qiymatni tanlang.
  • Ikkala o'zgaruvchi ham 0 ga teng emas: minimal qiymatni tanlang.

2-qism

V547 Ifoda har doim haqiqatdir. Ehtimol, bu erda "&&" operatoridan foydalanish kerak. 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;
  ....
}

Bu yerda ham operatorlar aralashib ketgan shekilli || и &&, yoki == и !=: O'zgaruvchi bir vaqtning o'zida 20 va 9 qiymatiga ega bo'lishi mumkin emas.

Cheksiz qatorni nusxalash

V512 "Sprintf" funksiyasining chaqiruvi "to'liq yo'l" buferining to'lib ketishiga olib keladi. 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);
  ....
}

Funktsiyani to'liq ko'rib chiqsangiz, ushbu kod muammoga olib kelmasligi aniq bo'ladi. Biroq, ular kelajakda paydo bo'lishi mumkin: bir beparvo o'zgarish va biz bufer to'lib ketishini olamiz - sprint hech narsa bilan cheklanmagan, shuning uchun yo'llarni birlashtirganda biz massiv chegaralaridan tashqariga chiqishimiz mumkin. Ushbu qo'ng'iroqqa e'tibor berish tavsiya etiladi snprintf (to‘liq yo‘l, PATH_MAX, ….).

Ortiqcha holat

V560 Shartli ifodaning bir qismi har doim to'g'ri bo'ladi: qo'shish > 0. scard.c 507

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

tekshirish qo'shish > 0 bu erda hojat yo'q: o'zgaruvchi har doim noldan katta bo'ladi, chunki o'qish % 4 bo'linishning qolgan qismini qaytaradi, lekin u hech qachon 4 ga teng bo'lmaydi.

xrdp

xrdp — ochiq kodli RDP serverini amalga oshirish. Loyiha 2 qismga bo'lingan:

  • xrdp - protokolni amalga oshirish. Apache 2.0 litsenziyasi ostida tarqatiladi.
  • xorgxrdp - xrdp bilan foydalanish uchun Xorg drayverlari to'plami. Litsenziya - X11 (masalan, MIT, lekin reklamada foydalanishni taqiqlaydi)

Loyihani ishlab chiqish rdesktop va FreeRDP natijalariga asoslangan. Dastlab, grafikalar bilan ishlash uchun alohida VNC serveridan yoki RDP qo'llab-quvvatlanadigan maxsus X11 serveridan - X11rdp dan foydalanish kerak edi, lekin xorgxrdp paydo bo'lishi bilan ularga ehtiyoj yo'qoldi.

Ushbu maqolada biz xorgxrdp-ni ko'rib chiqmaymiz.

Xrdp loyihasi, oldingi kabi, juda kichik va taxminan 80 ming qatorni o'z ichiga oladi.

PVS-Studio analizatori yordamida rdesktop va xrdp ni tekshirish

Ko'proq imlo xatolari

V525 Kod shunga o'xshash bloklar to'plamini o'z ichiga oladi. 87, 88, 89-qatorlardagi 'r', 'g', 'r' bandlarini tekshiring. 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++;
      }
      ....
  }
  ....
}

Ushbu kod RemoteFX uchun jpeg2000 kodekni amalga oshiradigan librfxcodec kutubxonasidan olingan. Bu erda, ehtimol, grafik ma'lumotlar kanallari aralashtiriladi - "ko'k" rang o'rniga "qizil" yoziladi. Ushbu xato, ehtimol, nusxa ko'chirish va joylashtirish natijasida paydo bo'lgan.

Xuddi shu muammo shunga o'xshash funktsiyada yuz berdi rfx_encode_format_argb, buni analizator bizga ham aytdi:

V525 Kod shunga o'xshash bloklar to'plamini o'z ichiga oladi. 260, 261, 262, 263-qatorlardagi 'a', 'r', 'g', 'r' bandlarini tekshiring. rfxencode_rgb_to_yuv.c 260

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

Massiv deklaratsiyasi

V557 Massivning oshib ketishi mumkin. "i — 8" indeksining qiymati 129 ga yetishi mumkin. 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];
    ....
  }
  ....
}

Bu ikki fayldagi massivning deklaratsiyasi va ta'rifi mos kelmaydi - o'lcham 1 ga farq qiladi. Biroq, hech qanday xatolik yuzaga kelmaydi - evdev-map.c faylida to'g'ri o'lcham ko'rsatilgan, shuning uchun chegaradan tashqarida bo'lmaydi. Demak, bu osonlikcha tuzatilishi mumkin bo'lgan xato.

Noto'g'ri taqqoslash

V560 Shartli ifodaning bir qismi har doim noto'g'ri bo'ladi: (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))
  {
    ....
  }
  ....
}

Funktsiya turdagi o'zgaruvchini o'qiydi imzosiz kalta kabi o'zgaruvchiga aylanadi int. Bu erda tekshirish kerak emas, chunki biz imzosiz o'zgaruvchini o'qiymiz va natijani kattaroq o'zgaruvchiga tayinlaymiz, shuning uchun o'zgaruvchi manfiy qiymat qabul qila olmaydi.

Keraksiz tekshiruvlar

V560 Shartli ifodaning bir qismi har doim to'g'ri bo'ladi: (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;
  }
  ....
}

Tengsizlikni tekshirish bu erda mantiqiy emas, chunki bizda boshida taqqoslash bor. Ehtimol, bu xato va ishlab chiquvchi operatordan foydalanmoqchi bo'lgan || noto'g'ri dalillarni filtrlash uchun.

xulosa

Tekshiruv davomida jiddiy xatolar aniqlanmadi, biroq ko‘plab kamchiliklar aniqlandi. Biroq, bu dizaynlar kichik hajmga ega bo'lsa-da, ko'plab tizimlarda qo'llaniladi. Kichkina loyihada ko'p xatolar bo'lishi shart emas, shuning uchun siz analizatorning ishlashini faqat kichik loyihalarda baholamasligingiz kerak. Bu haqda ko'proq maqolada o'qishingiz mumkin "Raqamlar bilan tasdiqlangan his-tuyg'ular".

PVS-Studio ning sinov versiyasini bizdan yuklab olishingiz mumkin сайт.

PVS-Studio analizatori yordamida rdesktop va xrdp ni tekshirish

Agar siz ushbu maqolani ingliz tilida so'zlashuvchi auditoriya bilan baham ko'rmoqchi bo'lsangiz, tarjima havolasidan foydalaning: Sergey Larin. PVS-Studio yordamida rdesktop va xrdp tekshirilmoqda

Manba: www.habr.com

a Izoh qo'shish