بررسی rdesktop و xrdp با استفاده از تحلیلگر PVS-Studio

این دومین بررسی از سری مقالات در مورد آزمایش برنامه های منبع باز برای کار با پروتکل RDP است. در آن ما به مشتری rdesktop و سرور xrdp نگاه خواهیم کرد.

به عنوان ابزاری برای شناسایی خطاها استفاده می شود PVS Studio. این یک تحلیلگر کد استاتیک برای زبان های C، C++، C# و جاوا است که بر روی پلتفرم های ویندوز، لینوکس و macOS موجود است.

مقاله فقط خطاهایی را ارائه می دهد که برای من جالب به نظر می رسید. با این حال، پروژه ها کوچک هستند، بنابراین اشتباهات کمی وجود داشت :).

یادداشت. مقاله قبلی در مورد تأیید پروژه FreeRDP را می توان یافت اینجا.

دسکتاپ

دسکتاپ - اجرای رایگان یک سرویس گیرنده RDP برای سیستم های مبتنی بر یونیکس. اگر پروژه را تحت 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 نیستند: حداقل مقدار را انتخاب کنید.

هنگام جایگزینی شرایط با writ_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 - پیاده سازی پروتکل. تحت مجوز آپاچی 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 را از ما دانلود کنید کاربران آنلاین حاضر در سایت ".

اگر می خواهید این مقاله را با مخاطبان انگلیسی زبان به اشتراک بگذارید، لطفاً از پیوند ترجمه استفاده کنید: Sergey Larin. بررسی rdesktop و xrdp با PVS-Studio

منبع: www.habr.com