PVS-Studio विश्लेषकको साथ rdesktop र xrdp जाँच गर्दै

RDP प्रोटोकलसँग काम गर्नका लागि खुला स्रोत कार्यक्रमहरू परीक्षण गर्ने बारे लेखहरूको श्रृंखलामा यो दोस्रो समीक्षा हो। यसमा हामी rdesktop क्लाइन्ट र xrdp सर्भर हेर्नेछौं।

त्रुटिहरू पहिचान गर्न उपकरणको रूपमा प्रयोग गरिन्छ PVS- स्टूडियोयो प्लेटफर्महरूमा उपलब्ध C, C++, C# र Java को लागि स्थिर कोड विश्लेषक हो। Windows, Linux и macOS.

लेखले ती त्रुटिहरू मात्र प्रस्तुत गर्दछ जुन मलाई रोचक लाग्थ्यो। यद्यपि, परियोजनाहरू साना छन्, त्यसैले त्यहाँ केही गल्तीहरू थिए :)।

भन्नु। FreeRDP परियोजना प्रमाणीकरण बारे अघिल्लो लेख फेला पार्न सकिन्छ यहाँ.

rdesktop

rdesktop — свободная реализация клиента RDP для UNIX-based систем. Его также можно использовать и под Windows, если собирать проект под Cygwin. Лицензирован под GPLv3.

यो क्लाइन्ट धेरै लोकप्रिय छ - यो पूर्वनिर्धारित रूपमा ReactOS मा प्रयोग गरिन्छ, र तपाइँ यसको लागि तेस्रो-पक्ष ग्राफिकल फ्रन्ट-एन्डहरू पनि फेला पार्न सक्नुहुन्छ। यद्यपि, उहाँ धेरै पुरानो हुनुहुन्छ: उनको पहिलो रिलीज अप्रिल 4, 2001 मा भएको थियो - लेखनको समयमा, उहाँ 17 वर्षको हुनुहुन्छ।

मैले पहिले उल्लेख गरेझैं, परियोजना धेरै सानो छ। यसले कोडको लगभग 30 हजार लाइनहरू समावेश गर्दछ, जुन यसको उमेरलाई विचार गर्दा अलि अनौठो छ। तुलनाको लागि, FreeRDP मा 320 हजार लाइनहरू छन्। यहाँ Cloc कार्यक्रम को आउटपुट छ:

पहुँचयोग्य कोड

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, र त्यसपछि array ओभररन हुनेछ उत्पादन.

चार प्रकारमा 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 हो, यसलाई फाइलको अन्त्यको रूपमा व्याख्या गरिनेछ (ईओएफ).

ईओएफ यो एक स्थिर छ, सामान्यतया -1 को रूपमा परिभाषित। उदाहरणका लागि, CP1251 एन्कोडिङमा, रूसी वर्णमालाको अन्तिम अक्षरमा कोड 0xFF छ, जुन नम्बर -1 सँग मेल खान्छ यदि हामी चरको बारेमा कुरा गर्दैछौं भने अक्षर। यो बाहिर जान्छ कि प्रतीक 0xFF, जस्तै ईओएफ (-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; // <=
  ....
}

सायद यस कोडको लेखकले यो गलत पाउनुभयो || и && अवस्थामा। मानहरूको लागि सम्भावित विकल्पहरू विचार गरौं लेखन_समय и परिवर्तन_समय:

• दुबै चरहरू ० बराबर छन्: यस अवस्थामा हामी एउटा शाखामा समाप्त हुनेछौं अरू: चर मोड_समय पछिको अवस्थालाई ध्यान नदिई सधैं ० हुनेछ।
• चर मध्ये एउटा ० हो: मोड_समय ० बराबर हुनेछ (अर्को चरको गैर-ऋणात्मक मान छ भने), किनभने MIN दुई विकल्प मध्ये सानो छनोट गर्नेछ।
• दुबै चरहरू ० बराबर छैनन्: न्यूनतम मान छान्नुहोस्।

सर्त प्रतिस्थापन गर्दा लेखन_समय र परिवर्तन_समय व्यवहार सही देखिनेछ:

• एक वा दुबै चरहरू ० को बराबर छैनन्: गैर-शून्य मान छनौट गर्नुहोस्।
• दुबै चरहरू ० बराबर छैनन्: न्यूनतम मान छान्नुहोस्।

टुक्रा १

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)
  {
    ....
  }
}

निरीक्षण थप्नुहोस् > ० यहाँ कुनै आवश्यकता छैन: चर सधैं शून्य भन्दा ठूलो हुनेछ, किनभने % 4 पढ्नुहोस् विभाजनको बाँकी फिर्ता गर्नेछ, तर यो 4 को बराबर हुनेछैन।

xrdp

xrdp - खुला स्रोत कोडको साथ RDP सर्भरको कार्यान्वयन। परियोजना 2 भागमा विभाजित छ:

• xrdp - प्रोटोकल कार्यान्वयन। Apache 2.0 लाइसेन्स अन्तर्गत वितरित।
• xorgxrdp - xrdp सँग प्रयोगको लागि Xorg ड्राइभरहरूको सेट। इजाजतपत्र - X11 (जस्तै MIT, तर विज्ञापनमा प्रयोग निषेधित गर्दछ)

परियोजनाको विकास rdesktop र FreeRDP को परिणामहरूमा आधारित छ। सुरुमा, ग्राफिक्स संग काम गर्न को लागी, तपाईले छुट्टै VNC सर्भर, वा RDP समर्थन - X11rdp को साथ एक विशेष X11 सर्भर प्रयोग गर्नुपर्थ्यो, तर xorgxrdp को आगमन संग, तिनीहरूको आवश्यकता गायब भयो।

यस लेखमा हामी xorgxrdp लाई कभर गर्दैनौं।

xrdp परियोजना, अघिल्लो जस्तै, धेरै सानो छ र लगभग 80 हजार लाइनहरू समावेश गर्दछ।

थप टाईपहरू

V525 कोडले समान ब्लकहरूको संग्रह समावेश गर्दछ। वस्तुहरू 'r', 'g', 'r' लाई 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++;
      }
      ....
  }
  ....
}

यो कोड librfxcodec पुस्तकालयबाट लिइएको हो, जसले RemoteFX को लागि jpeg2000 कोडेक लागू गर्दछ। यहाँ, स्पष्ट रूपमा, ग्राफिक डेटा च्यानलहरू मिश्रित छन् - "नीलो" रङको सट्टा, "रातो" रेकर्ड गरिएको छ। यो त्रुटि प्राय: प्रतिलिपि टाँस्ने परिणामको रूपमा देखा पर्‍यो।

एउटै प्रकार्यमा उस्तै समस्या भयो rfx_encode_format_argb, जुन विश्लेषकले हामीलाई भने:

V525 कोडले समान ब्लकहरूको संग्रह समावेश गर्दछ। वस्तुहरू 'a', 'r', 'g', 'r' लाई 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++;
}

एरे घोषणा

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

यी दुई फाइलहरूमा array को घोषणा र परिभाषा असंगत छन् - आकार 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