ກຳລັງກວດສອບ rdesktop ແລະ xrdp ໂດຍໃຊ້ເຄື່ອງວິເຄາະ PVS-Studio

ນີ້ແມ່ນການທົບທວນຄືນທີສອງໃນຊຸດຂອງບົດຄວາມກ່ຽວກັບການທົດສອບໂຄງການ open source ສໍາລັບການເຮັດວຽກກັບ RDP protocol. ໃນມັນພວກເຮົາຈະເບິ່ງລູກຄ້າ rdesktop ແລະເຄື່ອງແມ່ຂ່າຍ xrdp.

ໃຊ້ເປັນເຄື່ອງມືເພື່ອກໍານົດຄວາມຜິດພາດ PVS Studioມັນເປັນໂປຣແກຣມວິເຄາະລະຫັດແບບຄົງທີ່ສຳລັບ C, C++, C# ແລະ Java, ທີ່ມີຢູ່ໃນແພລດຟອມຕ່າງໆ. Windows, Linux и macOS.

ບົດຄວາມສະເຫນີພຽງແຕ່ຄວາມຜິດພາດທີ່ເບິ່ງຄືວ່າຫນ້າສົນໃຈກັບຂ້ອຍ. ຢ່າງໃດກໍຕາມ, ໂຄງການແມ່ນມີຂະຫນາດນ້ອຍ, ດັ່ງນັ້ນມີຄວາມຜິດພາດຫນ້ອຍ :).

Примечание. ບົດຄວາມທີ່ຜ່ານມາກ່ຽວກັບການກວດສອບໂຄງການ FreeRDP ສາມາດພົບໄດ້ ທີ່ນີ້.

ຄອມພິວເຕີຕັ້ງໂຕະ

ຄອມພິວເຕີຕັ້ງໂຕະ — свободная реализация клиента RDP для UNIX-based систем. Его также можно использовать и под 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 Array underrun ແມ່ນເປັນໄປໄດ້. ມູນຄ່າຂອງດັດຊະນີ '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);
  }
  ....
}

ຂໍ້ມູນຫຍໍ້ຂອງລະຫັດໃນກໍລະນີນີ້ອ່ານຈາກໄຟລ໌ເຂົ້າໄປໃນ buffer ຈົນກ່ວາໄຟລ໌ສິ້ນສຸດລົງ. ຢ່າງໃດກໍຕາມ, ບໍ່ມີຄວາມຜິດພາດໃນການຈັດການນີ້: ຖ້າບາງສິ່ງບາງຢ່າງຜິດພາດ, ຫຼັງຈາກນັ້ນ ອ່ານ ຈະກັບຄືນ -1, ແລະຫຼັງຈາກນັ້ນ array ຈະຖືກ overrun output.

ການນໍາໃຊ້ 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++;
  }
  ....
}

ໃນທີ່ນີ້ພວກເຮົາເຫັນການຈັດການທີ່ບໍ່ຖືກຕ້ອງຂອງການໄປເຖິງຈຸດສິ້ນສຸດຂອງໄຟລ໌: ຖ້າ fgetc ສົ່ງ​ຄືນ​ຕົວ​ອັກ​ສອນ​ທີ່​ມີ​ລະ​ຫັດ​ແມ່ນ 0xFF​, ມັນ​ຈະ​ຖືກ​ຕີ​ລາ​ຄາ​ເປັນ​ຕອນ​ທ້າຍ​ຂອງ​ໄຟລ​໌ (EOF).

EOF ມັນເປັນຄ່າຄົງທີ່, ໂດຍປົກກະຕິແມ່ນກໍານົດເປັນ -1. ຕົວຢ່າງ, ໃນການເຂົ້າລະຫັດ CP1251, ຕົວອັກສອນສຸດທ້າຍຂອງຕົວອັກສອນລັດເຊຍມີລະຫັດ 0xFF, ເຊິ່ງກົງກັບຕົວເລກ -1 ຖ້າພວກເຮົາເວົ້າກ່ຽວກັບຕົວແປເຊັ່ນ: char. ມັນ turns ໃຫ້​ເຫັນ​ວ່າ​ສັນ​ຍາ​ລັກ 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: ເລືອກຄ່າທີ່ບໍ່ແມ່ນສູນ.
• ທັງສອງຕົວແປບໍ່ເທົ່າກັບ 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' ຈະນໍາໄປສູ່ການລົ້ນຂອງ buffer '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);
  ....
}

ເມື່ອທ່ານເບິ່ງຫນ້າທີ່ຢ່າງເຕັມທີ່, ມັນຈະກາຍເປັນທີ່ຊັດເຈນວ່າລະຫັດນີ້ບໍ່ເຮັດໃຫ້ເກີດບັນຫາ. ຢ່າງໃດກໍຕາມ, ພວກເຂົາເຈົ້າອາດຈະເກີດຂຶ້ນໃນອະນາຄົດ: ການປ່ຽນແປງທີ່ບໍ່ສົນໃຈຫນຶ່ງແລະພວກເຮົາຈະໄດ້ຮັບການ overflow buffer - ແລ່ນ ບໍ່ໄດ້ຖືກຈໍາກັດໂດຍສິ່ງໃດ, ດັ່ງນັ້ນໃນເວລາທີ່ concatenating ເສັ້ນທາງພວກເຮົາສາມາດໄປເກີນຂອບເຂດຂອງ array ໄດ້. ມັນແນະນໍາໃຫ້ສັງເກດເຫັນການໂທນີ້ຢູ່ໃນ 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 ທີ່ມີລະຫັດແຫຼ່ງເປີດ. ໂຄງການດັ່ງກ່າວແບ່ງອອກເປັນ 2 ພາກສ່ວນຄື:

• xrdp - ການປະຕິບັດພິທີການ. ແຈກຢາຍພາຍໃຕ້ໃບອະນຸຍາດ Apache 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++;
}

ປະກາດ Array

V557 ການ overrun Array ແມ່ນເປັນໄປໄດ້. ຄ່າຂອງດັດຊະນີ '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];
    ....
  }
  ....
}

ການປະກາດແລະຄໍານິຍາມຂອງ array ໃນສອງໄຟລ໌ນີ້ແມ່ນບໍ່ເຂົ້າກັນ - ຂະຫນາດແຕກຕ່າງກັນໂດຍ 1. ຢ່າງໃດກໍຕາມ, ບໍ່ມີຂໍ້ຜິດພາດເກີດຂຶ້ນ - ຂະຫນາດທີ່ຖືກຕ້ອງແມ່ນລະບຸໄວ້ໃນໄຟລ໌ evdev-map.c, ດັ່ງນັ້ນບໍ່ມີຂອບເຂດ. ດັ່ງນັ້ນ, ນີ້ແມ່ນພຽງແຕ່ bug ທີ່ສາມາດແກ້ໄຂໄດ້ຢ່າງງ່າຍດາຍ.

ການປຽບທຽບບໍ່ຖືກຕ້ອງ

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 ຈາກພວກເຮົາ site.

ຖ້າທ່ານຕ້ອງການແບ່ງປັນບົດຄວາມນີ້ກັບຜູ້ຊົມທີ່ເວົ້າພາສາອັງກິດ, ກະລຸນາໃຊ້ການເຊື່ອມຕໍ່ການແປພາສາ: Sergey Larin. ກຳລັງກວດສອບ rdesktop ແລະ xrdp ດ້ວຍ PVS-Studio

ແຫຼ່ງຂໍ້ມູນ: www.habr.com