Bu, açık kaynak programların RDP protokolüyle çalışmasını test etmeye ilişkin bir dizi makalenin ikinci incelemesidir. İçinde rdesktop istemcisine ve xrdp sunucusuna bakacağız.
Hataları tanımlamak için bir araç olarak kullanılır . Windows, Linux ve macOS platformlarında bulunan C, C++, C# ve Java dilleri için statik bir kod analizörüdür.
Makale yalnızca benim için ilginç görünen hataları sunuyor. Ancak projeler küçük olduğundan çok az hata vardı :).
Dikkat. FreeRDP proje doğrulamasıyla ilgili önceki bir makaleyi bulabilirsiniz .
rmasaüstü
— UNIX tabanlı sistemler için bir RDP istemcisinin ücretsiz uygulaması. Projeyi Cygwin altında oluşturursanız Windows altında da kullanılabilir. GPLv3 kapsamında lisanslanmıştır.
Bu istemci çok popülerdir; ReactOS'ta varsayılan olarak kullanılır ve bunun için üçüncü taraf grafiksel ön uçlar da bulabilirsiniz. Ancak oldukça yaşlı: ilk çıkışı 4 Nisan 2001'de gerçekleşti - bu yazının yazıldığı sırada 17 yaşındaydı.
Daha önce de belirttiğim gibi proje çok küçük. Yaklaşık 30 bin satırlık kod barındırıyor, bu da yaşı göz önüne alındığında biraz garip. Karşılaştırma için FreeRDP 320 bin satır içeriyor. Cloc programının çıktısı şu şekildedir:
Ulaşılamaz kod
Kullanılamayan kod algılandı. Bir hatanın mevcut olması mümkündür. 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);
}Hata fonksiyonda hemen karşımıza çıkıyor ana: operatörden sonra gelen kodu görüyoruz dönüş — bu parça hafıza temizliğini gerçekleştirir. Ancak hata bir tehdit oluşturmaz: ayrılan tüm bellek, programdan çıktıktan sonra işletim sistemi tarafından temizlenecektir.
Hata işleme yok
Dizinin yetersiz çalışması mümkündür. 'n' indeksinin değeri -1'e ulaşabilir. 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);
}
....
}Bu durumda kod pasajı, dosya bitene kadar dosyadan ara belleğe okunur. Ancak burada hata işleme söz konusu değildir: eğer bir şeyler ters giderse, o zaman okumak -1 değerini döndürecek ve ardından dizi taşacak çıktı.
Karakter türünde EOF kullanma
EOF, 'char' tipindeki bir değerle karşılaştırılmamalıdır. '(c = fgetc(fp))', 'int' türünde olmalıdır. 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++;
}
....
}Burada dosyanın sonuna ulaşmanın hatalı bir şekilde ele alındığını görüyoruz: if fgetc kodu 0xFF olan bir karakter döndürür, dosyanın sonu olarak yorumlanır (EOF).
EOF genellikle -1 olarak tanımlanan bir sabittir. Örneğin CP1251 kodlamasında Rus alfabesinin son harfi 0xFF koduna sahiptir, bu da eğer bir değişkenden bahsediyorsak -1 rakamına karşılık gelir. tank. 0xFF sembolünün şöyle olduğu ortaya çıktı: EOF (-1) dosyanın sonu olarak yorumlanır. Bu tür hataları önlemek için fonksiyonun sonucu fgetc gibi bir değişkende saklanmalıdır int.
yazım hataları
Parça 1
'write_time' ifadesi her zaman yanlıştır. 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; // <=
....
}Belki bu kodun yazarı yanlış anladı || и && durumda. Değerler için olası seçenekleri ele alalım yazma_zamanı и zamanı değiştir:
- Her iki değişken de 0'a eşittir: bu durumda bir dalda olacağız başka: değişken mod_time sonraki koşuldan bağımsız olarak her zaman 0 olacaktır.
- Değişkenlerden biri 0'dır: mod_time 0'a eşit olacaktır (diğer değişkenin negatif olmayan bir değere sahip olması şartıyla), çünkü MIN iki seçenekten küçük olanı seçecektir.
- Her iki değişken de 0'a eşit değildir: minimum değeri seçin.
Koşulu şununla değiştirirken: yazma_zamanı && değişiklik_zamanı davranış doğru görünecek:
- Değişkenlerden biri veya her ikisi de 0'a eşit değil: sıfır olmayan bir değer seçin.
- Her iki değişken de 0'a eşit değildir: minimum değeri seçin.
Parça 2
İfade her zaman doğrudur. Muhtemelen burada '&&' operatörü kullanılmalıdır. 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;
....
}Görünüşe göre operatörler burada da karıştı || и &&Veya == и !=: Bir değişken aynı anda 20 ve 9 değerine sahip olamaz.
Sınırsız satır kopyalama
'Sprintf' fonksiyonunun çağrılması, 'tam yol' arabelleğinin taşmasına yol açacaktır. 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);
....
}Fonksiyonun tamamına baktığınızda bu kodun sorun yaratmadığı anlaşılacaktır. Ancak gelecekte ortaya çıkabilirler: Dikkatsiz bir değişiklik yaparsak arabellek taşması yaşanır - sprintf hiçbir şeyle sınırlı değildir, dolayısıyla yolları birleştirirken dizinin sınırlarının ötesine geçebiliriz. Bu çağrıya dikkat etmeniz önerilir. snprintf(tam yol, PATH_MAX, ….).
Yedekli durum
Koşullu ifadenin bir kısmı her zaman doğrudur: ekle > 0. Scard.c 507
static void
inRepos(STREAM in, unsigned int read)
{
SERVER_DWORD add = 4 - read % 4;
if (add < 4 && add > 0)
{
....
}
}Kontrol ekle > 0 burada buna gerek yok: değişken her zaman sıfırdan büyük olacaktır çünkü % 4'ü oku bölümün geri kalanını döndürür, ancak hiçbir zaman 4'e eşit olmayacaktır.
xrdp
— açık kaynak kodlu bir RDP sunucusunun uygulanması. Proje 2 bölüme ayrılmıştır:
- xrdp - protokol uygulaması. Apache 2.0 lisansı altında dağıtılmaktadır.
- xorgxrdp - xrdp ile kullanım için bir dizi Xorg sürücüsü. Lisans - X11 (MIT gibi, ancak reklamlarda kullanılması yasaktır)
Projenin geliştirilmesi rdesktop ve FreeRDP sonuçlarına dayanmaktadır. Başlangıçta, grafiklerle çalışmak için ayrı bir VNC sunucusu veya RDP destekli özel bir X11 sunucusu - X11rdp kullanmanız gerekiyordu, ancak xorgxrdp'nin gelişiyle bunlara olan ihtiyaç ortadan kalktı.
Bu yazıda xorgxrdp'yi ele almayacağız.
Xrdp projesi de bir önceki proje gibi oldukça küçük ve yaklaşık 80 bin satır içeriyor.
Daha fazla yazım hatası
Kod benzer blokların koleksiyonunu içerir. 87, 88, 89. satırlardaki 'r', 'g', 'r' öğelerini kontrol edin. 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++;
}
....
}
....
}Bu kod, RemoteFX için jpeg2000 codec bileşenini uygulayan librfxcodec kitaplığından alınmıştır. Görünüşe göre burada grafik veri kanalları karışmış - "mavi" renk yerine "kırmızı" kaydediliyor. Bu hata büyük olasılıkla kopyala-yapıştır sonucu ortaya çıktı.
Aynı sorun benzer bir işlevde de ortaya çıktı rfx_encode_format_argbanalizcinin bize ayrıca söylediği:
Kod benzer blokların koleksiyonunu içerir. 260, 261, 262, 263. satırlardaki 'a', 'r', 'g', 'r' öğelerini kontrol edin. rfxencode_rgb_to_yuv.c 260
while (x < 64)
{
*la_buf++ = a;
*lr_buf++ = r;
*lg_buf++ = g;
*lb_buf++ = r;
x++;
}Dizi Bildirimi
Dizi taşması mümkündür. 'i — 8' endeksinin değeri 129'a ulaşabilir. 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];
....
}
....
}Bu iki dosyadaki dizinin bildirimi ve tanımı uyumsuzdur - boyut farkı 1'dir. Ancak herhangi bir hata oluşmaz - evdev-map.c dosyasında doğru boyut belirtilmiştir, dolayısıyla sınırların dışına çıkılmamaktadır. Yani bu sadece kolayca düzeltilebilecek bir hatadır.
Yanlış karşılaştırma
Koşullu ifadenin bir kısmı her zaman yanlıştır: (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))
{
....
}
....
}İşlev bir tür değişkenini okur imzasız kısa gibi bir değişkene int. Burada işaretsiz bir değişken okuduğumuz ve sonucu daha büyük bir değişkene atadığımız için kontrol etmeye gerek yoktur, dolayısıyla değişken negatif değer alamaz.
Gereksiz kontroller
Koşullu ifadenin bir kısmı her zaman doğrudur: (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;
}
....
}Başlangıçta zaten bir karşılaştırma yaptığımız için eşitsizlik kontrollerinin burada bir anlamı yok. Bunun bir yazım hatası olması muhtemeldir ve geliştirici operatörü kullanmak istemiştir || geçersiz bağımsız değişkenleri filtrelemek için.
Sonuç
Denetim sırasında ciddi bir hata tespit edilmedi ancak birçok eksiklik bulundu. Ancak bu tasarımlar kapsamı küçük de olsa birçok sistemde kullanılmaktadır. Küçük bir projede mutlaka çok fazla hata olması gerekmez; bu nedenle analizörün performansını yalnızca küçük projelere göre değerlendirmemelisiniz. Bununla ilgili daha fazla bilgiyi "".
PVS-Studio'nun deneme sürümünü bizden indirebilirsiniz .
Bu makaleyi İngilizce konuşan bir kitleyle paylaşmak istiyorsanız lütfen çeviri bağlantısını kullanın: Sergey Larin.
Kaynak: habr.com