Kami menembak di kaki, memproses data input


Tautan dalam artikel hari ini berbeda dari biasanya. Ini bukan satu proyek di mana kode sumber dianalisis, tetapi serangkaian tanggapan dari aturan diagnostik yang sama di beberapa proyek yang berbeda. Apa yang menarik di sini? Faktanya adalah bahwa beberapa fragmen kode dianggap mengandung kesalahan yang dapat direproduksi saat bekerja dengan aplikasi, sementara yang lain mengandung kerentanan bersama (CVE). Selain itu, di akhir artikel kita akan membahas sedikit tentang topik cacat keamanan.

Kata Pengantar Singkat


Semua kesalahan yang akan dipertimbangkan hari ini di artikel memiliki pola yang sama:

  • program menerima data dari aliran stdin ;
  • pemeriksaan dilakukan untuk keberhasilan membaca data;
  • jika data berhasil dibaca, karakter carry dihapus dari baris.

Namun, semua fragmen yang akan dianggap mengandung kesalahan dan rentan terhadap input yang dicurangi. Karena data diterima dari pengguna, yang mungkin melanggar logika eksekusi aplikasi, ada godaan besar untuk mencoba memecahkan sesuatu. Yang saya lakukan.

Semua masalah di bawah ini ditemukan oleh analisa statis PVS-Studio , yang mencari kesalahan dalam kode tidak hanya untuk C, C ++, tetapi juga untuk C #, Java.

Tentu saja, menemukan masalah dengan penganalisa statis itu baik, tetapi menemukan dan mereproduksi adalah tingkat kesenangan yang sama sekali berbeda. :)

Freeswitch


Fragmen kode mencurigakan pertama ditemukan dalam kode modul fs_cli.exe , yang merupakan bagian dari kit distribusi FreeSWITCH:

static const char *basic_gets(int *cnt) { .... int c = getchar(); if (c < 0) { if (fgets(command_buf, sizeof(command_buf) - 1, stdin) != command_buf) { break; } command_buf[strlen(command_buf)-1] = '\0'; /* remove endline */ break; } .... } 

Peringatan PVS-Studio : V1010 CWE-20 Data tercemar yang tidak dicentang digunakan dalam indeks: 'strlen (command_buf)'.

Penganalisa memperingatkan panggilan yang mencurigakan oleh indeks ke array command_buf . Itu dianggap mencurigakan karena alasan bahwa data eksternal yang tidak diverifikasi digunakan sebagai indeks. Eksternal - karena mereka diperoleh melalui fungsi fgets dari aliran stdin . Tidak diverifikasi - karena tidak ada verifikasi yang dilakukan sebelum digunakan. Ungkapan ekspresi (command_buf, ....)! = Command_buf tidak masuk hitungan, karena dengan cara ini kami hanya memeriksa fakta menerima data, tetapi tidak isinya.

Masalah dengan kode ini adalah bahwa, dalam kondisi tertentu, '\ 0' akan ditulis di luar array, yang akan menyebabkan perilaku tidak terdefinisi. Untuk melakukan ini, cukup masukkan string dengan panjang nol (string dengan panjang nol dari sudut pandang bahasa C, yaitu, di mana karakter pertama adalah '\ 0').

Mari kita perkirakan apa yang terjadi jika Anda meneruskan string nol-panjang ke input:

  • fgets (command_buf, ....) -> command_buf ;
  • fgets (....)! = command_buf -> false ( maka cabang pernyataan if diabaikan);
  • strlen (command_buf) -> 0 ;
  • command_buf [strlen (command_buf) - 1] -> command_buf [-1] .

Ups!

Yang menarik di sini adalah bahwa peringatan analis ini dapat "dirasakan dengan tangan Anda." Untuk mengulangi masalah, Anda perlu:

  • membawa program ke fungsi ini;
  • fine-tune input sehingga panggilan getchar () mengembalikan nilai negatif;
  • Lewat untuk fungsi fiksi garis dengan terminal nol di awal, yang seharusnya berhasil dibaca.

