🥇Pagsusi sa rdesktop ug xrdp gamit ang PVS-Studio analyzer

Kini ang ikaduha nga pagrepaso sa serye sa mga artikulo bahin sa pagsulay sa mga open source nga programa alang sa pagtrabaho kauban ang RDP protocol. Diha niini atong tan-awon ang rdesktop client ug ang xrdp server.

Gigamit isip himan sa pag-ila sa mga sayop PVS Studio. Usa kini ka static code analyzer para sa C, C++, C# ug Java nga mga pinulongan, nga anaa sa Windows, Linux ug macOS nga mga plataporma.

Ang artikulo nagpresentar lamang sa mga sayop nga morag makapainteres kanako. Bisan pa, ang mga proyekto gamay ra, mao nga gamay ra ang mga sayup :).

Примечание. Ang miaging artikulo bahin sa pag-verify sa proyekto sa FreeRDP makit-an dinhi.

rdesktop

rdesktop — usa ka libre nga pagpatuman sa usa ka kliyente sa RDP alang sa mga sistema nga nakabase sa UNIX. Mahimo usab kini gamiton ubos sa Windows kung imong tukuron ang proyekto ubos sa Cygwin. Lisensyado ubos sa GPLv3.

Kini nga kliyente sikat kaayo - gigamit kini nga default sa ReactOS, ug mahimo ka usab nga makit-an ang mga third-party nga graphical front-end para niini. Bisan pa, medyo tigulang na siya: ang una niyang pagpagawas nahitabo kaniadtong Abril 4, 2001 - sa panahon sa pagsulat, siya 17 anyos.

Sama sa akong namatikdan sa sayo pa, ang proyekto gamay ra kaayo. Naglangkob kini sa gibana-bana nga 30 ka libo nga linya sa code, nga medyo katingad-an kung gikonsiderar ang edad niini. Alang sa pagtandi, ang FreeRDP adunay 320 ka libo nga linya. Ania ang output sa programa sa Cloc:

Dili maabot nga code

V779 Ang dili magamit nga code nakit-an. Posible nga adunay usa ka sayup. 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);
}

Ang kasaypanan makit-an dayon kanamo sa function nag-unang: atong makita ang code nga moabut human sa operator pagbalik - kini nga tipik naghimo sa paglimpyo sa memorya. Bisan pa, ang sayup wala maghatag usa ka hulga: ang tanan nga gigahin nga panumduman malimpyohan sa operating system pagkahuman sa paggawas sa programa.

Walay sayop nga pagdumala

V557 Posible ang pag-underrun sa array. Ang bili sa 'n' index mahimong moabot -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);
  }
  ....
}

Ang snippet sa code niini nga kaso mabasa gikan sa file ngadto sa usa ka buffer hangtod matapos ang file. Bisan pa, wala’y sayup nga pagdumala dinhi: kung adunay sayup, nan basaha mobalik -1, ug unya ang laray ma-overrun Output.

Gigamit ang EOF sa tipo sa char

V739 Ang EOF kinahanglan dili itandi sa usa ka kantidad sa tipo nga 'char'. Ang '(c = fgetc(fp))' kinahanglan nga 'int' nga tipo. 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++;
  }
  ....
}

Dinhi atong makita ang sayop nga pagdumala sa pagkab-ot sa katapusan sa file: kung fgetc nagbalik sa usa ka karakter kansang code mao ang 0xFF, kini hubaron ingon nga katapusan sa file (EOF).

EOF kini usa ka makanunayon, kasagaran gihubit nga -1. Pananglitan, sa pag-encode sa CP1251, ang katapusan nga letra sa alpabetong Ruso adunay code 0xFF, nga katumbas sa numero -1 kung naghisgot kami bahin sa usa ka variable sama sa. char. Kini nahimo nga ang simbolo 0xFF, sama sa EOF (-1) gihubad ingon nga katapusan sa file. Aron malikayan ang ingon nga mga sayup, ang sangputanan sa function mao fgetc kinahanglan nga tipigan sa usa ka variable sama sa int.

Mga typo

Tipik 1

V547 Ang ekspresyong 'write_time' kanunay bakak. 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; // <=
  ....
}

Tingali ang tagsulat niini nga code nasayop || и && sa kondisyon. Atong tagdon ang posible nga mga kapilian alang sa mga mithi pagsulat_panahon и kausaban_panahon:

Ang duha nga mga variable parehas sa 0: sa kini nga kaso kita matapos sa usa ka sanga lain: variable mod_panahon kanunay nga 0 bisan unsa pa ang sunod nga kondisyon.
Usa sa mga variable mao ang 0: mod_panahon mahimong katumbas sa 0 (basta ang lain nga variable adunay dili negatibo nga kantidad), tungod kay min mopili sa mas gamay sa duha ka mga kapilian.
Ang duha ka mga variable dili parehas sa 0: pilia ang minimum nga kantidad.

Kung gipulihan ang kondisyon sa write_time && change_time ang pamatasan tan-awon nga husto:

Ang usa o pareho nga mga variable dili katumbas sa 0: pagpili og dili zero nga kantidad.
Ang duha ka mga variable dili parehas sa 0: pilia ang minimum nga kantidad.

Tipik 2

V547 Ang ekspresyon kanunay nga tinuod. Tingali ang '&&' operator kinahanglan gamiton dinhi. 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;
  ....
}

Dayag nga ang mga operator nagsagol usab dinhi || и &&, o == и !=: Ang usa ka variable dili mahimong adunay kantidad nga 20 ug 9 sa parehas nga oras.

Walay kinutuban nga pagkopya sa linya

