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

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

تم استخدام PVS-Studio كأداة للكشف عن الأخطاء. إنه محلل ثابت للكود 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] = '\0'; // <= str_handle_lines(output, &rest, linehandler, data); } .... } 

يقرأ مقتطف الشفرة في هذه الحالة من الملف إلى المخزن المؤقت حتى ينتهي الملف. ومع ذلك ، لا يوجد خطأ في معالجة هنا: إذا حدث خطأ ما ، فستعود القراءة -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++; } .... } 

نرى هنا معالجة غير صحيحة للوصول إلى نهاية الملف: إذا أعادت 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; // <= .... } 

ربما يكون مؤلف هذا الرمز مشوشًا | و & المقدمة. ضع في اعتبارك الخيارات الممكنة لكتابة_وقت و 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); .... } 

عند التفكير في الوظيفة ، سيصبح من الواضح تمامًا أن هذا الرمز لا يسبب مشاكل. ومع ذلك ، قد تنشأ في المستقبل: تغيير واحد مهمل ، ونحصل على تجاوز سعة المخزن المؤقت - 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 - تنفيذ البروتوكول. موزعة تحت رخصة أباتشي 2.0.
• xorgxrdp - مجموعة من برامج تشغيل Xorg للاستخدام مع xrdp. ترخيص - X11 (مثل معهد ماساتشوستس للتكنولوجيا ، ولكن يحظر استخدامها في الإعلانات)

ويستند تطوير المشروع على نتائج 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