rdesktop და xrdp შემოწმება PVS-Studio ანალიზატორის გამოყენებით

ეს არის მეორე მიმოხილვა სტატიების სერიის შესახებ ღია კოდის პროგრამების ტესტირების შესახებ RDP პროტოკოლთან მუშაობისთვის. მასში ჩვენ გადავხედავთ rdesktop კლიენტს და xrdp სერვერს.

გამოიყენება როგორც შეცდომების იდენტიფიცირების საშუალება PVS- სტუდია. ეს არის სტატიკური კოდის ანალიზატორი C, C++, C# და Java ენებისთვის, ხელმისაწვდომია Windows, Linux და macOS პლატფორმებზე.

სტატიაში წარმოდგენილია მხოლოდ ის შეცდომები, რომლებიც ჩემთვის საინტერესო ჩანდა. თუმცა, პროექტები მცირეა, ამიტომ შეცდომები ცოტა იყო :).

შენიშვნა. წინა სტატია FreeRDP პროექტის გადამოწმების შესახებ შეგიძლიათ იხილოთ აქ.

rdesktop

rdesktop — RDP კლიენტის უფასო განხორციელება UNIX-ზე დაფუძნებული სისტემებისთვის. ის ასევე შეიძლება გამოყენებულ იქნას Windows-ის ქვეშ, თუ თქვენ ააშენებთ პროექტს Cygwin-ის ქვეშ. ლიცენზირებულია GPLv3-ით.

ეს კლიენტი ძალიან პოპულარულია - ის ნაგულისხმევად გამოიყენება ReactOS-ში და ასევე შეგიძლიათ იპოვოთ მესამე მხარის გრაფიკული წინა ნაწილები. თუმცა, ის საკმაოდ ასაკოვანია: მისი პირველი გამოშვება შედგა 4 წლის 2001 აპრილს - წერის დროს ის 17 წლისაა.

როგორც უკვე აღვნიშნე, პროექტი ძალიან მცირეა. ის შეიცავს დაახლოებით 30 ათას ხაზს კოდს, რაც ცოტა უცნაურია მისი ასაკის გათვალისწინებით. შედარებისთვის, FreeRDP შეიცავს 320 ათას ხაზს. აქ არის Cloc პროგრამის გამომავალი:

მიუწვდომელი კოდი

V779 აღმოჩენილია მიუწვდომელი კოდი. შესაძლებელია, რომ შეცდომა იყოს. 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);
}

შეცდომა მაშინვე ხვდება ფუნქციაში მთავარი: ჩვენ ვხედავთ კოდს, რომელიც მოდის ოპერატორის შემდეგ დაბრუნების — ეს ფრაგმენტი ასრულებს მეხსიერების გაწმენდას. თუმცა, შეცდომა არ წარმოადგენს საფრთხეს: ყველა გამოყოფილი მეხსიერება გასუფთავდება ოპერაციული სისტემის მიერ პროგრამის გასვლის შემდეგ.

შეცდომების დამუშავება არ არის

V557 შესაძლებელია მასივის დაქვეითება. 'n' ინდექსის მნიშვნელობა შეიძლება მიაღწიოს -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);
  }
  ....
}

კოდის ფრაგმენტი ამ შემთხვევაში იკითხება ფაილიდან ბუფერში ფაილის დასრულებამდე. თუმცა, აქ არ არის შეცდომის დამუშავება: თუ რამე არასწორედ მოხდება, მაშინ წაკითხული დაბრუნდება -1 და შემდეგ მასივი გადაფარდება გამომავალი.

EOF-ის გამოყენება char ტიპის

V739 EOF არ უნდა შევადაროთ 'char' ტიპის მნიშვნელობას. '(c = fgetc(fp))' უნდა იყოს '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++;
  }
  ....
}

აქ ჩვენ ვხედავთ ფაილის ბოლომდე მიღწევის არასწორ დამუშავებას: if fgetc აბრუნებს სიმბოლოს, რომლის კოდია 0xFF, ის განიმარტება, როგორც ფაილის დასასრული (EOF).

