Провера рдесктоп-а и крдп-а помоћу ПВС-Студио анализатора

Провера рдесктоп-а и крдп-а помоћу ПВС-Студио анализатора
Ово је други преглед у низу чланака о тестирању програма отвореног кода за рад са РДП протоколом. У њему ћемо погледати рдесктоп клијент и крдп сервер.

Користи се као алат за идентификацију грешака ПВС-Студио. То је статички анализатор кода за Ц, Ц++, Ц# и Јава језике, доступан на Виндовс, Линук и мацОС платформама.

У чланку су приказане само оне грешке које су ми се учиниле интересантним. Ипак, пројекти су мали, па је било мало грешака :).

Приметити. Претходни чланак о верификацији ФрееРДП пројекта може се наћи овде.

рдесктоп

рдесктоп — бесплатна имплементација РДП клијента за системе засноване на УНИКС-у. Такође се може користити под Виндовс-ом ако градите пројекат под Цигвин-ом. Лиценцирано под ГПЛв3.

Овај клијент је веома популаран - подразумевано се користи у РеацтОС-у, а за њега можете пронаћи и графичке фронт-ендове треће стране. Међутим, он је прилично стар: његово прво издање одржано је 4. априла 2001. - у време писања, он има 17 година.

Као што сам раније рекао, пројекат је прилично мали. Садржи отприлике 30 хиљада линија кода, што је мало чудно с обзиром на старост. Поређења ради, ФрееРДП садржи 320 хиљада линија. Ево излаза Цлоц програма:

Провера рдесктоп-а и крдп-а помоћу ПВС-Студио анализатора

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

ВКСНУМКС Откривен је недоступан код. Могуће је да постоји грешка. рдесктоп.ц 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);
}

Грешка нас наилази одмах у функцији главни: видимо код који долази после оператора повратак — овај фрагмент врши чишћење меморије. Међутим, грешка не представља претњу: оперативни систем ће обрисати сву додељену меморију након што програм изађе.

Нема руковања грешкама

ВКСНУМКС Могуће је подбацивање низа. Вредност 'н' индекса би могла да достигне -1. рдесктоп.ц 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, а затим ће низ бити прегажен излаз.

Коришћење ЕОФ-а у типу цхар

ВКСНУМКС ЕОФ не треба поредити са вредношћу типа „цхар“. „(ц = фгетц(фп))“ треба да буде типа „инт“. цтрл.ц 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++;
  }
  ....
}

Овде видимо нетачно руковање доласком до краја датотеке: иф фгетц враћа знак чији је код 0кФФ, он ће се тумачити као крај датотеке (ЕОФ).

ЕОФ то је константа, обично дефинисана као -1. На пример, у кодирању ЦП1251 последње слово руског алфабета има код 0кФФ, што одговара броју -1 ако говоримо о променљивој као што је Чар. Испоставило се да је симбол 0кФФ, као ЕОФ (-1) се тумачи као крај датотеке. Да би се избегле такве грешке, резултат функције је фгетц треба сачувати у променљивој као што је инт.

Грешке у куцању

Фрагмент 1

ВКСНУМКС Израз 'врите_тиме' је увек нетачан. диск.ц 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

ВКСНУМКС Израз је увек истинит. Вероватно овде треба користити оператор '&&'. диск.ц 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 у исто време.

Неограничено копирање линија

ВКСНУМКС Позив функције 'спринтф' ће довести до преливања бафера 'фуллпатх'. диск.ц 1257

RD_NTSTATUS
disk_query_directory(....)
{
  ....
  char *dirname, fullpath[PATH_MAX];
  ....
  /* Get information for directory entry */
  sprintf(fullpath, "%s/%s", dirname, pdirent->d_name);
  ....
}

Када погледате функцију у потпуности, биће јасно да овај код не изазива проблеме. Међутим, они се могу појавити у будућности: једна непажљива промена и добићемо преливање бафера - спринтф није ограничен ничим, тако да када спајамо путање можемо ићи изван граница низа. Препоручује се да приметите овај позив на снпринтф(пуна путања, ПАТХ_МАКС, ….).