Mengaduk-aduk sedikit di sumber, saya membuat urutan khusus untuk mereproduksi masalah:

  • Jalankan fs_cli.exe dalam mode batch ( fs_cli.exe -b ). Saya perhatikan bahwa untuk melakukan langkah lebih lanjut, koneksi fs_cli.exe ke server harus berhasil. Untuk melakukan ini, cukup, misalnya, untuk menjalankan FreeSwitchConsole.exe secara lokal sebagai administrator.
  • Kami melakukan input sehingga panggilan getchar () mengembalikan nilai negatif.
  • Masukkan baris dengan nol terminal di awal (misalnya, '\ 0Oooops').
  • ....
  • KEUNTUNGAN!

Berikut ini adalah pemutaran video masalahnya:


Ncftp


Masalah serupa ditemukan di proyek NcFTP, tetapi sudah bertemu di dua tempat. Karena kode terlihat mirip, pertimbangkan hanya satu tempat bermasalah:

 static int NcFTPConfirmResumeDownloadProc(....) { .... if (fgets(newname, sizeof(newname) - 1, stdin) == NULL) newname[0] = '\0'; newname[strlen(newname) - 1] = '\0'; .... } 

Peringatan PVS-Studio : V1010 CWE-20 Data tercemar yang tidak dicentang digunakan dalam indeks: 'strlen (nama baru)'.

Di sini, tidak seperti contoh dari FreeSWITCH, kode ini ditulis lebih buruk dan lebih rentan terhadap masalah. Misalnya, penulisan '\ 0' terjadi terlepas dari apakah bacaan itu berhasil menggunakan widget atau tidak. Artinya, ada lebih banyak lagi kemungkinan untuk cara mematahkan logika eksekusi normal. Mari kita pergi dengan cara yang terbukti - melalui garis nol panjang.

Masalah yang direproduksi sedikit lebih rumit daripada dengan FreeSWITCH. Urutan langkah-langkah dijelaskan di bawah ini:

  • mulai dan sambungkan ke server tempat Anda dapat mengunduh file. Sebagai contoh, saya menggunakan speedtest.tele2.net (pada akhirnya, perintah peluncuran aplikasi terlihat seperti ini: ncftp.exe speedtest.tele2.net );
  • mengunduh file dari server. Secara lokal, file dengan nama yang sama tetapi dengan properti yang berbeda harus sudah ada. Anda dapat, misalnya, mengunduh file dari server, mengubahnya, dan mencoba menjalankan perintah unduhan lagi (misalnya, dapatkan 512KB.zip );
  • jawab pertanyaan tentang memilih tindakan dengan garis yang dimulai dengan karakter 'N' (misalnya, Sekarang mari bersenang-senang );
  • masukkan '\ 0' (atau sesuatu yang lebih menarik);
  • ....
  • KEUNTUNGAN!

Reproduksi masalah juga direkam di video:


Openldap


Dalam proyek OpenLDAP (lebih tepatnya, di salah satu utilitas yang menyertainya) mereka menginjak menyapu yang sama seperti di FreeSWITCH. Upaya untuk menghapus karakter pemisah baris hanya terjadi jika baris berhasil dibaca, tetapi juga tidak ada perlindungan terhadap garis panjang nol.

Cuplikan kode:

 int main( int argc, char **argv ) { char buf[ 4096 ]; FILE *fp = NULL; .... if (....) { fp = stdin; } .... if ( fp == NULL ) { .... } else { while ((rc == 0 || contoper) && fgets(buf, sizeof(buf), fp) != NULL) { buf[ strlen( buf ) - 1 ] = '\0'; /* remove trailing newline */ if ( *buf != '\0' ) { rc = dodelete( ld, buf ); if ( rc != 0 ) retval = rc; } } } .... } 

Peringatan PVS-Studio : V1010 CWE-20 Data tercemar yang tidak dicentang digunakan dalam indeks: 'strlen (buf)'.

Kami membuang kelebihan sehingga esensi masalah menjadi lebih jelas:

 while (.... && fgets(buf, sizeof(buf), fp) != NULL) { buf[ strlen( buf ) - 1 ] = '\0'; .... } 