EOF ეს არის მუდმივი, ჩვეულებრივ განისაზღვრება როგორც -1. მაგალითად, CP1251 კოდირებით, რუსული ანბანის ბოლო ასოს აქვს კოდი 0xFF, რომელიც შეესაბამება რიცხვს -1, თუ ვსაუბრობთ ცვლადზე, როგორიცაა სატანკო. გამოდის, რომ სიმბოლო 0xFF, მოსწონს EOF (-1) ინტერპრეტირებულია, როგორც ფაილის დასასრული. ასეთი შეცდომების თავიდან ასაცილებლად, ფუნქციის შედეგია fgetc უნდა იყოს შენახული ცვლადში, როგორიცაა int.

შეცდომების შეცდომა

ფრაგმენტი 1

V547 გამოთქმა 'write_time' ყოველთვის მცდარია. 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; // <=
  ....
}

შესაძლოა ამ კოდის ავტორმა არასწორად გაიგო || и && მდგომარეობაში. განვიხილოთ ღირებულებების შესაძლო ვარიანტები ჩაწერის_დრო и ცვლილების_დრო:

• ორივე ცვლადი ტოლია 0-ის: ამ შემთხვევაში ჩვენ აღმოვჩნდებით ტოტში სხვა: ცვლადი mod_time ყოველთვის იქნება 0, მიუხედავად შემდგომი მდგომარეობისა.
• ერთ-ერთი ცვლადი არის 0: mod_time იქნება 0-ის ტოლი (იმ პირობით, რომ სხვა ცვლადს აქვს არაუარყოფითი მნიშვნელობა), რადგან MIN აირჩევს ორი ვარიანტიდან პატარას.
• ორივე ცვლადი არ არის 0-ის ტოლი: აირჩიეთ მინიმალური მნიშვნელობა.

მდგომარეობის შეცვლისას write_time && change_time ქცევა სწორად გამოიყურება:

• ერთი ან ორივე ცვლადი არ არის 0-ის ტოლი: აირჩიეთ არანულოვანი მნიშვნელობა.
• ორივე ცვლადი არ არის 0-ის ტოლი: აირჩიეთ მინიმალური მნიშვნელობა.

ფრაგმენტი 2

V547 გამოხატვა ყოველთვის მართალია. ალბათ აქ '&&' ოპერატორი უნდა იყოს გამოყენებული. 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;
  ....
}

როგორც ჩანს, ოპერატორები აქაც აირია || и &&ან == и !=: ცვლადს არ შეიძლება ჰქონდეს ერთდროულად 20 და 9 მნიშვნელობა.

ხაზის შეუზღუდავი კოპირება

V512 'sprintf' ფუნქციის გამოძახება გამოიწვევს ბუფერის '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);
  ....
}

როდესაც ფუნქციას სრულად დაათვალიერებთ, ცხადი გახდება, რომ ეს კოდი არ იწვევს პრობლემებს. თუმცა, ისინი შეიძლება წარმოიშვას მომავალში: ერთი უყურადღებო ცვლილება და ჩვენ მივიღებთ ბუფერულ გადინებას - სპრინტი არ არის შეზღუდული არაფრით, ამიტომ ბილიკების შეერთებისას ჩვენ შეგვიძლია გასცდეთ მასივის საზღვრებს. მიზანშეწონილია შეამჩნიოთ ეს ზარი snprintf (სრული გზა, PATH_MAX, ....).

ზედმეტი მდგომარეობა

V560 პირობითი გამოხატვის ნაწილი ყოველთვის მართალია: add > 0. scard.c 507

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

Проверка დაამატეთ > 0 აქ არ არის საჭირო: ცვლადი ყოველთვის იქნება ნულზე მეტი, რადგან წაიკითხეთ % 4 დააბრუნებს გაყოფის დარჩენილ ნაწილს, მაგრამ ის არასოდეს იქნება 4-ის ტოლი.

xrdp

xrdp — RDP სერვერის დანერგვა ღია კოდით. პროექტი დაყოფილია 2 ნაწილად:

• xrdp - პროტოკოლის განხორციელება. განაწილებულია Apache 2.0 ლიცენზიით.
• xorgxrdp - Xorg დრაივერების ნაკრები xrdp-თან გამოსაყენებლად. ლიცენზია - X11 (როგორც MIT, მაგრამ კრძალავს რეკლამაში გამოყენებას)

პროექტის შემუშავება ეფუძნება rdesktop-ისა და FreeRDP-ის შედეგებს. თავდაპირველად, გრაფიკასთან მუშაობისთვის, თქვენ უნდა გამოეყენებინათ ცალკე VNC სერვერი, ან სპეციალური X11 სერვერი RDP მხარდაჭერით - X11rdp, მაგრამ xorgxrdp-ის მოსვლასთან ერთად, მათი საჭიროება გაქრა.

