使用PVS-Studio分析仪验证rdesktop和xrdp

这是有关检查用于RDP协议的开源程序的系列文章中的第二篇评论。 在其中，我们将考虑rdesktop客户端和xrdp服务器。

PVS-Studio被用作检测错误的工具。 它是适用于C，C ++，C＃和Java的静态代码分析器，可在Windows，Linux和macOS上使用。

本文仅介绍那些对我来说似乎很有趣的错误。 但是，这些项目很小，因此几乎没有错误:)。

注意事项 关于检查FreeRDP项目的上一篇文章可以在这里找到。

桌面

rdesktop是针对基于UNIX的系统的RDP客户端的免费实现。 如果您在Cygwin下构建项目，它也可以在Windows下使用。 根据GPLv3许可。

这个客户端非常流行-它在ReactOS中默认使用，您也可以找到它的第三方图形前端。 尽管如此，他已经相当老了：第一次发布于2001年4月4日-在撰写本文时，他的年龄为17岁。

如前所述，该项目非常小。 它包含大约3万行代码，考虑到它的年龄，这有点奇怪。 为了进行比较，FreeRDP包含32万行。 这是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); } 

该错误在主函数中立即遇到我们：我们看到return语句后的代码-该片段清除了内存。 但是，该错误并不构成威胁：程序终止后，操作系统将清除所有分配的内存。

缺乏错误处理

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

在这种情况下，代码片段将从文件读取到缓冲区，直到文件结束。 但是，这里没有错误处理：如果出现问题，则read将返回-1，然后输出数组将超出数组的范围。

在char中使用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时，如果我们正在谈论char类型的变量，则俄语字母的最后一个字母的代码为0xFF，它对应于数字-1。 事实证明，字符0xFF与EOF （-1）一样，被视为文件的结尾。 为避免此类错误， fgetc函数的结果应存储在int变量中。

错别字

片段1

V547表达式“ write_time”始终为false。 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：在这种情况下，我们进入else分支：无论满足以下条件， 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'溢出。 磁盘.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条件表达式的一部分始终为true：add>0。scard.c 507

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

检查add> 0是没有用的：该变量将始终大于零，因为read％4将返回除法的余数，并且永远不会为4。

xrdp

xrdp是开源RDP服务器实现。 该项目分为两个部分：

• xrdp-协议实现。 根据Apache 2.0许可证分发。
• xorgxrdp-一组用于xrdp的Xorg驱动程序。 许可证-X11（作为MIT，但禁止在广告中使用）

项目开发基于rdesktop和FreeRDP的结果。 最初，我不得不使用单独的VNC服务器来处理图形，或者使用具有RDP支持的特殊X11服务器-X11rdp，但是随着xorgxrdp的问世，不再需要它们。

在本文中，我们将不涉及xorgxrdp。

与上一个项目一样，xrdp项目非常小，包含约8万行。

更多错别字

V525代码包含相似块的集合。 在第87、88、89行中检查项目“ r”，“ g”，“ r”。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++; } .... } .... } 

该代码取自实现jpeg2000编解码器以使RemoteFX正常工作的librfxcodec库。 显然，这里的图形数据通道是混合的-而不是“蓝色”，而是写了“红色”。 这种错误很可能是由于复制粘贴而出现的。

相同的问题落在类似的函数rfx_encode_format_argb中 ，分析器还告诉我们：

V525代码包含相似块的集合。 在第260、261、262、263行中检查项目“ a”，“ r”，“ g”，“ r”。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条件表达式的一部分始终为false：（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)) { .... } .... } 

在函数中，将unsigned short类型的变量读入int类型的变量。 这里不需要检查，因为我们读取了一个无符号类型的变量并将结果分配给一个较大的变量，因此该变量不能取负值。

不必要的检查

V560条件表达式的一部分始终为true：（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。 使用PVS-Studio检查rdesktop和xrdp