FreeRDP ni utekelezaji huria wa Itifaki ya Kompyuta ya Mbali (RDP), itifaki iliyotengenezwa na Microsoft kwa ajili ya udhibiti wa kompyuta kwa mbali. Mradi huu unaunga mkono mifumo mingi, ikiwa ni pamoja na Windows, Linux, macOS na hata iOS na AndroidMradi huu ulichaguliwa kama wa kwanza katika mfululizo wa makala zilizotolewa kwa ajili ya kujaribu wateja wa RDP kwa kutumia kichambuzi tuli cha PVS-Studio.
kidogo ya historia
Mradi ilikuja baada ya Microsoft kufungua vipimo vya itifaki yake ya umiliki ya RDP. Wakati huo, kulikuwa na mteja wa rdesktop, ambayo utekelezaji wake ulitokana na matokeo ya Uhandisi wa Reverse.
Itifaki ilipotekelezwa, ikawa vigumu zaidi kuongeza utendaji mpya kutokana na usanifu wa mradi uliokuwepo wakati huo. Mabadiliko ndani yake yalizua mzozo kati ya watengenezaji, ambayo ilisababisha kuundwa kwa uma ya rdesktop - FreeRDP. Usambazaji zaidi wa bidhaa ulidhibitiwa na leseni ya GPLv2, kwa sababu hiyo uamuzi ulifanywa wa kuirejesha kwa Apache License v2. Hata hivyo, si kila mtu alikubali kubadilisha leseni ya kanuni zao, kwa hiyo watengenezaji waliamua kuandika upya mradi huo, na kusababisha codebase ya kisasa.
Unaweza kusoma zaidi kuhusu historia ya mradi katika chapisho rasmi la blogi: "Historia ya mradi wa FreeRDP".
Inatumika kama zana ya kutambua makosa na udhaifu unaowezekana katika msimbo. Ni kichambuzi cha msimbo tuli cha C, C++, C# na Java, kinachopatikana kwenye mifumo Windows, Linux и macOS.
Makala hiyo inawasilisha tu makosa ambayo yalionekana kunivutia zaidi.
Kumbukumbu ya kumbukumbu
Chaguo za kukokotoa ziliondolewa bila kuachilia kielekezi cha 'cwd'. Uvujaji wa kumbukumbu unawezekana. mazingira.c 84
DWORD GetCurrentDirectoryA(DWORD nBufferLength, LPSTR lpBuffer)
{
char* cwd;
....
cwd = getcwd(NULL, 0);
....
if (lpBuffer == NULL)
{
free(cwd);
return 0;
}
if ((length + 1) > nBufferLength)
{
free(cwd);
return (DWORD) (length + 1);
}
memcpy(lpBuffer, cwd, length + 1);
return length;
....
}Kipande hiki kilichukuliwa kutoka kwa mfumo mdogo wa winpr, ambao hutekeleza kifungashio cha WINAPI kwa ajili yaWindows mifumo, yaani, ni analogi nyepesi ya Mvinyo. Hapa unaweza kuona uvujaji: kumbukumbu iliyotengwa na kitendakazi getcwd, hutolewa tu wakati wa kushughulikia kesi maalum. Ili kurekebisha hitilafu unahitaji kuongeza simu bure baada ya kumbukumbu.
Safu nje ya mipaka
Kupanda kwa safu kunawezekana. Thamani ya faharasa ya 'event->EventHandlerCount' inaweza kufikia 32. PubSub.c 117
#define MAX_EVENT_HANDLERS 32
struct _wEventType
{
....
int EventHandlerCount;
pEventHandler EventHandlers[MAX_EVENT_HANDLERS];
};
int PubSub_Subscribe(wPubSub* pubSub, const char* EventName,
pEventHandler EventHandler)
{
....
if (event->EventHandlerCount <= MAX_EVENT_HANDLERS)
{
event->EventHandlers[event->EventHandlerCount] = EventHandler;
event->EventHandlerCount++;
}
....
}Mfano huu unaongeza kipengele kipya kwenye orodha hata kama idadi ya vipengele imefikia upeo. Hapa ni ya kutosha kuchukua nafasi ya operator <= juu ya <, ili usiende zaidi ya mipaka ya safu.
Hitilafu nyingine ya aina hii ilipatikana:
- V557 Array overrun inawezekana. Thamani ya faharasa ya 'iBitmapFormat' inaweza kufikia 8. orders.c 2623
Typos
Sehemu ya 1
Usemi '!bomba->Ndani' ni uongo kila wakati. MessagePipe.c 63
wMessagePipe* MessagePipe_New()
{
....
pipe->In = MessageQueue_New(NULL);
if (!pipe->In)
goto error_in;
pipe->Out = MessageQueue_New(NULL);
if (!pipe->In) // <=
goto error_out;
....
}Hapa tunaona chapa ya kawaida: hali ya pili hukagua utofauti sawa na wa kwanza. Uwezekano mkubwa zaidi, hitilafu ilionekana kama matokeo ya kunakili msimbo bila kufaulu.
Sehemu ya 2
Vitalu viwili vya maandishi yanayofanana vilipatikana. Kizuizi cha pili huanza kutoka mstari wa 771. tsg.c 770
typedef struct _TSG_PACKET_VERSIONCAPS
{
....
UINT16 majorVersion;
UINT16 minorVersion;
....
} TSG_PACKET_VERSIONCAPS, *PTSG_PACKET_VERSIONCAPS;
static BOOL TsProxyCreateTunnelReadResponse(....)
{
....
PTSG_PACKET_VERSIONCAPS versionCaps = NULL;
....
/* MajorVersion (2 bytes) */
Stream_Read_UINT16(pdu->s, versionCaps->majorVersion);
/* MinorVersion (2 bytes) */
Stream_Read_UINT16(pdu->s, versionCaps->majorVersion);
....
}Chapa nyingine: maoni ya nambari yanamaanisha kuwa nyuzi inapaswa kuja NdogoVersion, hata hivyo, kusoma hutokea katika kutofautiana kwa jina toleo kuu. Walakini, sijui itifaki, kwa hivyo hii ni nadhani tu.
Sehemu ya 3
Ni ajabu kwamba kipengele cha chaguo za kukokotoa cha 'trio_index_last' ni sawa na kitendakazi cha 'trio_index'. triostr.c 933
/**
Find first occurrence of a character in a string.
....
*/
TRIO_PUBLIC_STRING char *
trio_index
TRIO_ARGS2((string, character),
TRIO_CONST char *string,
int character)
{
assert(string);
return strchr(string, character);
}
/**
Find last occurrence of a character in a string.
....
*/
TRIO_PUBLIC_STRING char *
trio_index_last
TRIO_ARGS2((string, character),
TRIO_CONST char *string,
int character)
{
assert(string);
return strchr(string, character);
}Kwa kuzingatia maoni, kazi faharisi_ya_tatu hupata herufi inayolingana ya kwanza kwenye mfuatano wakati trio_index_mwisho - jambo la mwisho. Lakini miili ya kazi hizi ni sawa! Uwezekano mkubwa zaidi hii ni typo, na katika kazi trio_index_mwisho haja ya kutumia strhrrchr badala ya strhr. Kisha tabia itatarajiwa.
Sehemu ya 4
Kielekezi cha 'data' katika usemi ni sawa na nullptr. Thamani inayotokana ya shughuli za hesabu kwenye pointer hii haina maana na haipaswi kutumiwa. nsc_encode.c 124
static BOOL nsc_encode_argb_to_aycocg(NSC_CONTEXT* context,
const BYTE* data,
UINT32 scanline)
{
....
if (!context || data || (scanline == 0))
return FALSE;
....
src = data + (context->height - 1 - y) * scanline;
....
}Inaonekana kama mwendeshaji wa kukanusha alikosekana hapa kimakosa ! Karibu data. Ni ajabu kwamba hii ilikwenda bila kutambuliwa.
Sehemu ya 5
Matumizi ya 'ikiwa (A) {...} vinginevyo ikiwa (A) {…}' mchoro ulitambuliwa. Kuna uwezekano wa kuwepo kwa makosa ya kimantiki. Angalia mistari: 213, 222. rdpei_common.c 213
BOOL rdpei_write_4byte_unsigned(wStream* s, UINT32 value)
{
BYTE byte;
if (value <= 0x3F)
{
....
}
else if (value <= 0x3FFF)
{
....
}
else if (value <= 0x3FFFFF)
{
byte = (value >> 16) & 0x3F;
Stream_Write_UINT8(s, byte | 0x80);
byte = (value >> 8) & 0xFF;
Stream_Write_UINT8(s, byte);
byte = (value & 0xFF);
Stream_Write_UINT8(s, byte);
}
else if (value <= 0x3FFFFF)
{
byte = (value >> 24) & 0x3F;
Stream_Write_UINT8(s, byte | 0xC0);
byte = (value >> 16) & 0xFF;
Stream_Write_UINT8(s, byte);
byte = (value >> 8) & 0xFF;
Stream_Write_UINT8(s, byte);
byte = (value & 0xFF);
Stream_Write_UINT8(s, byte);
}
....
}Masharti mawili ya mwisho ni sawa: inaonekana mtu alisahau kuziangalia baada ya kunakili. Inaonekana kutoka kwa nambari kwamba sehemu ya mwisho inafanya kazi na maadili ya baiti nne, kwa hivyo tunaweza kudhani kuwa hali ya mwisho inapaswa kuwa. thamani <= 0x3FFFFFFFF.
Hitilafu nyingine ya aina hii ilipatikana:
- V517 Matumizi ya muundo wa 'ikiwa (A) {…} vinginevyo ikiwa (A) {…}' yametambuliwa. Kuna uwezekano wa kuwepo kwa makosa ya kimantiki. Angalia mistari: 169, 173. file.c 169
Uthibitishaji wa data ya pembejeo
Sehemu ya 1
Usemi 'strcat(lengo, chanzo) != NULL' huwa kweli kila wakati. triostr.c 425
TRIO_PUBLIC_STRING int
trio_append
TRIO_ARGS2((target, source),
char *target,
TRIO_CONST char *source)
{
assert(target);
assert(source);
return (strcat(target, source) != NULL);
}Kuangalia matokeo ya chaguo za kukokotoa katika mfano huu si sahihi. Kazi strcat inarudisha pointer kwenye toleo la mwisho la kamba, i.e. parameta ya kwanza imepita. Katika kesi hii ni lengo. Walakini, ikiwa ni sawa NULL, basi ni kuchelewa sana kuiangalia, kwa kuwa katika kazi strcat itaahirishwa.
Sehemu ya 2
Maneno 'cache' ni kweli kila wakati. glyph.c 730
typedef struct rdp_glyph_cache rdpGlyphCache;
struct rdp_glyph_cache
{
....
GLYPH_CACHE glyphCache[10];
....
};
void glyph_cache_free(rdpGlyphCache* glyphCache)
{
....
GLYPH_CACHE* cache = glyphCache->glyphCache;
if (cache)
{
....
}
....
}Katika kesi hii, kutofautiana cache anwani ya safu tuli imepewa glyphCache->glyphCache. Kwa hivyo, angalia ikiwa (cache) inaweza kuachwa.
Hitilafu ya usimamizi wa rasilimali
Nyenzo hii ilipatikana kwa kutumia chaguo za kukokotoa za 'CreateFileA' lakini ilitolewa kwa kutumia chaguo la kukokotoa la 'fclose' lisilopatana. cheti.c 447
BOOL certificate_data_replace(rdpCertificateStore* certificate_store,
rdpCertificateData* certificate_data)
{
HANDLE fp;
....
fp = CreateFileA(certificate_store->file, GENERIC_READ | GENERIC_WRITE, 0,
NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
....
if (size < 1)
{
CloseHandle(fp);
return FALSE;
}
....
if (!data)
{
fclose(fp);
return FALSE;
}
....
}Kifafanuzi cha faili fp, iliyoundwa na simu ya kukokotoa UndaFaili imefungwa kwa makosa na chaguo la kukokotoa funga kutoka kwa maktaba ya kawaida, sio CloseHandle.
Masharti sawa
Semi za masharti za taarifa za 'ikiwa' ziko pamoja zinafanana. Angalia mistari: 269, 283. ndr_structure.c 283
void NdrComplexStructBufferSize(PMIDL_STUB_MESSAGE pStubMsg,
unsigned char* pMemory, PFORMAT_STRING pFormat)
{
....
if (conformant_array_description)
{
ULONG size;
unsigned char array_type;
array_type = conformant_array_description[0];
size = NdrComplexStructMemberSize(pStubMsg, pFormat);
WLog_ERR(TAG, "warning: NdrComplexStructBufferSize array_type: "
"0x%02X unimplemented", array_type);
NdrpComputeConformance(pStubMsg, pMemory + size,
conformant_array_description);
NdrpComputeVariance(pStubMsg, pMemory + size,
conformant_array_description);
MaxCount = pStubMsg->MaxCount;
ActualCount = pStubMsg->ActualCount;
Offset = pStubMsg->Offset;
}
if (conformant_array_description)
{
unsigned char array_type;
array_type = conformant_array_description[0];
pStubMsg->MaxCount = MaxCount;
pStubMsg->ActualCount = ActualCount;
pStubMsg->Offset = Offset;
WLog_ERR(TAG, "warning: NdrComplexStructBufferSize array_type: "
"0x%02X unimplemented", array_type);
}
....
}Mfano huu unaweza usiwe mdudu. Hata hivyo, hali zote mbili zina ujumbe sawa, moja ambayo inaweza uwezekano mkubwa kuondolewa.
Kusafisha viashiria visivyofaa
Kielekezi kisicho na maana kinapitishwa kwenye kitendakazi cha 'bure'. Kagua hoja ya kwanza. smartcard_pcsc.c 875
WINSCARDAPI LONG WINAPI PCSC_SCardListReadersW(
SCARDCONTEXT hContext,
LPCWSTR mszGroups,
LPWSTR mszReaders,
LPDWORD pcchReaders)
{
LPSTR mszGroupsA = NULL;
....
mszGroups = NULL; /* mszGroups is not supported by pcsc-lite */
if (mszGroups)
ConvertFromUnicode(CP_UTF8,0, mszGroups, -1,
(char**) &mszGroupsA, 0,
NULL, NULL);
status = PCSC_SCardListReaders_Internal(hContext, mszGroupsA,
(LPSTR) &mszReadersA,
pcchReaders);
if (status == SCARD_S_SUCCESS)
{
....
}
free(mszGroupsA);
....
}Kufanya kazi bure unaweza kupitisha null pointer na analyzer anajua kuihusu. Lakini ikiwa hali itagunduliwa ambayo pointer inapitishwa kila wakati, kama ilivyo kwenye snippet hii, onyo litatolewa.
Pointer mszVikundiA mwanzoni sawa NULL na haijaanzishwa popote pengine. Tawi pekee la msimbo ambapo pointer inaweza kuanzishwa haliwezi kufikiwa.
Kulikuwa na ujumbe mwingine kama vile:
- V575 Kielekezi batili kinapitishwa kwenye kitendakazi cha 'bure'. Kagua hoja ya kwanza. leseni.c 790
- V575 Kielekezi batili kinapitishwa kwenye kitendakazi cha 'bure'. Kagua hoja ya kwanza. rdpsnd_alsa.c 575
Uwezekano mkubwa zaidi, vigezo vile vilivyosahaulika hutokea wakati wa mchakato wa kurejesha upya na vinaweza kuondolewa tu.
Uwezekano wa kufurika
Uwezekano wa kufurika. Fikiria uchezaji wa kuigiza, sio matokeo. makecert.c 1087
// openssl/x509.h
ASN1_TIME *X509_gmtime_adj(ASN1_TIME *s, long adj);
struct _MAKECERT_CONTEXT
{
....
int duration_years;
int duration_months;
};
typedef struct _MAKECERT_CONTEXT MAKECERT_CONTEXT;
int makecert_context_process(MAKECERT_CONTEXT* context, ....)
{
....
if (context->duration_months)
X509_gmtime_adj(after, (long)(60 * 60 * 24 * 31 *
context->duration_months));
else if (context->duration_years)
X509_gmtime_adj(after, (long)(60 * 60 * 24 * 365 *
context->duration_years));
....
}Kuleta matokeo kwa muda mrefu si ulinzi wa kufurika, kwani hesabu yenyewe hutokea kwa kutumia aina int.
Kuachwa kwa pointer katika uanzishaji
Kielekezi cha 'muktadha' kilitumika kabla ya kuthibitishwa dhidi ya nullptr. Kagua mistari: 746, 748. gfx.c 746
static UINT gdi_SurfaceCommand(RdpgfxClientContext* context,
const RDPGFX_SURFACE_COMMAND* cmd)
{
....
rdpGdi* gdi = (rdpGdi*) context->custom;
if (!context || !cmd)
return ERROR_INVALID_PARAMETER;
....
}Hapa kuna pointer muktadha inaahirishwa wakati wa uanzishaji - kabla ya kuangaliwa.
Makosa mengine ya aina hii yalipatikana:
- V595 Kielekezi cha 'ntlm' kilitumika kabla ya kuthibitishwa dhidi ya nullptr. Angalia mistari: 236, 255. ntlm.c 236
- V595 Kielekezi cha 'muktadha' kilitumika kabla ya kuthibitishwa dhidi ya nullptr. Angalia mistari: 1003, 1007. rfx.c 1003
- V595 Kielekezi cha 'rdpei' kilitumika kabla ya kuthibitishwa dhidi ya nullptr. Angalia mistari: 176, 180. rdpei_main.c 176
- V595 Kielekezi cha 'gdi' kilitumika kabla ya kuthibitishwa dhidi ya nullptr. Angalia mistari: 121, 123. xf_gfx.c 121
Hali isiyo na maana
Usemi 'rdp->state >= CONNECTION_STATE_ACTIVE' huwa kweli kila wakati. muunganisho.c 1489
int rdp_server_transition_to_state(rdpRdp* rdp, int state)
{
....
switch (state)
{
....
case CONNECTION_STATE_ACTIVE:
rdp->state = CONNECTION_STATE_ACTIVE; // <=
....
if (rdp->state >= CONNECTION_STATE_ACTIVE) // <=
{
IFCALLRET(client->Activate, client->activated, client);
if (!client->activated)
return -1;
}
....
}
....
}Ni rahisi kuona kuwa hali ya kwanza haina maana kwa sababu ya dhamana inayolingana iliyopewa mapema.
Uchanganuzi wa mfuatano usio sahihi
Umbizo lisilo sahihi. Fikiria kuangalia hoja halisi ya tatu ya chaguo la kukokotoa la 'sscanf'. Kielekezi kwa aina ya int ambayo haijasainiwa inatarajiwa. wakala.c 220
Sehemu ya usemi wa masharti ni kweli kila wakati: (rc>= 0). wakala.c 222
static BOOL check_no_proxy(....)
{
....
int sub;
int rc = sscanf(range, "%u", &sub);
if ((rc == 1) && (rc >= 0))
{
....
}
....
}Kichanganuzi hutoa maonyo 2 mara moja kwa kipande hiki. Kielezi %u inatarajia kutofautisha kwa aina int isiyotiwa saini, lakini kutofautiana ndogo ina aina int. Ifuatayo tunaona ukaguzi wa tuhuma: hali ya kulia haina maana, kwani mwanzoni kuna kulinganisha na moja. Sijui mwandishi wa kanuni hii alimaanisha nini, lakini kuna kitu kibaya hapa.
Ukaguzi wa nje ya agizo
Usemi 'status == 0x00090314' sio kweli kila wakati. ntlm.c 299
BOOL ntlm_authenticate(rdpNtlm* ntlm, BOOL* pbContinueNeeded)
{
....
if (status != SEC_E_OK)
{
....
return FALSE;
}
if (status == SEC_I_COMPLETE_NEEDED) // <=
status = SEC_E_OK;
else if (status == SEC_I_COMPLETE_AND_CONTINUE) // <=
status = SEC_I_CONTINUE_NEEDED;
....
}Masharti yaliyoangaliwa yatakuwa ya uwongo kila wakati, kwani utekelezaji utafikia hali ya pili ikiwa hali == SEC_E_OK. Nambari sahihi inaweza kuonekana kama hii:
if (status == SEC_I_COMPLETE_NEEDED)
status = SEC_E_OK;
else if (status == SEC_I_COMPLETE_AND_CONTINUE)
status = SEC_I_CONTINUE_NEEDED;
else if (status != SEC_E_OK)
{
....
return FALSE;
}Hitimisho
Kwa hivyo, kuangalia mradi ulifunua shida nyingi, lakini ni sehemu ya kupendeza tu iliyoelezewa katika kifungu hicho. Wasanidi wa mradi wanaweza kuangalia mradi wenyewe kwa kuomba ufunguo wa leseni ya muda kwenye tovuti . Pia kulikuwa na chanya za uwongo, kazi ambayo itasaidia kuboresha analyzer. Hata hivyo, uchambuzi wa tuli ni muhimu ikiwa unataka sio tu kuboresha ubora wa msimbo wako, lakini pia kupunguza muda uliotumiwa kutafuta makosa, na PVS-Studio inaweza kusaidia kwa hili.
Ikiwa ungependa kushiriki makala hii na hadhira inayozungumza Kiingereza, tafadhali tumia kiungo cha kutafsiri: Sergey Larin.
Chanzo: mapenzi.com
