ProHoster > Blog > administrasi > Memeriksa rdesktop dan xrdp dengan penganalisis PVS-Studio

Memeriksa rdesktop dan xrdp dengan penganalisis PVS-Studio

Memeriksa rdesktop dan xrdp menggunakan penganalisa PVS-Studio
Ini adalah ulasan kedua dalam serangkaian artikel tentang pengujian program sumber terbuka untuk bekerja dengan protokol RDP. Di dalamnya kita akan melihat klien rdesktop dan server xrdp.

Digunakan sebagai alat untuk mengidentifikasi kesalahan PVS-StudioIni adalah penganalisis kode statis untuk C, C++, C# dan Java, tersedia di berbagai platform. Windows, Linux и macOS.

Artikel ini hanya menyajikan kesalahan-kesalahan yang menurut saya menarik. Namun, proyeknya kecil, jadi hanya ada sedikit kesalahan :).

Catatan. Artikel sebelumnya tentang verifikasi proyek FreeRDP dapat ditemukan di sini.

desktop

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

Klien ini sangat populer - digunakan secara default di ReactOS, dan Anda juga dapat menemukan front-end grafis pihak ketiga untuk klien ini. Namun, dia sudah cukup tua: rilis pertamanya terjadi pada tanggal 4 April 2001 - pada saat penulisan, dia berusia 17 tahun.

Seperti yang saya sebutkan sebelumnya, proyek ini cukup kecil. Ini berisi sekitar 30 ribu baris kode, yang agak aneh mengingat usianya. Sebagai perbandingan, FreeRDP berisi 320 ribu baris. Berikut keluaran program Cloc:

Memeriksa rdesktop dan xrdp menggunakan penganalisa PVS-Studio

Kode yang tidak dapat dijangkau

V779 Kode yang tidak tersedia terdeteksi. Mungkin saja ada kesalahan. 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);
}

Kesalahan langsung menemui kita di fungsinya utama: kita melihat kode muncul setelah operator kembali — fragmen ini melakukan pembersihan memori. Namun, kesalahan tersebut tidak menimbulkan ancaman: semua memori yang dialokasikan akan dihapus oleh sistem operasi setelah program keluar.

Tidak ada penanganan kesalahan

V557 Array yang tidak berjalan mungkin terjadi. Nilai indeks 'n' bisa mencapai -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);
  }
  ....
}

Cuplikan kode dalam hal ini dibaca dari file ke dalam buffer hingga file berakhir. Namun, tidak ada penanganan kesalahan di sini: jika terjadi kesalahan, maka Baca baca akan mengembalikan -1, dan kemudian array akan dibanjiri keluaran.

Menggunakan EOF dalam tipe char

V739 EOF tidak boleh dibandingkan dengan nilai tipe 'char'. '(c = fgetc(fp))' harus bertipe '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++;
  }
  ....
}

Di sini kita melihat penanganan yang salah saat mencapai akhir file: if fgetc mengembalikan karakter yang kodenya 0xFF, itu akan diartikan sebagai akhir file (EOF).

EOF itu adalah konstanta, biasanya didefinisikan sebagai -1. Misalnya, dalam pengkodean CP1251, huruf terakhir alfabet Rusia memiliki kode 0xFF, yang sesuai dengan angka -1 jika kita berbicara tentang variabel seperti tangki. Ternyata simbolnya 0xFF, seperti EOF (-1) diartikan sebagai akhir file. Untuk menghindari kesalahan seperti itu, hasil dari fungsinya adalah fgetc harus disimpan dalam variabel seperti int.

salah ketik

Fragmen 1

V547 Ekspresi 'write_time' selalu salah. 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; // <=
  ....
}

Mungkin pembuat kode ini salah || и && dalam kondisi. Mari kita pertimbangkan opsi nilai yang memungkinkan waktu_tulis и perubahan_waktu:

  • Kedua variabel sama dengan 0: dalam hal ini kita akan berakhir di sebuah cabang lain: variabel mod_time akan selalu 0 terlepas dari kondisi selanjutnya.
  • Salah satu variabelnya adalah 0: mod_time akan sama dengan 0 (asalkan variabel lain bernilai non-negatif), karena MIN akan memilih yang lebih kecil dari dua opsi.
  • Kedua variabel tidak sama dengan 0: pilih nilai minimum.

Saat mengganti kondisi dengan waktu_tulis && waktu_perubahan perilakunya akan terlihat benar:

  • Salah satu atau kedua variabel tidak sama dengan 0: pilih nilai yang bukan nol.
  • Kedua variabel tidak sama dengan 0: pilih nilai minimum.

Fragmen 2

V547 Ekspresi selalu benar. Mungkin operator '&&' harus digunakan di sini. 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;
  ....
}

Rupanya operator juga ikut campur di sini || и &&Atau == и !=: Suatu variabel tidak boleh memiliki nilai 20 dan 9 secara bersamaan.

Penyalinan baris tanpa batas

V512 Pemanggilan fungsi 'sprintf' akan menyebabkan buffer 'fullpath' meluap. 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);
  ....
}

