Ez a második áttekintése a nyílt forráskódú programok teszteléséről szóló cikksorozatnak az RDP protokollal való együttműködéshez. Ebben megnézzük az rdesktop klienst és az xrdp szervert.
Eszközként használják a hibák azonosítására . Ez egy statikus kódelemző C, C++, C# és Java nyelvekhez, elérhető Windows, Linux és macOS platformokon.
A cikk csak azokat a hibákat mutatja be, amelyek számomra érdekesnek tűntek. A projektek azonban kicsik, így kevés volt a hiba :).
Megjegyzés. A FreeRDP projekt ellenőrzéséről szóló korábbi cikk megtalálható .
rdesktop
— egy RDP-kliens ingyenes megvalósítása UNIX-alapú rendszerekhez. Windows alatt is használható, ha Cygwin alatt építi fel a projektet. GPLv3 licenc alatt.
Ez a kliens nagyon népszerű - alapértelmezés szerint a ReactOS használja, és harmadik féltől származó grafikus felületeket is találhat hozzá. Ő azonban meglehetősen idős: első megjelenése 4. április 2001-én történt - a cikk írásakor 17 éves.
Mint korábban megjegyeztem, a projekt nagyon kicsi. Körülbelül 30 ezer sornyi kódot tartalmaz, ami a korához képest kissé furcsa. Összehasonlításképpen a FreeRDP 320 ezer sort tartalmaz. Íme a Cloc program kimenete:
Elérhetetlen kód
Nem elérhető kód észlelve. Lehetséges, hogy hiba van jelen. 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);
}A hiba azonnal megjelenik a függvényben fő-: látjuk, hogy az operátor után jön a kód visszatérés — ez a töredék memóriatisztítást végez. A hiba azonban nem jelent veszélyt: a program kilépése után az operációs rendszer az összes lefoglalt memóriát törli.
Nincs hibakezelés
A tömb aláfutása lehetséges. Az 'n' index értéke elérheti a -1-et. 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);
}
....
}A kódrészlet ebben az esetben beolvas a fájlból egy pufferbe, amíg a fájl véget nem ér. Itt azonban nincs hibakezelés: ha valami elromlik, akkor olvas -1 értékkel tér vissza, majd a tömb túlfut teljesítmény.
EOF használata char típusban
Az EOF-t nem szabad összehasonlítani a „char” típusú értékkel. A '(c = fgetc(fp))'-nek 'int' típusúnak kell lennie. 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++;
}
....
}Itt azt látjuk, hogy helytelenül kezelik a fájl végét: if fgetc olyan karaktert ad vissza, amelynek kódja 0xFF, ez a fájl végeként lesz értelmezve (EOF).
EOF ez egy állandó, általában -1-ként definiálható. Például a CP1251 kódolásban az orosz ábécé utolsó betűjének kódja 0xFF, ami a -1 számnak felel meg, ha olyan változóról beszélünk, mint pl. faszén. Kiderült, hogy a szimbólum 0xFF, mint EOF (-1) a fájl végeként értelmeződik. Az ilyen hibák elkerülése érdekében a függvény eredménye az fgetc hasonló változóban kell tárolni int.
Gépelési hibák
1. töredék
A 'write_time' kifejezés mindig hamis. lemez.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; // <=
....
}Lehet, hogy a kód szerzője tévedett || и && állapotban. Tekintsük az értékek lehetséges opcióit írási_idő и változás_idő:
- Mindkét változó egyenlő 0-val: ebben az esetben egy elágazásba kerülünk más: változó mod_time mindig 0 lesz, függetlenül a következő feltételtől.
- Az egyik változó 0: mod_time egyenlő lesz 0-val (feltéve, hogy a másik változó értéke nem negatív), mert MIN a két lehetőség közül a kisebbet választja.
- Mindkét változó nem egyenlő 0-val: válassza ki a minimális értéket.
A feltétel cseréjekor a írási_idő && változás_idő a viselkedés helyesnek tűnik:
- Az egyik vagy mindkét változó nem egyenlő 0-val: válasszon nullától eltérő értéket.
- Mindkét változó nem egyenlő 0-val: válassza ki a minimális értéket.
2. töredék
A kifejezés mindig igaz. Valószínűleg itt az '&&' operátort kell használni. lemez.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;
....
}Nyilván itt is összekeveredtek az operátorok || и &&Vagy == и !=: Egy változónak nem lehet egyszerre 20 és 9 értéke.
Korlátlan sormásolás
Az 'sprintf' függvény meghívása a puffer 'fullpath' túlcsordulásához vezet. lemez.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);
....
}Ha teljes egészében megnézi a függvényt, világossá válik, hogy ez a kód nem okoz problémát. A jövőben azonban felmerülhetnek: egy óvatlan változtatás, és puffertúlcsordulást kapunk - sprintel nem korlátozza semmi, így az utak összefűzésekor túlléphetünk a tömb határain. Javasoljuk, hogy észrevegye ezt a hívást snprintf(teljes elérési út, PATH_MAX, ….).
Redundáns állapot
A feltételes kifejezés egy része mindig igaz: add > 0. scard.c 507
static void
inRepos(STREAM in, unsigned int read)
{
SERVER_DWORD add = 4 - read % 4;
if (add < 4 && add > 0)
{
....
}
}Проверка add > 0 itt semmi szükség: a változó mindig nagyobb lesz nullánál, mert olvassa el a % 4-et visszaadja az osztás maradékát, de soha nem lesz egyenlő 4-gyel.
xrdp
— nyílt forráskódú RDP-kiszolgáló megvalósítása. A projekt 2 részre oszlik:
- xrdp - protokoll megvalósítás. Az Apache 2.0 licenc alatt terjesztve.
- xorgxrdp – Xorg illesztőprogramok készlete az xrdp-vel való használatra. Licenc – X11 (mint az MIT, de tiltja a reklámozásban való felhasználást)
A projekt fejlesztése az rdesktop és a FreeRDP eredményein alapul. Kezdetben a grafikával való munkához külön VNC szervert kellett használni, vagy egy speciális X11 szervert RDP támogatással - X11rdp, de az xorgxrdp megjelenésével megszűnt rájuk az igény.
Ebben a cikkben nem foglalkozunk az xorgxrdp-vel.
Az xrdp projekt az előzőhöz hasonlóan nagyon kicsi, és körülbelül 80 ezer sort tartalmaz.
Több elírás
A kód hasonló blokkok gyűjteményét tartalmazza. Jelölje be az „r”, „g”, „r” elemeket a 87., 88., 89. sorban. 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++;
}
....
}
....
}Ez a kód a librfxcodec könyvtárból származik, amely a jpeg2000 kodeket valósítja meg a RemoteFX számára. Itt láthatóan összekeverednek a grafikus adatcsatornák - a „kék” szín helyett „piros” kerül rögzítésre. Ez a hiba valószínűleg a másolás-beillesztés eredményeként jelentkezett.
Ugyanez a probléma hasonló funkciónál jelentkezett rfx_encode_format_argb, amit az elemző is elmondott nekünk:
A kód hasonló blokkok gyűjteményét tartalmazza. Jelölje be az „a”, „r”, „g”, „r” elemeket a 260., 261., 262., 263. sorban. rfxencode_rgb_to_yuv.c 260
while (x < 64)
{
*la_buf++ = a;
*lr_buf++ = r;
*lg_buf++ = g;
*lb_buf++ = r;
x++;
}Tömb deklaráció
Tömbtúllépés lehetséges. Az 'i — 8' index értéke elérheti a 129-et. 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];
....
}
....
}A tömb deklarációja és definíciója ebben a két fájlban nem kompatibilis - a méret 1-gyel különbözik. Hiba azonban nem fordul elő - a megfelelő méret az evdev-map.c fájlban van megadva, így nincs határon túli helyzet. Tehát ez csak egy könnyen javítható hiba.
Helytelen összehasonlítás
A feltételes kifejezés egy része mindig hamis: (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))
{
....
}
....
}A függvény beolvas egy típusváltozót aláíratlanul rövid változóba tetszik int. Itt nincs szükség ellenőrzésre, mert előjel nélküli változót olvasunk, és az eredményt egy nagyobb változóhoz rendeljük, így a változó nem vehet fel negatív értéket.
Felesleges ellenőrzések
A feltételes kifejezés egy része mindig igaz: (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;
}
....
}Az egyenlőtlenségi ellenőrzéseknek itt nincs értelme, mivel az elején már van egy összehasonlítás. Valószínűleg ez elírás, és a fejlesztő az operátort akarta használni || az érvénytelen argumentumok kiszűrésére.
Következtetés
Az ellenőrzés során súlyos hibákat nem tártak fel, de számos hiányosságot találtak. Ezeket a terveket azonban sok rendszerben használják, bár kicsi a terjedelem. Egy kis projekt nem feltétlenül tartalmaz sok hibát, ezért ne csak kis projekteken ítélje meg az analizátor teljesítményét. Erről bővebben a " cikkben olvashat”.
A PVS-Studio próbaverzióját letöltheti tőlünk .
Ha meg szeretné osztani ezt a cikket egy angolul beszélő közönséggel, használja a fordítási linket: Sergey Larin.
Forrás: will.com