التحقق من rdesktop و xrdp باستخدام PVS-Studio

هذا هو المنشور الثاني في سلسلة مقالاتنا حول نتائج فحص البرامج مفتوحة المصدر التي تعمل مع بروتوكول RDP. اليوم سوف نلقي نظرة على عميل rdesktop وخادم xrdp.

تم إجراء التحليل بواسطة PVS-Studio . هذا محلل ثابت للرمز المكتوب بلغة C و C ++ و C # و Java ، ويتم تشغيله على أنظمة Windows و Linux و macOS.

سأناقش فقط تلك الأخطاء التي بدت أكثر إثارة للاهتمام بالنسبة لي. من ناحية أخرى ، نظرًا لأن المشاريع صغيرة جدًا ، لا يوجد الكثير من الأخطاء فيها على أي حال :).

المذكرة. المقالة السابقة حول التحقق من FreeRDP متاحة هنا .

rdesktop

rdesktop هو عميل RDP مجاني للأنظمة المستندة إلى UNIX. يمكن أيضًا تشغيله على Windows إذا تم إنشاؤه تحت Cygwin. يتم إصدار rdesktop تحت GPLv3.

هذا عميل شهير للغاية. يتم استخدامه كعميل افتراضي على ReactOS ، ويمكنك أيضًا العثور على الواجهة الرسومية الخارجية لجهة خارجية لتتوافق معها. هذا المشروع قديم إلى حد كبير ، فقد تم إصداره لأول مرة في 4 أبريل 2001 ، وكان عمره 17 عامًا ، حتى كتابة هذه السطور.

كما قلت بالفعل ، فإن المشروع صغير جدًا - حوالي 30 شركة نفط الكويت ، وهو أمر غريب بعض الشيء بالنظر إلى عمره. قارن ذلك مع FreeRDP مع 320 KLOC. وهنا إخراج كلوك:

رمز غير قابل للوصول

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] = '\0'; // <= str_handle_lines(output, &rest, linehandler, data); } .... } 

تتم قراءة محتويات الملف في المخزن المؤقت حتى يتم الوصول إلى EOF. في الوقت نفسه ، يفتقر هذا الرمز إلى آلية معالجة الأخطاء ، وإذا حدث خطأ ما ، فستعود القراءة -1 وسيبدأ التنفيذ في القراءة خارج حدود صفيف الإخراج .

باستخدام EOF في شار

لا ينبغي مقارنة 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++; } .... } 

ينفذ هذا الرمز معالجة EOF غير صحيحة: إذا كانت fgetc تُرجع حرفًا يكون رمزه 0xFF ، فسوف يتم تفسيره على أنه نهاية الملف ( EOF ).

EOF هو ثابت يُعرَّف عادةً بأنه -1. على سبيل المثال ، في تشفير CP1251 ، يتم ترميز الحرف الأخير من الأبجدية الروسية كـ 0xFF ، والذي يتوافق مع الرقم -1 في النوع char . وهذا يعني أن حرف 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; // <= .... } 

يجب أن يكون مؤلف هذا الرمز قد استخدم بالصدفة || عامل التشغيل بدلاً من && في الحالة. دعونا نرى ما هي القيم التي يمكن أن تحتوي عليها المتغيرات Write_time و change_time :

• كلا المتغيرين له 0. في هذه الحالة ، ينتقل التنفيذ إلى الفرع الآخر : سيتم دائمًا تقييم المتغير mod_time إلى 0 بغض النظر عن الحالة التالية.
• يحتوي أحد المتغيرات على 0. في هذه الحالة ، سيتم تعيين mod_time 0 (بالنظر إلى أن المتغير الآخر له قيمة غير سالبة) لأن MIN ستختار الأقل من الاثنين.
• لا يحتوي المتغير على 0: يتم اختيار الحد الأدنى للقيمة.

سيؤدي تغيير هذا الخط إلى write_time && change_time إلى إصلاح السلوك:

• متغير واحد فقط أو لا يحتوي على صفر: يتم اختيار القيمة غير الصفرية.
• لا يحتوي المتغير على 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); .... } 

إذا تمكنت من متابعة الوظيفة حتى النهاية ، فسترى أن الشفرة على ما يرام ، ولكن قد تتعطل يومًا واحدًا: سينتهي تغيير واحد فقط من الإهمال بتجاوز سعة المخزن المؤقت نظرًا لأن sprintf غير محدود بأي شكل من الأشكال ، لذلك فقد وصل يمكن أن تأخذ المسارات التنفيذ خارج حدود الصفيف. نوصي باستبدال هذه المكالمة بـ snprintf (fullpath ، 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 مفتوح المصدر. يتكون المشروع من جزأين:

• xrdp - تنفيذ البروتوكول. تم إصداره تحت Apache 2.0.
• xorgxrdp - مجموعة من برامج تشغيل Xorg ليتم استخدامها مع xrdp. تم إصداره تحت X11 (تمامًا مثل MIT ، لكن الاستخدام في الإعلانات محظور)

ويستند التطوير على rdesktop و FreeRDP. في البداية ، لكي تكون قادرًا على العمل مع الرسومات ، يجب عليك استخدام خادم VNC منفصل أو خادم X11 خاص بدعم RDP ، X11rdp ، لكن تلك التي أصبحت غير ضرورية مع إصدار xorgxrdp.

لن نتحدث عن xorgxrdp في هذه المقالة.

تمامًا مثل المشروع السابق ، يعد xrdp مشروعًا صغيرًا ، ويتألف من حوالي 80 KLOC.

المزيد من الأخطاء المطبعية

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

في ملف genkeymap.c ، يتم تعريف الصفيف بعنصر واحد أقصر من الضمني في التنفيذ. لن يحدث أي خطأ ، لأن ملف 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)) { .... } .... } 

تتم قراءة قيمة متغير النوع type غير الموقَّع في متغير type 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 متاحة على موقعنا.