Kode ini lebih baik dari NcFTP, tetapi masih rentan. Jika ada permintaan, masukkan string dengan panjang nol ke input:

  • fgets (buf, ....) -> buf ;
  • fgets (....)! = NULL -> true (isi dari while while mulai dijalankan);
  • strlen (buf) - 1 -> 0 - 1 -> -1 ;
  • buf [-1] = '\ 0' .

libidn


Terlepas dari kenyataan bahwa kesalahan yang dibahas di atas cukup menarik (mereka direproduksi secara stabil, dan mereka dapat "disentuh" ​​(kecuali saya tidak bisa menyentuh masalah OpenLDAP)), mereka tidak dapat disebut kerentanan, jika hanya karena alasan bahwa masalah tidak diberikan pengidentifikasi CVE.

Namun, beberapa kerentanan nyata memiliki pola masalah yang sama. Kedua cuplikan kode di bawah ini berlaku untuk proyek libidn.

Cuplikan kode:

 int main (int argc, char *argv[]) { .... else if (fgets (readbuf, BUFSIZ, stdin) == NULL) { if (feof (stdin)) break; error (EXIT_FAILURE, errno, _("input error")); } if (readbuf[strlen (readbuf) - 1] == '\n') readbuf[strlen (readbuf) - 1] = '\0'; .... } 

Peringatan PVS-Studio : V1010 CWE-20 Data tercemar yang tidak dicentang digunakan dalam indeks: 'strlen (readbuf)'.

Situasinya mirip, kecuali bahwa, tidak seperti contoh sebelumnya, di mana rekaman dilakukan pada indeks -1 , pembacaan terjadi di sini. Namun, ini masih merupakan perilaku yang tidak terdefinisi. Kesalahan ini telah ditetapkan pengidentifikasi CVE- nya sendiri ( CVE-2015-8948 ).

Setelah mendeteksi masalah, kode ini diubah sebagai berikut:

 int main (int argc, char *argv[]) { .... else if (getline (&line, &linelen, stdin) == -1) { if (feof (stdin)) break; error (EXIT_FAILURE, errno, _("input error")); } if (line[strlen (line) - 1] == '\n') line[strlen (line) - 1] = '\0'; .... } 

Agak kaget? Itu terjadi. Kerentanan baru, pengidentifikasi CVE yang sesuai: CVE-2016-6262 .

Peringatan PVS-Studio : V1010 CWE-20 Data tercemar yang tidak dicentang digunakan dalam indeks: 'strlen (line)'.

Pada upaya lain, masalahnya diperbaiki dengan menambahkan tanda centang untuk panjang string masukan:

 if (strlen (line) > 0) if (line[strlen (line) - 1] == '\n') line[strlen (line) - 1] = '\0'; 

Mari kita lihat tanggalnya. Komit 'penutupan' CVE-2015-8948 - 08/10/2015. Penutupan komitmen CVE-2016-62-62 - 14/01/2016. Artinya, perbedaan antara koreksi di atas adalah 5 bulan ! Di sini Anda ingat tentang keunggulan analisis statis seperti mendeteksi kesalahan pada tahap awal penulisan kode ...

Analisis dan Keamanan Statis


Tidak akan ada contoh kode lebih lanjut, sebaliknya, statistik dan alasan. Pada bagian ini, pendapat penulis mungkin tidak sesuai dengan pendapat pembaca lebih dari sebelumnya dalam artikel ini.

Catatan Saya sarankan Anda membaca artikel lain tentang topik serupa - " Bagaimana PVS-Studio dapat membantu dalam menemukan kerentanan? ". Ada beberapa contoh kerentanan yang menarik yang terlihat seperti kesalahan sederhana. Selain itu, dalam artikel itu saya berbicara sedikit tentang terminologi dan mengapa analisis statis harus dimiliki jika Anda khawatir tentang topik keamanan.

Mari kita lihat statistik jumlah kerentanan yang ditemukan selama 10 tahun terakhir untuk menilai situasi. Saya mengambil data dari situs web CVE Details .

