ProHoster > Blog > administrasie > Kontroleer rdesktop en xrdp met behulp van die PVS-Studio-ontleder

Kontroleer rdesktop en xrdp met behulp van die PVS-Studio-ontleder

Gaan rdesktop en xrdp na met die PVS-Studio-ontleder
Dit is die tweede resensie in 'n reeks artikels oor die toets van oopbronprogramme om met die HOP-protokol te werk. Daarin sal ons kyk na die rdesktop-kliënt en die xrdp-bediener.

Word gebruik as 'n hulpmiddel om foute te identifiseer PVS-ateljee. Dit is 'n statiese kode-ontleder vir C, C++, C# en Java-tale, beskikbaar op Windows-, Linux- en macOS-platforms.

Die artikel bied slegs daardie foute aan wat vir my interessant gelyk het. Die projekte is egter klein, so daar was min foute :).

Let daarop. 'n Vorige artikel oor FreeRDP-projekverifikasie kan gevind word hier.

r -tafelblad

r -tafelblad — 'n gratis implementering van 'n RDP-kliënt vir UNIX-gebaseerde stelsels. Dit kan ook onder Windows gebruik word as jy die projek onder Cygwin bou. Gelisensieer onder GPLv3.

Hierdie kliënt is baie gewild - dit word by verstek in ReactOS gebruik, en jy kan ook derdeparty grafiese front-ends daarvoor vind. Hy is egter redelik oud: sy eerste vrylating het op 4 April 2001 plaasgevind – met die skryf hiervan is hy 17 jaar oud.

Soos ek vroeër opgemerk het, is die projek baie klein. Dit bevat ongeveer 30 duisend reëls kode, wat 'n bietjie vreemd is met inagneming van sy ouderdom. Ter vergelyking bevat FreeRDP 320 duisend reëls. Hier is die uitset van die Cloc-program:

Gaan rdesktop en xrdp na met die PVS-Studio-ontleder

Onbereikbare kode

V779 Onbeskikbare kode bespeur. Dit is moontlik dat 'n fout teenwoordig is. 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);
}

Die fout kom ons dadelik teë in die funksie hoof: ons sien die kode kom na die operateur terugkeer — hierdie fragment voer geheue skoonmaak uit. Die fout hou egter nie 'n bedreiging in nie: alle toegewese geheue sal deur die bedryfstelsel uitgevee word nadat die program afgesluit is.

Geen fouthantering nie

V557 Skikking onderloop is moontlik. Die waarde van 'n'-indeks kan -1 bereik. 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);
  }
  ....
}

Die kodebrokkie in hierdie geval lees vanaf die lêer in 'n buffer totdat die lêer eindig. Hier is egter geen fouthantering nie: as iets verkeerd loop, dan lees sal -1 terugstuur, en dan sal die skikking oorskry word uitset.

Gebruik EOF in char tipe

V739 EOF moet nie vergelyk word met 'n waarde van die 'char'-tipe nie. Die '(c = fgetc(fp))' moet van die 'int'-tipe wees. 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++;
  }
  ....
}

Hier sien ons verkeerde hantering om die einde van die lêer te bereik: if fgetc gee 'n karakter terug wie se kode 0xFF is, dit sal geïnterpreteer word as die einde van die lêer (EOF).

EOF dit is 'n konstante, gewoonlik gedefinieer as -1. Byvoorbeeld, in die CP1251-kodering het die laaste letter van die Russiese alfabet die kode 0xFF, wat ooreenstem met die nommer -1 as ons praat van 'n veranderlike soos wa. Dit blyk dat die simbool 0xFF, soos EOF (-1) word geïnterpreteer as die einde van die lêer. Om sulke foute te vermy, is die resultaat van die funksie fgetc moet gestoor word in 'n veranderlike soos int.

Tikfoute

Fragment 1

V547 Uitdrukking 'skryf_tyd' is altyd onwaar. skyf.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; // <=
  ....
}

Miskien het die skrywer van hierdie kode dit verkeerd verstaan || и && in toestand. Kom ons oorweeg moontlike opsies vir waardes skryf_tyd и verander_tyd:

  • Beide veranderlikes is gelyk aan 0: in hierdie geval sal ons in 'n tak beland anders: veranderlike mod_tyd sal altyd 0 wees ongeag die daaropvolgende toestand.
  • Een van die veranderlikes is 0: mod_tyd sal gelyk wees aan 0 (mits die ander veranderlike 'n nie-negatiewe waarde het), want MIN sal die kleinste van die twee opsies kies.
  • Beide veranderlikes is nie gelyk aan 0 nie: kies die minimum waarde.

Wanneer die toestand vervang word met skryf_tyd && verander_tyd die gedrag sal korrek lyk:

  • Een of albei veranderlikes is nie gelyk aan 0 nie: kies 'n nie-nul waarde.
  • Beide veranderlikes is nie gelyk aan 0 nie: kies die minimum waarde.

Fragment 2

V547 Uitdrukking is altyd waar. Waarskynlik moet die '&&'-operateur hier gebruik word. skyf.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;
  ....
}