Сувишно стање

ВКСНУМКС Део условног израза је увек тачан: додај > 0. сцард.ц 507

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

Проверка додај > 0 овде нема потребе: променљива ће увек бити већа од нуле, јер прочитајте % 4 ће вратити остатак дељења, али никада неће бити једнак 4.

крдп

крдп — имплементација РДП сервера са отвореним кодом. Пројекат је подељен на 2 дела:

  • крдп - имплементација протокола. Дистрибуирано под лиценцом Апацхе 2.0.
  • коргкрдп - Скуп Ксорг драјвера за употребу са крдп. Лиценца - Кс11 (као МИТ, али забрањује употребу у оглашавању)

Развој пројекта је заснован на резултатима рдесктоп-а и ФрееРДП-а. У почетку, да бисте радили са графиком, морали сте да користите посебан ВНЦ сервер или посебан Кс11 сервер са РДП подршком - Кс11рдп, али са појавом коргкрдп, потреба за њима је нестала.

У овом чланку нећемо покривати коргкрдп.

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

Провера рдесктоп-а и крдп-а помоћу ПВС-Студио анализатора

Више грешака у куцању

ВКСНУМКС Код садржи колекцију сличних блокова. Означите ставке 'р', 'г', 'р' у редовима 87, 88, 89. рфкенцоде_ргб_то_иув.ц 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++;
      }
      ....
  }
  ....
}

Овај код је преузет из библиотеке либрфкцодец, која имплементира јпег2000 кодек за РемотеФКС. Овде су, очигледно, помешани канали графичких података - уместо „плаве“ боје, снима се „црвена“. Ова грешка се највероватније појавила као резултат цопи-пасте-а.

Исти проблем се појавио у сличној функцији рфк_енцоде_формат_аргб, што нам је анализатор такође рекао:

ВКСНУМКС Код садржи колекцију сличних блокова. Означите ставке 'а', 'р', 'г', 'р' у редовима 260, 261, 262, 263. рфкенцоде_ргб_то_иув.ц 260

while (x < 64)
{
    *la_buf++ = a;
    *lr_buf++ = r;
    *lg_buf++ = g;
    *lb_buf++ = r;
    x++;
}

Декларација низа

ВКСНУМКС Могуће је прекорачење низа. Вредност индекса „и — 8“ би могла да достигне 129. генкеимап.ц 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. Међутим, нема грешака - тачна величина је наведена у датотеци евдев-мап.ц, тако да нема ван граница. Дакле, ово је само грешка која се лако може поправити.

Нетачно поређење

ВКСНУМКС Део условног израза је увек нетачан: (цап_лен < 0). крдп_цапс.ц 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))
  {
    ....
  }
  ....
}

Функција чита променљиву типа непотписан кратак у променљиву попут инт. Провера овде није потребна јер читамо неозначену променљиву и додељујемо резултат већој променљивој, тако да променљива не може узети негативну вредност.

Непотребне провере

ВКСНУМКС Део условног израза је увек тачан: (бпп != 16). либкрдп.ц 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;
  }
  ....
}

Провере неједнакости овде немају смисла јер већ имамо поређење на почетку. Вероватно је ово грешка у куцању и програмер је желео да користи оператера || да бисте филтрирали неважеће аргументе.

Закључак

Током ревизије нису идентификоване озбиљне грешке, али су утврђени многи недостаци. Међутим, ови дизајни се користе у многим системима, иако малог обима. Мали пројекат не мора да има много грешака, тако да не би требало да оцењујете перформансе анализатора само на малим пројектима. Више о томе можете прочитати у чланку „Осећања која су потврђена бројевима".

Можете преузети пробну верзију ПВС-Студио од нас Онлине.

Провера рдесктоп-а и крдп-а помоћу ПВС-Студио анализатора

Ако желите да поделите овај чланак са публиком која говори енглески, користите линк за превод: Сергеј Ларин. Провера рдесктоп-а и крдп-а са ПВС-Студио-ом

Извор: ввв.хабр.цом

Додај коментар