ProHoster > blog > Gweinyddiaeth > Gwirio rdesktop a xrdp gan ddefnyddio'r dadansoddwr PVS-Studio

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

Gwirio rdesktop a xrdp gan ddefnyddio'r dadansoddwr PVS-Studio
Dyma'r ail adolygiad mewn cyfres o erthyglau am brofi rhaglenni ffynhonnell agored ar gyfer gweithio gyda phrotocol y Cynllun Datblygu Gwledig. Ynddo byddwn yn edrych ar y cleient rdesktop a'r gweinydd xrdp.

Fe'i defnyddir fel offeryn i nodi gwallau PVS-Stiwdio. Mae'n ddadansoddwr cod statig ar gyfer ieithoedd C, C++, C# a Java, sydd ar gael ar lwyfannau Windows, Linux a macOS.

Mae'r erthygl yn cyflwyno dim ond y gwallau hynny a oedd yn ymddangos yn ddiddorol i mi. Fodd bynnag, mae'r prosiectau yn fach, felly ychydig o gamgymeriadau oedd :).

Nodyn. Gellir dod o hyd i erthygl flaenorol am ddilysu prosiect FreeRDP yma.

bwrdd gwaith

bwrdd gwaith - gweithredu cleient RDP am ddim 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 boblogaidd iawn - fe'i defnyddir yn ddiofyn yn ReactOS, a gallwch hefyd ddod o hyd i bennau blaen graffigol trydydd parti ar ei gyfer. Fodd bynnag, mae'n eithaf hen: digwyddodd ei ryddhad cyntaf ar Ebrill 4, 2001 - ar adeg ysgrifennu, mae'n 17 oed.

Fel y nodais yn gynharach, mae'r prosiect yn eithaf bach. Mae'n cynnwys tua 30 mil o linellau o god, sy'n rhyfedd braidd o ystyried ei oedran. Er mwyn cymharu, mae FreeRDP yn cynnwys 320 mil o linellau. Dyma allbwn y rhaglen Cloc:

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

Cod anghyraeddadwy

V779 Cod nad oedd ar gael wedi'i ganfod. 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 dod ar draws ni ar unwaith yn y swyddogaeth prif: gwelwn y cod yn dod ar Γ΄l y gweithredwr dychwelyd β€” mae'r darn hwn yn glanhau cof. Fodd bynnag, nid yw'r gwall yn fygythiad: bydd yr holl gof a neilltuwyd yn cael ei glirio gan y system weithredu ar Γ΄l i'r rhaglen ddod i ben.

Dim trin gwall

V557 Mae tanrediad Array 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 pyt cod yn yr achos hwn yn darllen o'r ffeil i mewn i glustog nes bod y ffeil yn dod i ben. Fodd bynnag, nid oes unrhyw drin gwall yma: os aiff rhywbeth o'i le, yna darllen bydd yn dychwelyd -1, ac yna bydd yr arae yn cael ei or-redeg allbwn.

Defnyddio EOF mewn math torgoch

V739 Ni ddylid cymharu EOF Γ’ gwerth o'r math 'torgoch'. 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 drin anghywir o gyrraedd diwedd y ffeil: if fgetc yn dychwelyd nod y mae ei god yn 0xFF, bydd yn cael ei ddehongli fel diwedd y ffeil (EOF).

EOF mae'n gysonyn, a ddiffinnir fel arfer fel -1. Er enghraifft, yn yr amgodio CP1251, mae gan lythyren olaf yr wyddor Rwsieg y cod 0xFF, sy'n cyfateb i'r rhif -1 os ydym yn sΓ΄n am newidyn fel char. Mae'n ymddangos bod y symbol 0xFF, fel EOF (-1) yn cael ei ddehongli fel diwedd y ffeil. Er mwyn osgoi gwallau o'r fath, canlyniad y swyddogaeth yw fgetc dylid ei storio mewn newidyn tebyg int.

Typos

Darn 1

V547 Mae'r ymadrodd 'write_time' bob amser yn ffug. disg.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 gwneud camgymeriad || ΠΈ && mewn cyflwr. Gadewch i ni ystyried opsiynau posibl ar gyfer gwerthoedd ysgrifennu_amser ΠΈ newid_amser:

  • Mae'r ddau newidyn yn hafal i 0: yn yr achos hwn byddwn yn y pen draw mewn cangen arall: newidyn mod_amser bydd bob amser yn 0 waeth beth fo'r cyflwr dilynol.
  • Un o'r newidynnau yw 0: mod_amser yn hafal i 0 (ar yr amod bod gan y newidyn arall werth annegyddol), oherwydd MIN yn dewis y lleiaf o'r ddau opsiwn.
  • Nid yw'r ddau newidyn yn hafal i 0: dewiswch y gwerth lleiaf.

