Bu, RDP protokolu ilə işləmək üçün açıq mənbə proqramlarının sınaqdan keçirilməsinə dair bir sıra məqalələrdə ikinci baxışdır. Orada rdesktop müştəri və xrdp serverinə baxacağıq.
Səhvləri müəyyən etmək üçün bir vasitə kimi istifadə olunur . Bu, Windows, Linux və macOS platformalarında mövcud olan C, C++, C# və Java dilləri üçün statik kod analizatorudur.
Məqalədə yalnız mənə maraqlı görünən səhvlər təqdim olunur. Lakin layihələr kiçik olduğu üçün səhvlər az oldu :).
Qeyd. FreeRDP layihəsinin yoxlanılması haqqında əvvəlki məqaləni tapa bilərsiniz .
masa üstü
— UNIX əsaslı sistemlər üçün RDP müştərisinin pulsuz tətbiqi. Layihəni Cygwin altında qurarsanız, Windows altında da istifadə edilə bilər. GPLv3 altında lisenziyalıdır.
Bu müştəri çox populyardır - o, standart olaraq ReactOS-da istifadə olunur və siz onun üçün üçüncü tərəf qrafik ön uclarını da tapa bilərsiniz. Bununla belə, o, kifayət qədər qocalıb: onun ilk buraxılışı 4 aprel 2001-ci ildə baş verib - yazı yazarkən onun 17 yaşı var.
Əvvəldə qeyd etdiyim kimi, layihə kifayət qədər kiçikdir. Təxminən 30 min sətir kod ehtiva edir ki, bu da yaşına görə bir qədər qəribədir. Müqayisə üçün qeyd edək ki, FreeRDP 320 min sətirdən ibarətdir. Budur Cloc proqramının çıxışı:
Əlçatmaz kod
Əlçatmaz kod aşkarlandı. Səhv 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);
}Səhv funksiyada dərhal bizimlə qarşılaşır Elanlar : operatordan sonra gələn kodu görürük qayıtmaq — bu fraqment yaddaşın təmizlənməsini həyata keçirir. Bununla belə, səhv təhlükə yaratmır: proqramdan çıxdıqdan sonra bütün ayrılmış yaddaş əməliyyat sistemi tərəfindən təmizlənəcək.
Səhv idarəsi yoxdur
Array underrun mümkündür. 'n' indeksinin dəyəri -1-ə çata bilər. 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 halda kod parçası fayl bitənə qədər fayldan buferə oxunur. Bununla belə, burada heç bir səhv idarəsi yoxdur: bir şey səhv olarsa, o zaman oxumaq -1 qaytaracaq və sonra massiv aşılacaq çıxış.
Char tipində EOF-dan istifadə
EOF "char" tipli dəyərlə müqayisə edilməməlidir. '(c = fgetc(fp))' 'int' tipli 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 faylın sonuna çatmağın səhv idarə edilməsini görürük: if fgetc kodu 0xFF olan simvolu qaytarır, o, faylın sonu kimi şərh olunacaq (EOF).
EOF sabitdir, adətən -1 kimi müəyyən edilir. Məsələn, CP1251 kodlaşdırmasında rus əlifbasının son hərfində 0xFF kodu var, bu, kimi bir dəyişəndən danışırıqsa, -1 rəqəminə uyğundur. chariot. Belə çıxır ki, 0xFF simvolu kimidir EOF (-1) faylın sonu kimi şərh olunur. Belə xətaların qarşısını almaq üçün funksiyanın nəticəsi belədir fgetc kimi dəyişəndə saxlanmalıdır int.
Səhvlər
1-ci fraqment
'yazma_zamanı' ifadəsi həmişə yanlışdı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; // <=
....
}Ola bilsin ki, bu kodun müəllifi səhv başa düşüb || и && vəziyyətdə. Dəyərlər üçün mümkün variantları nəzərdən keçirək yazma_zamanı и dəyişdirmə_zamanı:
- Hər iki dəyişən 0-a bərabərdir: bu halda biz budaqda sona çatacağıq daha: dəyişən mod_zaman sonrakı vəziyyətdən asılı olmayaraq həmişə 0 olacaqdır.
- Dəyişənlərdən biri 0-dır: mod_zaman 0-a bərabər olacaq (digər dəyişənin mənfi olmayan qiymətə malik olması şərti ilə), çünki MIN iki variantdan kiçik olanı seçəcək.
- Hər iki dəyişən 0-a bərabər deyil: minimum dəyəri seçin.
Şərti əvəz edərkən yazma_zamanı && vaxtını dəyişdirin davranış düzgün görünəcək:
- Bir və ya hər iki dəyişən 0-a bərabər deyil: sıfırdan fərqli dəyər seçin.
- Hər iki dəyişən 0-a bərabər deyil: minimum dəyəri seçin.
2-ci fraqment
İfadə həmişə doğrudur. Yəqin ki, burada '&&' operatorundan istifadə edilməlidir. 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ür, burada da operatorlar qarışıb || и &&Və ya == и !=: Dəyişən eyni vaxtda 20 və 9 qiymətinə malik ola bilməz.
Limitsiz xətt kopyalama
'sprintf' funksiyasının çağırışı buferin 'fullpath' daşmasına səbəb olacaq. 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);
....
}Funksiyaya tam baxdığınız zaman bu kodun problem yaratmadığı aydın olacaq. Bununla belə, onlar gələcəkdə yarana bilər: bir ehtiyatsız dəyişiklik və biz bufer daşması alacağıq - sprint heç nə ilə məhdudlaşmır, ona görə də yolları birləşdirərkən massivin hüdudlarından kənara çıxa bilərik. Bu çağırışa diqqət yetirmək tövsiyə olunur snprintf(tam yol, PATH_MAX, ….).
Ehtiyatsız vəziyyət
Şərti ifadənin bir hissəsi həmişə doğrudur: əlavə edin > 0. scard.c 507
static void
inRepos(STREAM in, unsigned int read)
{
SERVER_DWORD add = 4 - read % 4;
if (add < 4 && add > 0)
{
....
}
}Проверка əlavə edin > 0 burada ehtiyac yoxdur: dəyişən həmişə sıfırdan böyük olacaq, çünki oxumaq % 4 bölmənin qalan hissəsini qaytaracaq, lakin heç vaxt 4-ə bərabər olmayacaq.
xrdp
— açıq mənbə kodu ilə RDP serverinin həyata keçirilməsi. Layihə 2 hissəyə bölünür:
- xrdp - protokolun icrası. Apache 2.0 lisenziyası altında paylanmışdır.
- xorgxrdp - xrdp ilə istifadə üçün Xorg sürücüləri dəsti. Lisenziya - X11 (MIT kimi, lakin reklamda istifadəni qadağan edir)
Layihənin inkişafı rdesktop və FreeRDP nəticələrinə əsaslanır. Əvvəlcə qrafika ilə işləmək üçün ayrıca VNC serverindən və ya RDP dəstəyi ilə xüsusi X11 serverindən - X11rdp-dən istifadə etməli idiniz, lakin xorgxrdp-nin meydana çıxması ilə onlara ehtiyac yox oldu.
Bu yazıda xorgxrdp-ni əhatə etməyəcəyik.
Xrdp layihəsi, əvvəlki kimi, çox kiçikdir və təxminən 80 min sətirdən ibarətdir.
Daha çox yazı xətası
Kod oxşar blokların kolleksiyasını ehtiva edir. 87, 88, 89-cu sətirlərdə 'r', 'g', 'r' maddələrini yoxlayın. 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 üçün jpeg2000 kodekini tətbiq edən librfxcodec kitabxanasından götürülmüşdür. Burada, görünür, qrafik məlumat kanalları qarışdırılır - "mavi" rəng əvəzinə "qırmızı" qeyd olunur. Bu xəta çox güman ki, kopyala-yapışdırmaq nəticəsində yaranıb.
Eyni problem oxşar funksiyada baş verdi rfx_encode_format_argb, bunu analizator da bizə dedi:
Kod oxşar blokların kolleksiyasını ehtiva edir. 260, 261, 262, 263-cü sətirlərdə 'a', 'r', 'g', 'r' elementlərini yoxlayın. rfxencode_rgb_to_yuv.c 260
while (x < 64)
{
*la_buf++ = a;
*lr_buf++ = r;
*lg_buf++ = g;
*lb_buf++ = r;
x++;
}Massiv bəyannaməsi
Massiv aşmaq mümkündür. 'i — 8' indeksinin dəyəri 129-a çata bilər. 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 faylda massivin bəyanı və tərifi uyğun gəlmir - ölçü 1 ilə fərqlənir. Bununla belə, heç bir səhv baş vermir - evdev-map.c faylında düzgün ölçü göstərilib, ona görə də sərhəddən kənar yoxdur. Beləliklə, bu asanlıqla düzəldilə bilən bir səhvdir.
Yanlış müqayisə
Şərti ifadənin bir hissəsi həmişə yanlışdı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))
{
....
}
....
}Funksiya tip dəyişənini oxuyur imzasız qısa kimi dəyişənə çevrilir int. Burada yoxlamaya ehtiyac yoxdur, çünki biz işarəsiz dəyişəni oxuyuruq və nəticəni daha böyük dəyişənə təyin edirik, ona görə də dəyişən mənfi qiymət ala bilməz.
Lazımsız yoxlamalar
Şərti ifadənin bir hissəsi həmişə 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;
}
....
}Bərabərsizliyin yoxlanılmasının burada mənası yoxdur, çünki bizdə əvvəldə bir müqayisə var. Çox güman ki, bu, hərf səhvidir və tərtibatçı operatordan istifadə etmək istəyib || etibarsız arqumentləri süzgəcdən keçirmək.
Nəticə
Audit zamanı ciddi səhvlər aşkar edilməsə də, bir çox nöqsanlar aşkar edilib. Bununla belə, bu dizaynlar əhatə dairəsi kiçik olsa da, bir çox sistemlərdə istifadə olunur. Kiçik bir layihədə çoxlu səhvlər olması mütləq deyil, ona görə də analizatorun işini yalnız kiçik layihələrdə mühakimə etməməlisiniz. Bu barədə daha çox məqalədə oxuya bilərsiniz "".
PVS-Studio-nun sınaq versiyasını bizdən yükləyə bilərsiniz .
Bu məqaləni ingilisdilli auditoriya ilə bölüşmək istəyirsinizsə, tərcümə linkindən istifadə edin: Sergey Larin.
Mənbə: www.habr.com