ამ სტატიაში ჩვენ არ გავაშუქებთ xorgxrdp.

xrdp პროექტი, ისევე როგორც წინა, ძალიან მცირეა და შეიცავს დაახლოებით 80 ათას ხაზს.

მეტი შეცდომა

V525 კოდი შეიცავს მსგავსი ბლოკების კოლექციას. შეამოწმეთ ელემენტი 'r', 'g', 'r' სტრიქონებში 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++;
      }
      ....
  }
  ....
}

ეს კოდი აღებულია librfxcodec ბიბლიოთეკიდან, რომელიც ახორციელებს jpeg2000 კოდეკს RemoteFX-ისთვის. აქ, როგორც ჩანს, გრაფიკული მონაცემთა არხები აირია - "ლურჯი" ფერის ნაცვლად, "წითელი" არის ჩაწერილი. ეს შეცდომა, სავარაუდოდ, გაჩნდა კოპირ-პასტის შედეგად.

იგივე პრობლემა წარმოიშვა მსგავს ფუნქციაში rfx_encode_format_argb, რომელიც ანალიზატორმა ასევე გვითხრა:

V525 კოდი შეიცავს მსგავსი ბლოკების კოლექციას. შეამოწმეთ ელემენტი 'a', 'r', 'g', 'r' სტრიქონებში 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++;
}

მასივის დეკლარაცია

V557 მასივის გადაფარვა შესაძლებელია. 'i — 8' ინდექსის მნიშვნელობამ შეიძლება მიაღწიოს 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];
    ....
  }
  ....
}

ამ ორ ფაილში მასივის დეკლარაცია და განმარტება შეუთავსებელია - ზომა განსხვავდება 1-ით. თუმცა, შეცდომები არ ხდება - სწორი ზომა მითითებულია evdev-map.c ფაილში, ამიტომ არ არის საზღვრები. ასე რომ, ეს მხოლოდ შეცდომაა, რომელიც ადვილად გამოსწორდება.

არასწორი შედარება

V560 პირობითი გამოხატვის ნაწილი ყოველთვის მცდარია: (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))
  {
    ....
  }
  ....
}

ფუნქცია კითხულობს ტიპის ცვლადს ხელმოუწერელი მოკლე ცვლადში, როგორიცაა int. შემოწმება აქ არ არის საჭირო, რადგან ჩვენ ვკითხულობთ ხელმოუწერელ ცვლადს და შედეგს ვანიჭებთ უფრო დიდ ცვლადს, ამიტომ ცვლადი ვერ მიიღებს უარყოფით მნიშვნელობას.

არასაჭირო შემოწმებები

V560 პირობითი გამოხატვის ნაწილი ყოველთვის მართალია: (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;
  }
  ....
}

უთანასწორობის შემოწმებას აქ აზრი არ აქვს, რადგან ჩვენ უკვე გვაქვს შედარება დასაწყისში. სავარაუდოა, რომ ეს შეცდომაა და დეველოპერს სურდა ოპერატორის გამოყენება || არასწორი არგუმენტების გაფილტვრა.

დასკვნა

აუდიტის დროს სერიოზული შეცდომები არ გამოვლენილა, მაგრამ ბევრი ხარვეზი დაფიქსირდა. თუმცა, ეს დიზაინები გამოიყენება ბევრ სისტემაში, თუმცა მცირე მოცულობით. მცირე პროექტს სულაც არ აქვს ბევრი შეცდომა, ამიტომ არ უნდა განსაჯოთ ანალიზატორის მუშაობა მხოლოდ მცირე პროექტებზე. ამის შესახებ მეტი შეგიძლიათ წაიკითხოთ სტატიაში "ნომრებით დადასტურებული გრძნობები".

ჩვენგან შეგიძლიათ ჩამოტვირთოთ PVS-Studio-ს საცდელი ვერსია ონლაინი.

თუ გსურთ გაუზიაროთ ეს სტატია ინგლისურენოვან აუდიტორიას, გთხოვთ, გამოიყენოთ თარგმანის ბმული: სერგეი ლარინი. rdesktop და xrdp შემოწმება PVS-Studio-ით

წყარო: www.habr.com