V512 Ang usa ka tawag sa 'sprintf' nga function motultol sa pag-awas sa buffer '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);
  ....
}

Kung imong tan-awon ang function sa bug-os, mahimong klaro nga kini nga code dili hinungdan sa mga problema. Bisan pa, mahimo silang motungha sa umaabot: usa ka walay pagtagad nga pagbag-o ug makakuha kami usa ka buffer overflow - sprint dili limitado sa bisan unsang butang, mao nga kung maghiusa ang mga agianan mahimo naton nga lapas sa mga utlanan sa laray. Kini girekomendar sa pagmatikod niini nga tawag sa snprintf(fullpath, PATH_MAX, ….).

Kabus nga kahimtang

V560 Usa ka bahin sa conditional nga ekspresyon kanunay tinuod: idugang > 0. scard.c 507

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

inspection idugang > 0 wala'y kinahanglan dinhi: ang variable kanunay nga labaw sa zero, tungod kay pagbasa% 4 ibalik ang nahabilin sa dibisyon, apan dili gyud kini katumbas sa 4.

xrdp

xrdp - pagpatuman sa usa ka RDP server nga adunay open source code. Ang proyekto gibahin sa 2 ka bahin:

xrdp - pagpatuman sa protocol. Giapod-apod ubos sa Apache 2.0 nga lisensya.
xorgxrdp - Usa ka hugpong sa mga drayber sa Xorg nga gamiton sa xrdp. Lisensya - X11 (sama sa MIT, apan gidili ang paggamit sa advertising)

Ang pagpalambo sa proyekto gibase sa mga resulta sa rdesktop ug FreeRDP. Sa sinugdan, aron magtrabaho uban ang mga graphic, kinahanglan nimo nga gamiton ang usa ka bulag nga VNC server, o usa ka espesyal nga X11 server nga adunay suporta sa RDP - X11rdp, apan sa pag-abut sa xorgxrdp, ang panginahanglan alang kanila nawala.

Niini nga artikulo dili nato hisgotan ang xorgxrdp.

Ang proyekto sa xrdp, sama sa nauna, gamay kaayo ug adunay gibana-bana nga 80 ka libo nga linya.

Dugang typo

V525 Ang code naglangkob sa koleksyon sa parehas nga mga bloke. Susiha ang mga butang 'r', 'g', 'r' sa mga linya 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++;
      }
      ....
  }
  ....
}

Kini nga code gikuha gikan sa librfxcodec library, nga nagpatuman sa jpeg2000 codec para sa RemoteFX. Dinhi, dayag, ang mga graphic data channel gisagol - imbes nga "asul" nga kolor, "pula" ang natala. Kini nga sayup lagmit nga nagpakita ingon usa ka sangputanan sa pagkopya-paste.

Ang parehas nga problema nahitabo sa parehas nga function rfx_encode_format_argb, nga gisulti usab kanamo sa analisador:

V525 Ang code naglangkob sa koleksyon sa parehas nga mga bloke. Susiha ang mga butang 'a', 'r', 'g', 'r' sa mga linya 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++;
}

Deklarasyon sa Array

V557 Posible ang pag-overrun sa array. Ang bili sa 'i — 8' index mahimong moabot sa 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];
    ....
  }
  ....
}

Ang deklarasyon ug kahulugan sa laray niining duha ka mga file dili magkauyon - ang gidak-on lahi sa 1. Apan, walay mga sayup nga mahitabo - ang husto nga gidak-on gipiho sa evdev-map.c file, mao nga walay out of bounds. Mao nga kini usa lamang ka bug nga dali ra masulbad.

Sayop nga pagtandi

V560 Ang usa ka bahin sa conditional nga ekspresyon kanunay nga bakak: (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))
  {
    ....
  }
  ....
}

Ang function nagbasa sa usa ka variable nga tipo unsigned mubu ngadto sa usa ka variable sama sa int. Dili kinahanglan ang pagsusi dinhi tungod kay nagbasa kami usa ka wala gipirmahan nga variable ug gi-assign ang resulta sa usa ka mas dako nga variable, mao nga ang variable dili makakuha usa ka negatibo nga kantidad.

Wala kinahanglana nga mga pagsusi

V560 Ang usa ka bahin sa conditional expression kanunay tinuod: (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;
  }
  ....
}

Ang mga pagsusi sa inequality dili makatarunganon dinhi tungod kay aduna na kitay pagtandi sa sinugdanan. Kini lagmit nga kini usa ka typo ug gusto sa developer nga gamiton ang operator || sa pagsala sa dili balido nga mga argumento.

konklusyon

Atol sa pag-audit, walay seryoso nga mga sayop nga naila, apan daghang mga kakulangan ang nakit-an. Bisan pa, kini nga mga disenyo gigamit sa daghang mga sistema, bisan kung gamay ang kasangkaran. Ang usa ka gamay nga proyekto dili kinahanglan nga adunay daghang mga sayup, mao nga dili nimo kinahanglan nga hukman ang pasundayag sa analisador sa gagmay nga mga proyekto. Mahimo nimong mabasa ang dugang bahin niini sa artikulo nga "Mga pagbati nga gikumpirma sa mga numero".

Mahimo nimong i-download ang usa ka pagsulay nga bersyon sa PVS-Studio gikan kanamo site.

Kung gusto nimong ipaambit kini nga artikulo sa usa ka tigpaminaw nga nagsultig English, palihug gamita ang link sa paghubad: Sergey Larin. Pagsusi sa rdesktop ug xrdp gamit ang PVS-Studio

Source: www.habr.com

Pagsusi sa rdesktop ug xrdp gamit ang PVS-Studio analyzer