🥇使用 PVS-Studio 分析器检查 rdesktop 和 xrdp

这是有关测试使用 RDP 协议的开源程序的系列文章中的第二篇评论。在其中我们将了解 rdesktop 客户端和 xrdp 服务器。

用作识别错误的工具 PVS工作室。它是 C、C++、C# 和 Java 语言的静态代码分析器，可在 Windows、Linux 和 macOS 平台上使用。

这篇文章只介绍了那些我觉得有趣的错误。不过，项目很小，所以错误很少:)。

注意。之前关于FreeRDP项目验证的文章可以找到这里.

桌面

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

这个客户端非常受欢迎——ReactOS 中默认使用它，你还可以找到它的第三方图形前端。然而，他已经相当老了：他的第一次发布发生在 4 年 2001 月 17 日——在撰写本文时，他已经 XNUMX 岁了。

正如我之前指出的，该项目非常小。它包含大约 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，然后数组将溢出产量.

在 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++;
  }
  ....
}

在这里我们看到到达文件末尾的错误处理：如果 弗吉特 返回一个代码为 0xFF 的字符，它将被解释为文件结尾（EOF).

EOF 它是一个常数，通常定义为-1。例如，在 CP1251 编码中，俄语字母表的最后一个字母的代码为 0xFF，如果我们谈论像这样的变量，则对应于数字 -1 坦克。结果发现符号0xFF，就像 EOF (-1) 被解释为文件结尾。为了避免此类错误，该函数的结果是 弗吉特 应该存储在像这样的变量中 INT.

错别字

片段 1

V547 表达式“write_time”始终为 false。磁盘.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：在这种情况下，我们将进入一个分支其他：多变的 修改时间 无论后续条件如何，都将始终为 0。
其中一个变量为 0： 修改时间 将等于 0（假设另一个变量具有非负值），因为闵将选择两个选项中较小的一个。
两个变量都不等于 0：选择最小值。

将条件替换为 写入时间 && 更改时间 行为看起来是正确的：

一个或两个变量不等于 0：选择一个非零值。
两个变量都不等于 0：选择最小值。

片段 2

V547 表达式总是正确的。也许这里应该使用“&&”运算符。磁盘.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);
  ....
}

当您完整地查看该函数时，您会发现这段代码不会导致问题。然而，它们将来可能会出现：一个不小心的更改，我们就会遇到缓冲区溢出 - 冲刺不受任何限制，因此在连接路径时我们可以超越数组的边界。建议注意此电话 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 - 一组与 xrdp 一起使用的 Xorg 驱动程序。许可证 - X11（与 MIT 类似，但禁止在广告中使用）

该项目的开发基于rdesktop和FreeRDP的成果。最初，要处理图形，您必须使用单独的 VNC 服务器或支持 RDP 的特殊 X11 服务器 - X11rdp，但随着 xorgxrdp 的出现，对它们的需求消失了。

在本文中，我们不会讨论 xorgxrdp。

xrdp 项目与前一个项目一样，非常小，包含大约 80 万行。

数组声明

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。使用 PVS-Studio 检查 rdesktop 和 xrdp

来源： habr.com

使用 PVS-Studio 分析器检查 rdesktop 和 xrdp

桌面

无法访问的代码

无错误处理

在 char 类型中使用 EOF

错别字

无限行复制

冗余条件

XRDP

更多错别字

数组声明

比较不正确

不必要的检查

结论

添加评论取消回复

使用 PVS-Studio 分析器检查 rdesktop 和 xrdp

桌面

无法访问的代码

无错误处理

在 char 类型中使用 EOF

错别字

无限行复制

冗余条件

XRDP

更多错别字

数组声明

比较不正确

不必要的检查

结论

添加评论 取消回复

添加评论取消回复