Rdesktop-ի և xrdp-ի ստուգում PVS-Studio անալիզատորի միջոցով

Սա RDP արձանագրության հետ աշխատելու համար բաց կոդով ծրագրերի փորձարկման մասին հոդվածների շարքի երկրորդ ակնարկն է: Դրանում մենք կանդրադառնանք rdesktop հաճախորդին և xrdp սերվերին:

Օգտագործվում է որպես սխալների հայտնաբերման գործիք PVS- ստուդիա. Այն C, C++, C# և Java լեզուների ստատիկ կոդերի անալիզատոր է, որը հասանելի է Windows, Linux և macOS հարթակներում:

Հոդվածում ներկայացված են միայն այն սխալները, որոնք ինձ հետաքրքիր էին թվում։ Այնուամենայնիվ, նախագծերը փոքր են, ուստի սխալները քիչ են եղել :):

Նշում. Նախորդ հոդվածը FreeRDP նախագծի ստուգման մասին կարող եք գտնել այստեղ.

rdesktop

rdesktop — RDP հաճախորդի անվճար իրականացում UNIX-ի վրա հիմնված համակարգերի համար: Այն կարող է օգտագործվել նաև 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 Հնարավոր է զանգվածի ներքևում: «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-ի օգտագործումը 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++;
  }
  ....
}

Այստեղ մենք տեսնում ենք ֆայլի ավարտին հասնելու սխալ մշակում. 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-ի. ընտրել նվազագույն արժեքը:

Վիճակը փոխարինելիս 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» ֆունկցիայի կանչը կհանգեցնի բուֆերի «լրիվ ուղու» հեղեղմանը: 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 Պայմանական արտահայտության մի մասը միշտ ճշմարիտ է՝ ավելացնել > 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-ի համար: Այստեղ, ըստ երևույթին, գրաֆիկական տվյալների ալիքները խառնվել են. «կապույտ» գույնի փոխարեն ձայնագրվում է «կարմիր»: Այս սխալը, ամենայն հավանականությամբ, հայտնվել է 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-ով

Source: www.habr.com