Jika Anda melihat fungsinya secara lengkap, terlihat jelas bahwa kode ini tidak menimbulkan masalah. Namun, hal tersebut mungkin muncul di masa mendatang: satu perubahan yang ceroboh dan kita akan mendapatkan buffer overflow - lari cepat tidak dibatasi oleh apa pun, jadi saat menggabungkan jalur, kita bisa melampaui batas array. Disarankan untuk memperhatikan panggilan ini snprintf(jalur lengkap, PATH_MAX,….).

Kondisi berlebihan

V560 Bagian dari ekspresi kondisional selalu benar: tambahkan > 0. scard.c 507

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

Проверка tambahkan > 0 tidak perlu di sini: variabel akan selalu lebih besar dari nol, karena membaca% 4 akan mengembalikan sisa pembagian, tetapi tidak akan pernah sama dengan 4.

xrdp

xrdp — implementasi server RDP dengan kode sumber terbuka. Proyek ini dibagi menjadi 2 bagian:

  • xrdp - implementasi protokol. Didistribusikan di bawah lisensi Apache 2.0.
  • xorgxrdp - Satu set driver Xorg untuk digunakan dengan xrdp. Lisensi - X11 (seperti MIT, tetapi melarang penggunaan dalam periklanan)

Pengembangan proyek ini didasarkan pada hasil rdesktop dan FreeRDP. Awalnya, untuk bekerja dengan grafik, Anda harus menggunakan server VNC terpisah, atau server X11 khusus dengan dukungan RDP - X11rdp, tetapi dengan munculnya xorgxrdp, kebutuhan akan server tersebut menghilang.

Pada artikel ini kami tidak akan membahas xorgxrdp.

Proyek xrdp, seperti proyek sebelumnya, berukuran sangat kecil dan berisi sekitar 80 ribu baris.

Memeriksa rdesktop dan xrdp menggunakan penganalisa PVS-Studio

Lebih banyak kesalahan ketik

V525 Kode tersebut berisi kumpulan blok serupa. Centang item 'r', 'g', 'r' pada baris 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++;
      }
      ....
  }
  ....
}

Kode ini diambil dari perpustakaan librfxcodec, yang mengimplementasikan codec jpeg2000 untuk RemoteFX. Di sini, tampaknya, saluran data grafis tercampur - alih-alih warna "biru", "merah" yang direkam. Kesalahan ini kemungkinan besar muncul akibat copy-paste.

Masalah yang sama terjadi pada fungsi serupa rfx_encode_format_argb, yang juga diberitahukan oleh penganalisa kepada kami:

V525 Kode tersebut berisi kumpulan blok serupa. Centang item 'a', 'r', 'g', 'r' pada baris 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++;
}

Deklarasi Array

V557 Pembebanan array mungkin terjadi. Nilai indeks 'i — 8' bisa mencapai 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];
    ....
  }
  ....
}

Deklarasi dan definisi array dalam kedua file ini tidak kompatibel - ukurannya berbeda 1. Namun, tidak ada kesalahan yang terjadi - ukuran yang benar ditentukan dalam file evdev-map.c, jadi tidak ada batasan. Jadi ini hanya bug yang bisa diperbaiki dengan mudah.

Perbandingan yang salah

V560 Bagian dari ekspresi kondisional selalu salah: (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))
  {
    ....
  }
  ....
}

Fungsi membaca variabel tipe pendek unsigned menjadi variabel seperti int. Pengecekan tidak diperlukan disini karena kita membaca variabel unsigned dan menugaskan hasilnya ke variabel yang lebih besar, sehingga variabel tersebut tidak dapat mengambil nilai negatif.

Pemeriksaan yang tidak perlu

V560 Bagian dari ekspresi kondisional selalu benar: (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;
  }
  ....
}

Pemeriksaan ketimpangan tidak masuk akal di sini karena kita sudah mempunyai perbandingan di awal. Kemungkinan ini salah ketik dan pengembang ingin menggunakan operator tersebut || untuk menyaring argumen yang tidak valid.

Kesimpulan

Selama audit, tidak ditemukan kesalahan serius, namun banyak ditemukan kekurangan. Namun, desain ini digunakan di banyak sistem, meskipun cakupannya kecil. Sebuah proyek kecil belum tentu memiliki banyak kesalahan, jadi Anda tidak boleh menilai kinerja penganalisis hanya pada proyek kecil. Anda dapat membaca lebih lanjut tentang ini di artikel “Perasaan yang dikonfirmasi oleh angka".

Anda dapat mengunduh PVS-Studio versi uji coba dari kami Online.

Memeriksa rdesktop dan xrdp menggunakan penganalisa PVS-Studio

Jika Anda ingin membagikan artikel ini kepada audiens berbahasa Inggris, silakan gunakan tautan terjemahan: Sergey Larin. Memeriksa rdesktop dan xrdp dengan PVS-Studio

Sumber: www.habr.com

Beli hosting yang andal untuk situs dengan perlindungan DDoS, server VPS VDS 🔥 Beli hosting website andal dengan perlindungan DDoS, server VPS VDS | ProHoster