Þetta er önnur umfjöllunin í röð greina um að prófa opinn hugbúnað til að vinna með RDP samskiptareglunum. Í því munum við skoða rdesktop biðlarann og xrdp þjóninn.
Notað sem tæki til að bera kennsl á villur . Það er kyrrstöðugreiningartæki fyrir C, C++, C# og Java tungumál, fáanlegt á Windows, Linux og macOS kerfum.
Greinin sýnir aðeins þær villur sem mér þóttu áhugaverðar. Hins vegar eru verkefnin lítil og því lítið um mistök :).
Athugið. Fyrri grein um sannprófun FreeRDP verkefna er að finna .
rskrifborð
— ókeypis útfærsla á RDP biðlara fyrir UNIX-undirstaða kerfi. Það er líka hægt að nota það undir Windows ef þú byggir verkefnið undir Cygwin. Leyfi samkvæmt GPLv3.
Þessi viðskiptavinur er mjög vinsæll - hann er notaður sjálfgefið í ReactOS og þú getur líka fundið grafíska framenda þriðja aðila fyrir hann. Hann er hins vegar nokkuð gamall: Fyrsta útgáfu hans fór fram 4. apríl 2001 - þegar þetta er skrifað er hann 17 ára.
Eins og ég tók fram áðan er verkefnið mjög lítið. Það inniheldur um það bil 30 þúsund línur af kóða, sem er svolítið skrítið miðað við aldur hans. Til samanburðar inniheldur FreeRDP 320 þúsund línur. Hér er úttakið af Cloc forritinu:
Kóði sem ekki er hægt að ná í
Ótiltækur kóði fannst. Hugsanlegt er að villa sé til staðar. 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);
}Villan rekst á okkur strax í fallinu helstu: við sjáum kóðann koma á eftir símafyrirtækinu aftur — þetta brot framkvæmir minnishreinsun. Hins vegar skapar villan ekki ógn: allt úthlutað minni verður hreinsað af stýrikerfinu eftir að forritið hættir.
Engin villumeðferð
Array underrun er möguleg. Gildi 'n' vísitölunnar gæti orðið -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] = ' '; // <=
str_handle_lines(output, &rest, linehandler, data);
}
....
}Kóðabúturinn í þessu tilfelli les úr skránni í biðminni þar til skránni lýkur. Hins vegar er engin villa meðhöndlun hér: ef eitthvað fer úrskeiðis, þá lesa mun skila -1, og þá verður fylkið yfirkeyrt framleiðsla.
Notkun EOF í bleikjugerð
EOF ætti ekki að bera saman við gildi af gerðinni „bleikju“. '(c = fgetc(fp))' ætti að vera af gerðinni '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++;
}
....
}Hér sjáum við ranga meðhöndlun á því að ná endalokum skráarinnar: if fgetc skilar staf sem hefur kóðann 0xFF, hann verður túlkaður sem endir skráarinnar (EOF).
EOF það er fasti, venjulega skilgreindur sem -1. Til dæmis, í CP1251 kóðuninni, hefur síðasti stafurinn í rússneska stafrófinu kóðann 0xFF, sem samsvarar tölunni -1 ef við erum að tala um breytu eins og bleikju. Það kemur í ljós að táknið 0xFF, eins og EOF (-1) er túlkað sem endir skráarinnar. Til að forðast slíkar villur er niðurstaða aðgerðarinnar fgetc ætti að geyma í breytu eins og int.
Villur
Brot 1
Tjáningin 'write_time' er alltaf ósönn. 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; // <=
....
}Kannski hefur höfundur þessa kóða rangt fyrir sér || и && í ástandi. Við skulum íhuga mögulega valkosti fyrir gildi skrifa_tími и breyta_tíma:
- Báðar breyturnar eru jafnar 0: í þessu tilfelli munum við enda í grein annars: breytilegt mod_tími verður alltaf 0 óháð síðari ástandi.
- Ein af breytunum er 0: mod_tími verður jafnt og 0 (að því gefnu að hin breytan hafi óneikvætt gildi), því MIN mun velja þann minnsta af tveimur valkostum.
- Báðar breyturnar eru ekki jafnar 0: veldu lágmarksgildið.
Þegar skipt er um ástand með skrifa_tími && breyta_tíma hegðunin mun líta rétt út:
- Önnur eða báðar breyturnar eru ekki jafnar 0: veldu gildi sem ekki er núll.
- Báðar breyturnar eru ekki jafnar 0: veldu lágmarksgildið.
Brot 2
Tjáning er alltaf sönn. Sennilega ætti að nota '&&' stjórnanda hé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;
....
}Svo virðist sem rekstraraðilar séu ruglaðir hér líka || и &&Eða == и !=: Breyta getur ekki haft gildið 20 og 9 á sama tíma.
Ótakmörkuð línuafritun
Símtal í 'sprintf' fallinu mun leiða til yfirflæðis á biðminni '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);
....
}Þegar þú skoðar aðgerðina í heild sinni verður ljóst að þessi kóði veldur ekki vandamálum. Hins vegar gætu þær komið upp í framtíðinni: ein kærulaus breyting og við munum fá biðminni yfirflæði - sprettf er ekki takmarkað af neinu, þannig að þegar slóðir eru sameinaðar getum við farið út fyrir mörk fylkisins. Mælt er með því að taka eftir þessu símtali á snprintf(fullpath, PATH_MAX, ….).
Óþarfi ástand
Hluti skilyrtrar tjáningar er alltaf satt: bæta við > 0. scard.c 507
static void
inRepos(STREAM in, unsigned int read)
{
SERVER_DWORD add = 4 - read % 4;
if (add < 4 && add > 0)
{
....
}
}Проверка bæta við > 0 það er engin þörf hér: breytan verður alltaf stærri en núll, vegna þess að lestu % 4 skilar því sem eftir er af deildinni, en það verður aldrei jafnt og 4.
xrdp
— innleiðing á RDP netþjóni með opnum kóða. Verkefnið skiptist í 2 hluta:
- xrdp - samskiptareglur útfærsla. Dreift undir Apache 2.0 leyfinu.
- xorgxrdp - Set af Xorg rekla til notkunar með xrdp. Leyfi - X11 (eins og MIT, en bannar notkun í auglýsingum)
Þróun verkefnisins byggir á niðurstöðum rdesktop og FreeRDP. Upphaflega, til að vinna með grafík, þurfti að nota sérstakan VNC netþjón, eða sérstakan X11 netþjón með RDP stuðningi - X11rdp, en með tilkomu xorgxrdp hvarf þörfin fyrir þá.
Í þessari grein munum við ekki fjalla um xorgxrdp.
xrdp verkefnið, eins og það fyrra, er mjög lítið og inniheldur um það bil 80 þúsund línur.
Fleiri innsláttarvillur
Kóðinn inniheldur safn svipaðra blokka. Athugaðu atriði 'r', 'g', 'r' í línum 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++;
}
....
}
....
}Þessi kóði var tekinn úr librfxcodec bókasafninu, sem útfærir jpeg2000 merkjamálið fyrir RemoteFX. Hér, greinilega, er grafískum gagnarásum blandað saman - í stað „bláa“ litsins er „rautt“ skráð. Þessi villa birtist líklega vegna copy-paste.
Sama vandamál kom upp í svipaðri aðgerð rfx_encode_format_argb, sem greiningartækið sagði okkur líka:
Kóðinn inniheldur safn svipaðra blokka. Athugaðu atriði 'a', 'r', 'g', 'r' í línum 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++;
}Fylkisyfirlýsing
Möguleg umframmagn. Gildi „i — 8“ vísitölunnar gæti orðið 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];
....
}
....
}Yfirlýsingin og skilgreiningin á fylkinu í þessum tveimur skrám eru ósamrýmanleg - stærðin munar um 1. Engar villur eiga sér þó stað - rétt stærð er tilgreind í evdev-map.c skránni, þannig að það er engin utan marka. Þannig að þetta er bara galli sem auðvelt er að laga.
Rangur samanburður
Hluti skilyrtrar tjáningar er alltaf ósatt: (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))
{
....
}
....
}Fallið les tegundarbreytu óundirritaður stuttur inn í breytu eins og int. Athugun er ekki þörf hér vegna þess að við erum að lesa ómerkta breytu og úthluta niðurstöðunni til stærri breytu, þannig að breytan getur ekki tekið neikvætt gildi.
Óþarfa athuganir
Hluti skilyrtrar tjáningar er alltaf sannur: (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;
}
....
}Ójafnréttiseftirlit er ekki skynsamlegt hér þar sem við höfum þegar samanburð í upphafi. Líklegt er að þetta sé innsláttarvilla og framkvæmdaraðili vildi nota rekstraraðilann || að sía út ógild rök.
Ályktun
Við úttektina komu engar alvarlegar villur í ljós en margir annmarkar komu í ljós. Hins vegar er þessi hönnun notuð í mörgum kerfum, þó að umfangi sé lítið. Lítið verkefni hefur ekki endilega margar villur, svo þú ættir ekki að dæma frammistöðu greiningartækisins eingöngu fyrir lítil verkefni. Þú getur lesið meira um þetta í greininni “".
Þú getur hlaðið niður prufuútgáfu af PVS-Studio frá okkur .
Ef þú vilt deila þessari grein með enskumælandi áhorfendum, vinsamlegast notaðu þýðingartengilinn: Sergey Larin.
Heimild: www.habr.com