ProHoster > Блог > Администрација > Проверка на rdesktop и xrdp со помош на анализаторот PVS-Studio

Проверка на rdesktop и xrdp со помош на анализаторот PVS-Studio

Проверка на rdesktop и xrdp со помош на анализаторот PVS-Studio
Ова е втор преглед во серијата написи за тестирање на програми со отворен код за работа со протоколот RDP. Во него ќе ги разгледаме клиентот rdesktop и серверот xrdp.

Се користи како алатка за идентификување на грешки ПВС-СтудиоТоа е статички анализатор на код за C, C++, C# и Java, достапен на платформи Windows, Linux и macOS.

Во написот се претставени само оние грешки што ми изгледаа интересни. Сепак, проектите се мали, па имаше малку грешки :).

Имајте на ум. Може да се најде претходен напис за проверка на проектот FreeRDP тука.

работна површина

работна површина — свободная реализация клиента RDP для UNIX-based систем. Его также можно использовать и под Windows, если собирать проект под Cygwin. Лицензирован под GPLv3.

Овој клиент е многу популарен - стандардно се користи во ReactOS, а за него можете да најдете и графички предни делови од трети страни. Сепак, тој е прилично стар: неговото прво ослободување се случи на 4 април 2001 година - во времето на пишувањето, тој има 17 години.

Како што напоменав претходно, проектот е многу мал. Содржи приближно 30 илјади линии код, што е малку чудно со оглед на неговата старост. За споредба, FreeRDP содржи 320 илјади линии. Еве го излезот од програмата Cloc:

Проверка на rdesktop и xrdp со помош на анализаторот PVS-Studio

Недостапен код

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, а потоа низата ќе биде пречекорена излез.

Користење на 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, тој ќе се толкува како крај на датотеката (ЕОФ).

ЕОФ тоа е константа, обично дефинирана како -1. На пример, во кодирањето CP1251, последната буква од руската азбука ја има шифрата 0xFF, што одговара на бројот -1 ако зборуваме за променлива како кочија. Излегува дека симболот 0xFF, како ЕОФ (-1) се толкува како крај на датотеката. За да се избегнат ваквите грешки, резултатот од функцијата е fgetc треба да се складира во променлива како int.

Печатни грешки

Фрагмент 1

V547 Изразот „write_time“ е секогаш неточен. диск.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 Изразот е секогаш вистинит. Веројатно тука треба да се користи операторот „&&“. диск.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 - Збир на двигатели на Xorg за употреба со xrdp. Лиценца - X11 (како MIT, но забранува употреба во рекламирање)

Развојот на проектот се базира на резултатите од rdesktop и FreeRDP. Првично, за да работите со графика, требаше да користите посебен VNC сервер или специјален X11 сервер со поддршка за RDP - X11rdp, но со доаѓањето на xorgxrdp, потребата за нив исчезна.

Во оваа статија нема да го покриеме xorgxrdp.

Проектот xrdp, како и претходниот, е многу мал и содржи приближно 80 илјади линии.

Проверка на rdesktop и xrdp со помош на анализаторот PVS-Studio

Повеќе печатни грешки

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. Овде, очигледно, каналите за графички податоци се измешани - наместо „сината“ боја, се снима „црвено“. Оваа грешка најверојатно се појавила како резултат на copy-paste.

Истиот проблем се појави во слична функција 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 од нас Онлајн.

Проверка на rdesktop и xrdp со помош на анализаторот PVS-Studio

Ако сакате да ја споделите оваа статија со публика што зборува англиски, ве молиме користете ја врската за превод: Сергеј Ларин. Проверка на rdesktop и xrdp со PVS-Studio

Извор: www.habr.com

Купете доверлив хостинг за сајтови со DDoS заштита, VPS VDS сервери 🔥 Купете сигурен веб-хостинг со DDoS заштита, VPS VDS сервери | ProHoster