PVS-Studio анализаторунун жардамы менен rdesktop жана xrdp текшерилүүдө

PVS-Studio анализаторунун жардамы менен rdesktop жана xrdp текшерүү
Бул RDP протоколу менен иштөө үчүн ачык булак программаларын сынап көрүү жөнүндө макалалардын сериясындагы экинчи кароо. Анда биз rdesktop кардарын жана xrdp серверин карайбыз.

каталарды аныктоо үчүн курал катары колдонулат PVS-Studio. Бул Windows, Linux жана macOS платформаларында жеткиликтүү болгон C, C++, C# жана Java тилдери үчүн статикалык код анализатору.

Макалада мага кызыктуу көрүнгөн каталар гана берилген. Бирок, долбоорлор кичинекей болгондуктан, каталар аз болду :).

пикир. FreeRDP долбоорун текшерүү жөнүндө мурунку макаланы тапса болот бул жерде.

rdesktop

rdesktop — UNIX негизиндеги системалар үчүн RDP кардарын акысыз ишке ашыруу. Эгер сиз Cygwin астында долбоорду курсаңыз, аны Windows астында да колдонсо болот. GPLv3 боюнча лицензияланган.

Бул кардар абдан популярдуу - ал ReactOSто демейки боюнча колдонулат, жана сиз ал үчүн үчүнчү тараптын графикалык фронтторун да таба аласыз. Бирок, ал абдан кары: анын биринчи релиз 4-жылдын 2001-апрелинде болгон - жазуу учурунда, ал 17 жашта.

Мен жогоруда белгилегендей, долбоор абдан кичинекей. Ал болжол менен 30 миң сап кодду камтыйт, бул анын жашын эске алганда бир аз кызык. Салыштыруу үчүн, FreeRDP 320 миң сапты камтыйт. Бул жерде Cloc программасынын натыйжасы:

PVS-Studio анализаторунун жардамы менен rdesktop жана xrdp текшерүү

Жеткиликсиз код

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 кайтып келет, андан кийин массив ашып кетет продукция.

Chart түрүндөгү 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++;
  }
  ....
}

Бул жерде биз файлдын аягына чейин туура эмес иштөөнү көрөбүз: if fgetc коду 0xFF болгон символду кайтарат, ал файлдын аягы катары чечмеленет (EOF).

EOF бул туруктуу, адатта -1 катары аныкталат. Мисалы, CP1251 коддоосунда орус алфавитинин акыркы тамгасында 0xFF коду бар, ал -1 санына туура келет, эгерде биз сыяктуу өзгөрмө жөнүндө сөз кылсак. Исахар. Көрсө, 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; // <=
  ....
}

Балким, бул коддун автору туура эмес түшүнүп алгандыр || и && абалда. Келгиле, баалуулуктардын мүмкүн болгон варианттарын карап көрөлү жазуу_убакыт и өзгөртүү_убакыт:

  • Эки өзгөрмө тең 0гө барабар: бул учурда биз бутакка айланабыз дагы: өзгөрмө mod_time кийинки шартка карабастан ар дайым 0 болот.
  • Өзгөрмөлөрдүн бири 0: mod_time 0гө барабар болот (башка өзгөрмө терс эмес мааниге ээ болгон шартта), анткени MIN эки варианттын кичинесин тандайт.
  • Эки өзгөрмө тең 0гө барабар эмес: минималдуу маанини тандаңыз.

менен шартты алмаштырууда жазуу_убакыт && убакытты_өзгөртүү жүрүм-туруму туура көрүнөт:

  • Бир же эки өзгөрмө 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);
  ....
}

Функцияны толугу менен карап чыкканда, бул код көйгөй жаратпай турганы айкын болот. Бирок, алар келечекте пайда болушу мүмкүн: бир байкабай өзгөрүү жана биз буфердин толуп кетишин алабыз - спринт эч нерсе менен чектелбейт, ошондуктан жолдорду бириктиргенде массивдин чегинен чыга алабыз. Бул чакырууну байкап туруу сунушталат 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 миң сапты камтыйт.

PVS-Studio анализаторунун жардамы менен rdesktop жана xrdp текшерүү

Дагы каталар

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

Бул код RemoteFX үчүн jpeg2000 кодегин ишке ашырган 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 Шарттуу туюнтумдун бир бөлүгү дайыма жалган болот: (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 сынамык версиясын жүктөп алсаңыз болот сайты.

PVS-Studio анализаторунун жардамы менен rdesktop жана xrdp текшерүү

Эгер сиз бул макаланы англис тилдүү аудитория менен бөлүшкүңүз келсе, котормо шилтемесин колдонуңуз: Сергей Ларин. PVS-Studio менен rdesktop жана xrdp текшерилүүдө

Source: www.habr.com

Комментарий кошуу