PVS-Studio विश्लेषकाद्वारे rdesktop आणि xrdp तपासणे

आरडीपी प्रोटोकॉलसह कार्य करण्यासाठी ओपन सोर्स प्रोग्रामच्या चाचणीबद्दल लेखांच्या मालिकेतील हे दुसरे पुनरावलोकन आहे. त्यामध्ये आपण rdesktop क्लायंट आणि xrdp सर्व्हर पाहू.

त्रुटी ओळखण्यासाठी एक साधन म्हणून वापरले जाते पीव्हीएस-स्टुडिओहे C, C++, C# आणि Java साठी एक स्टॅटिक कोड अॅनालायझर आहे, जे प्लॅटफॉर्मवर उपलब्ध आहे. Windows, Linux и macOS.

लेखात फक्त त्या त्रुटी आहेत ज्या मला मनोरंजक वाटल्या. तथापि, प्रकल्प लहान आहेत, म्हणून काही चुका होत्या :).

शेरा. फ्रीआरडीपी प्रकल्प पडताळणीबद्दल मागील लेख आढळू शकतो येथे.

rdesktop

rdesktop — युनिक्स-आधारित प्रणालींसाठी आरडीपी क्लायंटची एक विनामूल्य अंमलबजावणी. याचा वापर खालीलप्रमाणे देखील केला जाऊ शकतो: Windowsजर तुम्ही Cygwin अंतर्गत प्रोजेक्ट बिल्ड केला तर. GPLv3 अंतर्गत परवानाकृत.

हा क्लायंट खूप लोकप्रिय आहे - तो ReactOS मध्ये डीफॉल्टनुसार वापरला जातो आणि आपण त्यासाठी तृतीय-पक्ष ग्राफिकल फ्रंट-एंड देखील शोधू शकता. तथापि, तो बराच जुना आहे: त्याचे पहिले प्रकाशन 4 एप्रिल 2001 रोजी झाले - लेखनाच्या वेळी तो 17 वर्षांचा आहे.

मी आधी नमूद केल्याप्रमाणे, प्रकल्प खूपच लहान आहे. यात अंदाजे 30 हजार ओळी कोड आहेत, जे वय लक्षात घेता थोडे विचित्र आहे. तुलनेसाठी, FreeRDP मध्ये 320 हजार ओळी आहेत. क्लॉक प्रोग्रामचे आउटपुट येथे आहे:

अगम्य कोड

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 वापरणे

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++;
  }
  ....
}

येथे आपण फाइलच्या शेवटी पोहोचण्याचे चुकीचे हाताळणी पाहतो: जर fgetc एक वर्ण परत करतो ज्याचा कोड 0xFF आहे, तो फाइलचा शेवट म्हणून अर्थ लावला जाईल (EOF).

EOF हे एक स्थिर आहे, सहसा -1 म्हणून परिभाषित केले जाते. उदाहरणार्थ, CP1251 एन्कोडिंगमध्ये, रशियन वर्णमालाच्या शेवटच्या अक्षरात 0xFF कोड आहे, जो क्रमांक -1 शी संबंधित आहे जर आपण व्हेरिएबल बद्दल बोलत आहोत. चार. हे चिन्ह 0xFF, जसे की बाहेर वळते EOF (-1) फाईलचा शेवट असा अर्थ लावला जातो. अशा त्रुटी टाळण्यासाठी, फंक्शनचा परिणाम आहे fgetc सारख्या व्हेरिएबलमध्ये संग्रहित केले पाहिजे int.

टायपॉस

तुकडा १

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 च्या बरोबरीचे असेल (जर इतर व्हेरिएबलचे मूल्य नॉन-ऋणात्मक असेल तर), कारण मिनिट दोन पर्यायांपैकी लहान पर्याय निवडेल.
• दोन्ही व्हेरिएबल्स 0 च्या समान नाहीत: किमान मूल्य निवडा.

सह अट बदलताना write_time && change_time वर्तन योग्य दिसेल:

• एक किंवा दोन्ही व्हेरिएबल्स 0 च्या समान नाहीत: शून्य नसलेले मूल्य निवडा.
• दोन्ही व्हेरिएबल्स 0 च्या समान नाहीत: किमान मूल्य निवडा.

तुकडा १

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 — ओपन सोर्स कोडसह आरडीपी सर्व्हरची अंमलबजावणी. प्रकल्प 2 भागांमध्ये विभागलेला आहे:

• xrdp - प्रोटोकॉल अंमलबजावणी. Apache 2.0 परवान्या अंतर्गत वितरीत केले.
• xorgxrdp - xrdp सह वापरण्यासाठी Xorg ड्रायव्हर्सचा संच. परवाना - X11 (MIT प्रमाणे, परंतु जाहिरातींमध्ये वापरण्यास प्रतिबंधित करते)

प्रकल्पाचा विकास rdesktop आणि FreeRDP च्या परिणामांवर आधारित आहे. सुरुवातीला, ग्राफिक्ससह कार्य करण्यासाठी, तुम्हाला स्वतंत्र VNC सर्व्हर किंवा RDP सपोर्ट - X11rdp सह एक विशेष X11 सर्व्हर वापरावा लागला, परंतु xorgxrdp च्या आगमनाने, त्यांची गरज नाहीशी झाली.

या लेखात आम्ही xorgxrdp कव्हर करणार नाही.

xrdp प्रकल्प, मागील प्रकल्पाप्रमाणे, खूपच लहान आहे आणि त्यात अंदाजे 80 हजार ओळी आहेत.

अधिक टायपो

V525 कोडमध्ये समान ब्लॉक्सचा संग्रह आहे. 87, 88, 89 मधील आयटम 'r', 'g', 'r' तपासा. 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 लायब्ररीतून घेतला होता, जो RemoteFX साठी jpeg2000 कोडेक लागू करतो. येथे, वरवर पाहता, ग्राफिक डेटा चॅनेल मिसळले आहेत - "निळ्या" रंगाऐवजी, "लाल" रेकॉर्ड केले आहे. ही त्रुटी बहुधा कॉपी-पेस्टच्या परिणामी दिसून आली.

समान फंक्शनमध्ये हीच समस्या आली rfx_encode_format_argb, जे विश्लेषकाने आम्हाला देखील सांगितले:

V525 कोडमध्ये समान ब्लॉक्सचा संग्रह आहे. ओळी 260, 261, 262, 263 मध्ये आयटम 'a', 'r', 'g', 'r' तपासा. 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 ची चाचणी आवृत्ती डाउनलोड करू शकता साइट.

जर तुम्हाला हा लेख इंग्रजी भाषिक प्रेक्षकांसह सामायिक करायचा असेल, तर कृपया भाषांतर दुवा वापरा: सेर्गेई लॅरिन. PVS-Studio सह rdesktop आणि xrdp तपासत आहे

स्त्रोत: www.habr.com