ProHoster > Blog > Gweinyddiaeth > Gwirio rdesktop ac xrdp gyda'r dadansoddwr PVS-Studio

Gwirio rdesktop ac xrdp gyda'r dadansoddwr PVS-Studio

Gwirio rdesktop a xrdp gan ddefnyddio'r dadansoddwr PVS-Studio
Dyma'r ail erthygl mewn cyfres ar brofi meddalwedd ffynhonnell agored ar gyfer gweithio gyda'r protocol RDP. Yn yr erthygl hon, byddwn yn edrych ar y cleient rdesktop a'r gweinydd xrdp.

Yr offeryn a ddefnyddiwyd i nodi gwallau oedd PVS-StiwdioMae'n ddadansoddwr cod statig ar gyfer C, C++, C# a Java, sydd ar gael ar lwyfannau Windows, Linux и macOS.

Dim ond y gwallau a gefais yn ddiddorol sydd yn yr erthygl hon. Fodd bynnag, roedd y prosiectau'n fach, felly nid oedd llawer o wallau.

NodynGellir dod o hyd i erthygl flaenorol am archwiliad prosiect FreeRDP yma.

rdesktop

rdesktop — gweithrediad am ddim o'r cleient RDP ar gyfer systemau sy'n seiliedig ar UNIX. Gellir ei ddefnyddio hefyd o dan Windows, os ydych chi'n adeiladu'r prosiect o dan Cygwin. Trwyddedig o dan GPLv3.

Mae'r cleient hwn yn eithaf poblogaidd—dyma'r rhagosodyn yn ReactOS, ac mae blaen-wynebau graffigol trydydd parti ar gael hefyd. Fodd bynnag, mae'n eithaf hen: cafodd ei ryddhau gyntaf ar Ebrill 4, 2001—gan ei wneud yn 17 mlwydd oed ar adeg ysgrifennu.

Fel y soniais yn gynharach, mae'r prosiect yn eithaf bach. Mae'n cynnwys tua 30 o linellau o god, sydd braidd yn syndod o ystyried ei oedran. Mewn cymhariaeth, mae FreeRDP yn cynnwys 320 o linellau. Dyma allbwn y rhaglen Cloc:

Gwirio rdesktop a xrdp gan ddefnyddio'r dadansoddwr PVS-Studio

Cod na ellir ei gyrraedd

V779 Canfuwyd cod nad yw ar gael. Mae'n bosibl bod gwall yn bresennol. 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);
}

Mae'r gwall yn cwrdd â ni ar unwaith yn y swyddogaeth prif: gwelwn y cod sy'n dod ar ôl y gweithredwr dychwelyd — Mae'r darn hwn yn glanhau cof. Fodd bynnag, nid yw'r gwall yn peri unrhyw fygythiad: bydd yr holl gof a neilltuwyd yn cael ei glirio gan y system weithredu ar ôl i'r rhaglen derfynu.

Diffyg trin gwallau

V557 Mae tanrediad arae yn bosibl. Gallai gwerth mynegai 'n' gyrraedd -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);
  }
  ....
}

Mae'r darn cod yn yr achos hwn yn darllen o'r ffeil i'r byffer nes bod y ffeil yn dod i ben. Fodd bynnag, nid oes unrhyw drin gwallau yma: os bydd rhywbeth yn mynd o'i le, yna darllen yn dychwelyd -1, ac yna bydd gwall arae y tu allan i'r terfynau yn digwydd. allbwn.

Defnyddio EOF mewn math char

V739 Ni ddylid cymharu EOF â gwerth o'r math 'char'. Dylai'r '(c = fgetc(fp))' fod o'r math '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++;
  }
  ....
}

Yma gwelwn driniaeth anghywir o gyrraedd diwedd y ffeil: os fgetc yn dychwelyd cymeriad sydd â chod o 0xFF, yna caiff ei weld fel diwedd y ffeil (EOF).