Blykbaar is die operateurs ook hier deurmekaar || и &&Of == и !=: 'n Veranderlike kan nie die waarde 20 en 9 op dieselfde tyd hê nie.

Onbeperkte lynkopiering

V512 'n Oproep van die 'sprintf'-funksie sal lei tot oorloop van die buffer 'fullpath'. skyf.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);
  ....
}

Wanneer jy na die funksie volledig kyk, sal dit duidelik word dat hierdie kode nie probleme veroorsaak nie. Hulle kan egter in die toekoms ontstaan: een onverskillige verandering en ons sal 'n buffer-oorloop kry - sprintf word deur niks beperk nie, so wanneer ons paaie aaneenskakel, kan ons verby die grense van die skikking gaan. Dit word aanbeveel om hierdie oproep op te let snprintf(volpad, PATH_MAX, ….).

Oortollige toestand

V560 'n Deel van voorwaardelike uitdrukking is altyd waar: voeg by > 0. scard.c 507

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

Проверка voeg > 0 by daar is geen behoefte hier nie: die veranderlike sal altyd groter as nul wees, want lees % 4 sal die res van die verdeling teruggee, maar dit sal nooit gelyk wees aan 4 nie.

xrdp

xrdp - implementering van 'n RDP-bediener met oopbronkode. Die projek is in 2 dele verdeel:

  • xrdp - protokol implementering. Versprei onder die Apache 2.0-lisensie.
  • xorgxrdp - 'n Stel Xorg-drywers vir gebruik met xrdp. Lisensie - X11 (soos MIT, maar verbied gebruik in advertensies)

Die ontwikkeling van die projek is gebaseer op die resultate van rdesktop en FreeRDP. Aanvanklik, om met grafika te werk, moes jy 'n aparte VNC-bediener gebruik, of 'n spesiale X11-bediener met RDP-ondersteuning - X11rdp, maar met die koms van xorgxrdp het die behoefte daaraan verdwyn.

In hierdie artikel sal ons nie xorgxrdp dek nie.

Die xrdp-projek, soos die vorige een, is baie klein en bevat ongeveer 80 duisend reëls.

Gaan rdesktop en xrdp na met die PVS-Studio-ontleder

Nog tikfoute

V525 Die kode bevat die versameling soortgelyke blokke. Kontroleer items 'r', 'g', 'r' in reëls 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++;
      }
      ....
  }
  ....
}

Hierdie kode is geneem uit die librfxcodec-biblioteek, wat die jpeg2000-kodek vir RemoteFX implementeer. Hier is blykbaar die grafiese datakanale deurmekaar - in plaas van die "blou" kleur, word "rooi" aangeteken. Hierdie fout het heel waarskynlik verskyn as gevolg van copy-paste.

Dieselfde probleem het in 'n soortgelyke funksie voorgekom rfx_encode_format_argb, wat die ontleder ook vir ons gesê het:

V525 Die kode bevat die versameling soortgelyke blokke. Merk items 'a', 'r', 'g', 'r' in reëls 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++;
}

Skikkingsverklaring

V557 Skikking oorskryding is moontlik. Die waarde van 'i — 8'-indeks kan 129 bereik. 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];
    ....
  }
  ....
}

Die verklaring en definisie van die skikking in hierdie twee lêers is onversoenbaar - die grootte verskil met 1. Geen foute kom egter voor nie - die korrekte grootte word in die evdev-map.c lêer gespesifiseer, so daar is geen buite perke nie. Dit is dus net 'n fout wat maklik reggemaak kan word.

Verkeerde vergelyking

V560 'n Deel van voorwaardelike uitdrukking is altyd vals: (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))
  {
    ....
  }
  ....
}

Die funksie lees 'n tipe veranderlike ongetekende kort in 'n veranderlike soos int. Kontrolering is nie hier nodig nie, want ons lees 'n ongetekende veranderlike en ken die resultaat aan 'n groter veranderlike toe, so die veranderlike kan nie 'n negatiewe waarde neem nie.

Onnodige tjeks

V560 'n Deel van voorwaardelike uitdrukking is altyd waar: (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;
  }
  ....
}

Ongelykheidstoetse maak nie sin hier nie, aangesien ons reeds aan die begin 'n vergelyking het. Dit is waarskynlik 'n tikfout en die ontwikkelaar wou die operateur gebruik || om ongeldige argumente uit te filter.

Gevolgtrekking

Tydens die oudit is geen ernstige foute geïdentifiseer nie, maar baie tekortkominge is gevind. Hierdie ontwerpe word egter in baie stelsels gebruik, al is dit klein in omvang. 'n Klein projek het nie noodwendig baie foute nie, dus moet jy nie die ontleder se prestasie net op klein projekte beoordeel nie. Jy kan meer hieroor lees in die artikel "Gevoelens wat deur syfers bevestig is«.

Jy kan 'n proefweergawe van PVS-Studio by ons aflaai Online.

Gaan rdesktop en xrdp na met die PVS-Studio-ontleder

As jy hierdie artikel met 'n Engelssprekende gehoor wil deel, gebruik asseblief die vertaalskakel: Sergey Larin. Gaan rdesktop en xrdp na met PVS-Studio

Bron: will.com

Voeg 'n opmerking