Geekly articles weekly

Memvalidasi rdesktop dan xrdp menggunakan analisa PVS-Studio

Gambar3

Ini adalah ulasan kedua dalam serangkaian artikel tentang memeriksa program open source untuk bekerja dengan protokol RDP. Di dalamnya, kita akan mempertimbangkan klien rdesktop dan server xrdp.

PVS-Studio digunakan sebagai alat untuk mendeteksi kesalahan. Ini adalah penganalisa kode statis untuk C, C ++, C #, dan Java, tersedia di Windows, Linux, dan macOS.

Artikel ini hanya menyajikan kesalahan-kesalahan yang tampak menarik bagi saya. Namun, proyeknya kecil, jadi ada beberapa kesalahan :).

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

rdesktop


rdesktop adalah implementasi gratis klien RDP untuk sistem berbasis UNIX. Ini juga dapat digunakan di Windows, jika Anda membangun proyek di bawah Cygwin. Berlisensi berdasarkan GPLv3.

Klien ini sangat populer - digunakan secara default di ReactOS, Anda juga dapat menemukan ujung depan grafis pihak ketiga untuknya. Namun demikian, ia sudah cukup tua: rilis pertama terjadi pada 4 April 2001 - pada saat penulisan, usianya 17 tahun.

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

Gambar1


Kode tidak dapat dijangkau


V779 Kode tidak terjangkau 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 menemui kita segera di fungsi utama : kita melihat kode yang muncul setelah pernyataan kembali - fragmen ini membersihkan memori. Namun demikian, kesalahan tidak menimbulkan ancaman: semua memori yang dialokasikan dibersihkan oleh sistem operasi setelah program berakhir.

Kurangnya penanganan kesalahan


V557 Array underrun dimungkinkan. 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] = '\0'; // <= str_handle_lines(output, &rest, linehandler, data); } .... }

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

Menggunakan EOF di char


V739 EOF tidak boleh dibandingkan dengan nilai tipe 'char'. '(C = fgetc (fp))' harus dari jenis '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 pemrosesan salah mencapai akhir file: jika fgetc mengembalikan karakter yang kodenya 0xFF, maka itu akan ditafsirkan sebagai akhir file ( EOF ).

EOF adalah konstanta, biasanya didefinisikan sebagai -1. Sebagai contoh, dalam pengkodean CP1251, huruf terakhir dari alfabet Rusia memiliki kode 0xFF, yang sesuai dengan angka -1, jika kita berbicara tentang variabel tipe char . Ternyata karakter 0xFF, seperti EOF (-1), dianggap sebagai akhir file. Untuk menghindari kesalahan seperti itu, hasil dari fungsi fgetc harus disimpan dalam variabel int .

Salah ketik


Fragmen 1

Ekspresi V547 '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 bingung || dan && disediakan. Pertimbangkan opsi yang memungkinkan untuk write_time dan change_time :

  • Kedua variabel tersebut adalah 0: dalam hal ini, kita sampai ke cabang else : variabel mod_time akan selalu 0 terlepas dari kondisi berikut.
  • Salah satu variabel adalah 0: mod_time akan menjadi 0 (asalkan variabel lain memiliki nilai non-negatif), karena MIN akan memilih yang terkecil dari dua opsi.
  • Kedua variabel tidak sama dengan 0: kami memilih nilai minimum.

Saat mengganti kondisi dengan write_time && change_time, perilaku tersebut akan terlihat benar:
  • Satu atau kedua variabel tidak sama dengan 0: pilih nilai bukan nol.
  • Kedua variabel tidak sama dengan 0: kami memilih nilai minimum.

Fragmen 2

Ekspresi V547 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; .... }

Ternyata, operator || dan && , baik == dan ! = : variabel tidak dapat mengambil nilai 20 dan 9 secara bersamaan.

Menyalin baris tanpa batas


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

Ketika mempertimbangkan fungsi, menjadi sangat jelas bahwa kode ini tidak menyebabkan masalah. Namun, mereka mungkin muncul di masa depan: satu perubahan ceroboh, dan kami mendapatkan buffer overflow - sprintf tidak dibatasi oleh apa pun, jadi saat menyatukan jalur, kami bisa melampaui batas array. Dianjurkan untuk memperhatikan panggilan ini di snprintf (fullpath, PATH_MAX, ....) .

Kondisi redundan


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

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

Memeriksa add> 0 tidak berguna: variabel akan selalu lebih besar dari nol, karena baca% 4 akan mengembalikan sisa divisi, dan itu tidak akan pernah menjadi 4.

xrdp


xrdp adalah implementasi server RDP open source. 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 iklan)

Pengembangan proyek didasarkan pada hasil rdesktop dan FreeRDP. Awalnya, saya harus menggunakan server VNC terpisah untuk bekerja dengan grafik, atau server X11 khusus dengan dukungan RDP - X11rdp, tetapi dengan munculnya xorgxrdp, mereka tidak lagi diperlukan.

Pada artikel ini, kami tidak akan menyentuh xorgxrdp.

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

Gambar2


Kesalahan ketik lainnya


V525 Kode ini berisi kumpulan blok serupa. Periksa 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 pustaka librfxcodec yang mengimplementasikan codec jpeg2000 agar RemoteFX berfungsi. Di sini, tampaknya, saluran data grafis dicampur - alih-alih warna "biru", "merah" ditulis. Kesalahan seperti itu kemungkinan besar muncul sebagai hasil dari copy-paste.

Masalah yang sama jatuh ke fungsi yang serupa rfx_encode_format_argb , yang analisanya juga memberi tahu kami tentang:

V525 Kode ini berisi kumpulan blok serupa. Periksa 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 susunan


V557 Array overrun dimungkinkan. 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 dua file ini tidak kompatibel - ukurannya berbeda dengan 1. Namun, tidak ada kesalahan terjadi - ukuran yang benar ditentukan dalam file evdev-map.c, jadi tidak ada jalan keluar dari batas. Jadi ini hanyalah cacat yang mudah diperbaiki.

Perbandingan tidak valid


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

Dalam suatu fungsi, variabel bertipe unsigned short dibaca ke dalam variabel bertipe int . Pemeriksaan tidak diperlukan di sini, karena kami membaca variabel dari tipe yang tidak ditandatangani dan menetapkan hasilnya ke variabel yang lebih besar, sehingga variabel 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; } .... }

Cek ketidaksetaraan tidak masuk akal di sini, karena kami sudah memiliki perbandingan di awal. Kemungkinan ini adalah kesalahan ketik dan pengembang ingin menggunakan || untuk menyaring argumen yang tidak valid.

Kesimpulan


Selama inspeksi, tidak ada kesalahan serius yang terdeteksi, tetapi banyak kekurangan ditemukan. Namun, proyek-proyek ini digunakan dalam banyak sistem, walaupun dalam ruang lingkup kecil. Sebuah proyek kecil tidak harus memiliki banyak kesalahan, jadi Anda tidak boleh menilai pekerjaan penganalisa hanya pada proyek-proyek kecil. Anda dapat membaca lebih lanjut tentang ini di artikel " Sensasi, yang dikonfirmasi oleh angka ."

Anda dapat mengunduh versi percobaan PVS-Studio di situs web kami.



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

Source: https://habr.com/ru/post/id447880/

More articles:


All Articles