EOF Mae hwn yn gysonyn, a ddiffinnir fel arfer fel -1. Er enghraifft, yn y cod CP1251, mae gan lythyren olaf yr wyddor Rwsiaidd y cod 0xFF, sy'n cyfateb i'r rhif -1 os ydym yn sôn am newidyn o'r math charMae'n ymddangos bod y symbol 0xFF, fel EOF Dehonglir (-1) fel diwedd ffeil. Er mwyn osgoi gwallau o'r fath, canlyniad y ffwythiant fgetc dylid ei storio mewn newidyn o'r math int.

Typos

Darn 1

V547 Mae'r mynegiant 'amser_ysgrifennu' bob amser yn ffug. 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; // <=
  ....
}

Efallai bod awdur y cod hwn wedi cymysgu pethau || и && yn y cyflwr. Gadewch i ni ystyried gwerthoedd posibl amser_ysgrifennu и amser_newid:

  • Mae'r ddau newidyn yn hafal i 0: yn yr achos hwn byddwn yn mynd i mewn i'r gangen arall: newidyn amser_mod bydd bob amser yn hafal i 0 waeth beth fo'r amod dilynol.
  • Mae un o'r newidynnau yn hafal i 0: amser_mod bydd yn hafal i 0 (ar yr amod bod gan y newidyn arall werth nad yw'n negatif), oherwydd MIN bydd yn dewis y lleiaf o'r ddau opsiwn.
  • Nid yw'r ddau newidyn yn hafal i 0: dewiswch y gwerth lleiaf.

Wrth ddisodli'r amod gyda amser_ysgrifennu a newid_amser bydd yr ymddygiad yn edrych yn gywir:

  • Nid yw un neu'r ddau newidyn yn hafal i 0: dewiswch werth nad yw'n sero.
  • Nid yw'r ddau newidyn yn hafal i 0: dewiswch y gwerth lleiaf.

Darn 2

V547 Mae'r mynegiant bob amser yn wir. Mae'n debyg y dylid defnyddio'r gweithredwr '&&' yma. 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;
  ....
}

Mae'n debyg bod y gweithredwyr wedi'u cymysgu yma hefyd. || и &&Neu == и !=: ni all newidyn gael y gwerth 20 a 9 ar yr un pryd.

Copïo llinynnau diderfyn

V512 Bydd galwad o'r ffwythiant 'sprintf' yn arwain at orlif o'r byffer 'llwybr llawn'. 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);
  ....
}

Wrth archwilio'r swyddogaeth yn ei chyfanrwydd, mae'n dod yn amlwg nad oes unrhyw broblemau gyda'r cod hwn. Fodd bynnag, gallent godi yn y dyfodol: un newid diofal, a byddwn yn cael gorlif byffer— sprintf heb ei gyfyngu gan unrhyw beth, felly wrth gysylltu llwybrau gallwn fynd y tu hwnt i ffiniau'r arae. Argymhellir sylwi ar yr alwad hon ar snprintf(llwybr llawn, PATH_MAX, ….).

Cyflwr diangen

V560 Mae rhan o fynegiant amodol bob amser yn wir: adio > 0. scard.c 507

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

Проверка ychwanegu > 0 does dim pwynt yma: bydd y newidyn bob amser yn fwy na sero, oherwydd darllen % 4 yn dychwelyd gweddill y rhaniad, ac ni fydd byth yn hafal i 4.

xrdp

xrdp — gweithrediad gweinydd RDP ffynhonnell agored. Mae'r prosiect wedi'i rannu'n ddwy ran:

  • Mae xrdp yn weithrediad o'r protocol. Wedi'i ddosbarthu o dan drwydded Apache 2.0.
  • Set o yrwyr Xorg i'w defnyddio gydag xrdp yw xorgxrdp. Trwydded: X11 (fel MIT, ond yn gwahardd ei ddefnyddio mewn hysbysebu).

Mae datblygiad y prosiect yn seiliedig ar ganlyniadau rdesktop a FreeRDP. I ddechrau, roedd prosesu graffeg angen gweinydd VNC ar wahân neu weinydd X11 pwrpasol gyda chefnogaeth RDP—X11rdp—ond gyda dyfodiad xorgxrdp, nid oedd angen y rhain mwyach.

Yn yr erthygl hon ni fyddwn yn cyffwrdd â xorgxrdp.