Wrth amnewid y cyflwr gyda amser_ysgrifennu && 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 mynegiant bob amser yn wir. Mae'n debyg y dylid defnyddio'r gweithredwr '&&' yma. disg.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 yn gymysg yma hefyd || ΠΈ &&Neu == ΠΈ !=: Ni all newidyn gael y gwerth 20 a 9 ar yr un pryd.

CopΓ―o llinell anghyfyngedig

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

Pan edrychwch ar y swyddogaeth yn llawn, daw'n amlwg nad yw'r cod hwn yn achosi problemau. Fodd bynnag, efallai y byddant yn codi yn y dyfodol: un newid diofal a byddwn yn cael gorlif byffer - gwibio heb ei gyfyngu gan unrhyw beth, felly wrth gydgatenu llwybrau gallwn fynd y tu hwnt i ffiniau'r rhesi. Argymhellir sylwi ar yr alwad hon ymlaen 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 nid oes angen yma: bydd y newidyn bob amser yn fwy na sero, oherwydd darllen % 4 yn dychwelyd gweddill y rhaniad, ond ni fydd byth yn hafal i 4.

xrdp

xrdp β€” gweithredu gweinydd RDP gyda chod ffynhonnell agored. Rhennir y prosiect yn 2 ran:

  • xrdp - gweithredu protocol. Wedi'i ddosbarthu o dan drwydded Apache 2.0.
  • xorgxrdp - Set o yrwyr Xorg i'w defnyddio gyda xrdp. Trwydded - X11 (fel MIT, ond yn gwahardd ei ddefnyddio mewn hysbysebu)

Mae datblygiad y prosiect yn seiliedig ar ganlyniadau rdesktop a FreeRDP. I ddechrau, i weithio gyda graffeg, roedd yn rhaid i chi ddefnyddio gweinydd VNC ar wahΓ’n, neu weinydd X11 arbennig gyda chefnogaeth RDP - X11rdp, ond gyda dyfodiad xorgxrdp, diflannodd yr angen amdanynt.

Yn yr erthygl hon ni fyddwn yn ymdrin Γ’ xorgxrdp.

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

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

Mwy o deipos

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. Yma, mae'n debyg, mae'r sianeli data graffig yn gymysg - yn lle'r lliw β€œglas”, mae β€œcoch” yn cael ei gofnodi. Ymddangosodd y gwall hwn yn fwyaf tebygol o ganlyniad i gopΓ―o-gludo.

Digwyddodd yr un broblem mewn swyddogaeth debyg rfx_encode_format_argb, y dywedodd y dadansoddwr wrthym hefyd:

V525 Mae'r cod yn cynnwys 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 Array

V557 Array gor-redeg 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 datganiad a diffiniad yr arae yn y ddwy ffeil hyn yn anghydnaws - mae'r maint yn amrywio o 1. Fodd bynnag, nid oes unrhyw wallau - mae'r maint cywir wedi'i nodi yn y ffeil evdev-map.c, felly nid oes unrhyw gyfyngiadau. Felly dim ond nam yw hwn y gellir ei drwsio'n hawdd.

Cymhariaeth anghywir

V560 Mae rhan o fynegiant amodol bob amser yn anwir: ( 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 math byr heb ei arwyddo i mewn i newidyn fel int. Nid oes angen gwirio yma oherwydd ein bod yn darllen newidyn heb ei lofnodi 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;
  }
  ....
}

Nid yw gwiriadau anghydraddoldeb yn gwneud synnwyr yma gan fod gennym gymhariaeth yn barod ar y dechrau. Mae'n debygol mai teipio yw hwn ac roedd y datblygwr eisiau defnyddio'r gweithredwr || i hidlo dadleuon annilys.

Casgliad

Yn ystod yr archwiliad, ni chanfuwyd unrhyw wallau difrifol, ond canfuwyd llawer o ddiffygion. Fodd bynnag, defnyddir y dyluniadau hyn mewn llawer o systemau, er eu bod yn fach o ran cwmpas. Nid oes gan brosiect bach lawer o wallau o reidrwydd, felly ni ddylech farnu perfformiad y dadansoddwr ar brosiectau bach yn unig. Gallwch ddarllen mwy am hyn yn yr erthygl β€œTeimladau a gadarnhawyd gan niferoedd".

Gallwch lawrlwytho fersiwn prawf o PVS-Studio oddi wrthym 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 a xrdp gyda PVS-Studio

Ffynhonnell: hab.com

Ychwanegu sylw