Gambar 2



Situasi yang menarik tampak. Hingga 2014, jumlah CVE yang tercatat tidak melebihi tanda 6.000 unit, dan mulai dari - tidak turun di bawah. Tapi yang paling menarik di sini, tentu saja, adalah statistik untuk 2017 - pemimpin absolut (14.714 unit). Adapun saat ini - 2018 - tahun, itu belum berakhir, tetapi sudah memecahkan rekor - 15.310 unit.

Apakah ini berarti bahwa semua perangkat lunak baru penuh dengan lubang seperti saringan? Saya tidak berpikir, dan inilah alasannya:

  • Meningkatnya minat pada topik kerentanan. Tentunya, bahkan jika Anda tidak terlalu dekat dengan topik keamanan, Anda telah berulang kali menemukan artikel, catatan, laporan, dan video tentang topik keamanan. Dengan kata lain, semacam "hype" telah dibuat. Apakah itu buruk? Mungkin tidak. Pada akhirnya, semuanya bermuara pada kenyataan bahwa pengembang lebih memperhatikan keamanan aplikasi, yang bagus.
  • Peningkatan jumlah aplikasi yang dikembangkan. Semakin banyak kode - semakin besar kemungkinan terjadi kerentanan yang akan mengisi kembali statistik.
  • Alat pencarian kerentanan yang ditingkatkan dan jaminan kualitas kode. Lebih banyak permintaan -> lebih banyak pasokan. Analisis, fuzzer dan alat-alat lain menjadi lebih maju, yang berperan di tangan mereka yang ingin mencari kerentanan (terlepas dari sisi mana barikade mereka berada).

Jadi, tren yang muncul tidak bisa disebut negatif secara eksklusif - penerbit lebih mementingkan keamanan informasi, alat untuk menemukan masalah sedang diperbaiki, dan semua ini pasti positif.

Apakah ini berarti Anda bisa santai dan tidak "mandi"? Saya kira tidak. Jika Anda khawatir tentang topik keamanan aplikasi Anda, Anda harus mengambil langkah keamanan sebanyak mungkin. Ini benar terutama jika kode sumber ada dalam domain publik, karena:

  • lebih rentan untuk menanamkan kerentanan eksternal;
  • lebih rentan untuk "menyelidiki" oleh "tuan-tuan" yang tertarik pada lubang di aplikasi Anda untuk tujuan mengeksploitasi mereka. Meskipun simpatisan baik dalam hal ini akan dapat membantu Anda lebih banyak.

Saya tidak ingin mengatakan bahwa Anda tidak perlu menerjemahkan proyek Anda di bawah sumber terbuka. Ingatlah selalu kontrol kualitas / keamanan yang sesuai.

Apakah analisis statis merupakan ukuran tambahan? Ya Analisis statis melakukan pekerjaan yang baik untuk mendeteksi potensi kerentanan yang dapat menjadi nyata di masa depan.

Menurut saya (saya akui saya salah) bahwa banyak orang menganggap kerentanan sebagai fenomena tingkat tinggi. Ya dan tidak Masalah kode yang tampak seperti kesalahan pemrograman sederhana juga bisa menjadi kerentanan serius. Sekali lagi, beberapa contoh kerentanan tersebut disediakan dalam artikel yang disebutkan sebelumnya . Jangan meremehkan kesalahan 'sederhana'.

Kesimpulan


Jangan lupa bahwa data input dapat memiliki panjang nol, dan ini juga perlu diperhitungkan.

Kesimpulan tentang apakah semua hype dengan kerentanan hanyalah hype, atau jika masalahnya ada, lakukan sendiri.

Untuk bagian saya, kecuali saya mengusulkan untuk mencoba proyek PVS-Studio Anda , jika Anda belum melakukannya.

Semua yang terbaik!



Jika Anda ingin berbagi artikel ini dengan audiens yang berbahasa Inggris, silakan gunakan tautan ke terjemahan: Sergey Vasiliev. Tembak kaki Anda saat memegang data input

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


All Articles