Mae'r prosiect xrdp, fel yr un blaenorol, yn eithaf bach ac yn cynnwys tua 80 mil o linellau.

Gwirio rdesktop a xrdp gan ddefnyddio'r dadansoddwr PVS-Studio

Mwy o gamgymeriadau teipio

V525 Mae'r cod yn cynnwys y casgliad o flociau tebyg. Gwiriwch eitemau 'r', 'g', 'r' yn llinellau 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++;
      }
      ....
  }
  ....
}

Cymerwyd y cod hwn o'r llyfrgell librfxcodec, sy'n gweithredu'r codec jpeg2000 ar gyfer RemoteFX. Ymddengys bod y sianeli data graffeg wedi'u gwrthdroi—mae'r lliw "glas" yn cael ei ysgrifennu fel "coch." Mae'n fwyaf tebygol bod y gwall hwn wedi'i achosi gan gopïo a gludo.

Mae'r un broblem wedi ymddangos mewn swyddogaeth debyg hefyd. fformat_amgodio_rfx_argb, a ddywedodd y dadansoddwr wrthym amdano hefyd:

V525 Mae'r cod yn cynnwys y casgliad o flociau tebyg. Gwiriwch eitemau 'a', 'r', 'g', 'r' yn llinellau 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++;
}

Datganiad arae

V557 Mae gor-redeg arae yn bosibl. Gallai gwerth mynegai 'i — 8' gyrraedd 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];
    ....
  }
  ....
}

Mae'r datganiad a'r diffiniad arae yn y ddwy ffeil hyn yn anghyson—mae'r maint yn wahanol o 1. Fodd bynnag, nid oes unrhyw wallau'n digwydd—mae'r maint cywir wedi'i nodi yn y ffeil evdev-map.c, felly nid oes gwall y tu allan i'r terfynau. Felly dim ond esgeulustod y gellir ei drwsio'n hawdd yw hwn.

Cymhariaeth anghywir

V560 Mae rhan o fynegiant amodol bob amser yn ffug: (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))
  {
    ....
  }
  ....
}

Mae'r ffwythiant yn darllen newidyn o'r math byr heb ei lofnodi i mewn i newidyn o fath intNid oes angen y gwiriad yma, oherwydd ein bod yn darllen newidyn heb lofnod ac yn aseinio'r canlyniad i newidyn mwy, felly ni all y newidyn gymryd gwerth negyddol.

Gwiriadau diangen

V560 Mae rhan o fynegiant amodol bob amser yn wir: (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;
  }
  ....
}

Mae gwiriadau anghydraddoldeb yn ddibwrpas yma, gan fod gennym gymhariaeth eisoes ar y dechrau. Mae'n eithaf posibl mai camgymeriad teipio yw hwn a bod y datblygwr wedi bwriadu defnyddio'r gweithredwr. || i hidlo dadleuon annilys allan.

Casgliad

Ni ddatgelodd yr archwiliad unrhyw wallau difrifol, ond fe ddatgelodd lawer o ddiffygion. Fodd bynnag, defnyddir y prosiectau hyn mewn llawer o systemau, er eu bod yn fach o ran maint. Nid oes gan brosiect bach lawer o wallau o reidrwydd, felly nid yw'n werth barnu perfformiad y dadansoddwr ar brosiectau bach yn unig. Gallwch ddarllen mwy am hyn yn yr erthygl "Teimladau a gadarnhawyd gan niferoedd".

Gallwch lawrlwytho fersiwn dreial o PVS-Studio gennym ni yn Ar-lein.

Gwirio rdesktop a xrdp gan ddefnyddio'r dadansoddwr PVS-Studio

Os ydych chi am rannu'r erthygl hon â chynulleidfa Saesneg ei hiaith, defnyddiwch y ddolen gyfieithu: Sergey Larin. Gwirio rdesktop ac xrdp gyda PVS-Studio

Ffynhonnell: hab.com

Prynu gwesteio dibynadwy ar gyfer gwefannau sydd â diogelwch DDoS, gweinyddwyr VPS VDS 🔥 Prynu cynnal gwefannau dibynadwy gyda diogelwch DDoS, gweinyddion VPS VDS | ProHoster