पीवीएस-स्टूडियो के साथ फ्रीआरडीपी की जाँच

FreeRDP Microsoft द्वारा स्वामित्व प्रोटोकॉल, दूरस्थ डेस्कटॉप प्रोटोकॉल (RDP) का एक ओपन-सोर्स कार्यान्वयन है। यह परियोजना विंडोज, लिनक्स, मैकओएस और यहां तक ​​कि आईओएस और एंड्रॉइड सहित कई प्लेटफार्मों का समर्थन करती है। हमने इसे आरडीपी-ग्राहकों के चेक के बारे में लेखों की एक श्रृंखला के लिए स्थिर कोड विश्लेषक पीवीएस-स्टूडियो के साथ विश्लेषण करने वाली पहली परियोजना के रूप में चुना।

कुछ इतिहास

Microsoft द्वारा अपने स्वामित्व प्रोटोकॉल RDP के विनिर्देशों को खोलने के बाद FreeRDP परियोजना शुरू की गई थी। उस समय, rdesktop नामक एक ग्राहक पहले से ही उपयोग में था, जो ज्यादातर रिवर्स इंजीनियरिंग के काम पर आधारित था।

जैसा कि वे प्रोटोकॉल को लागू कर रहे थे, डेवलपर्स ने वास्तुशिल्प मुद्दों के कारण नई कार्यक्षमता को जोड़ना मुश्किल पाया। आर्किटेक्चर में बदलाव ने डेवलपर्स के बीच संघर्ष को जन्म दिया और फ्री होल्ड के रूप में जाने जाने वाले रेकस्कोप का एक कांटा बनाने का नेतृत्व किया। आगे वितरण GPLv2 लाइसेंस द्वारा सीमित था, और लेखकों ने Apache लाइसेंस v2 के लिए अवशेष का फैसला किया। हालांकि, कुछ लोग लाइसेंस को बदलने के लिए तैयार नहीं थे, इसलिए डेवलपर्स ने कोड बेस को स्क्रैच से फिर से लिखने का फैसला किया और यही कि जैसा कि हम जानते हैं कि यह प्रोजेक्ट आज अस्तित्व में आया है।

परियोजना का पूरा इतिहास आधिकारिक ब्लॉग पर उपलब्ध है: " फ्रीआरडीपी परियोजना का इतिहास "।

मैंने बग्स और संभावित कमजोरियों के लिए प्रोजेक्ट को स्कैन करने के लिए पीवीएस-स्टूडियो का उपयोग किया। पीवीएस-स्टूडियो सी, सी ++, सी #, और जावा में लिखे गए कोड के लिए एक स्थिर विश्लेषक है और विंडोज, लिनक्स, और मैकओएस पर चलता है।

ध्यान दें कि मैं केवल उन बगों पर चर्चा करूंगा जो मुझे सबसे दिलचस्प लगे।

स्मृति रिसाव

V773 'cwd' पॉइंटर जारी किए बिना फ़ंक्शन को बाहर किया गया था। एक स्मृति रिसाव संभव है। पर्यावरण। 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; .... } 

यह स्निपेट विजेता सबसिस्टम से आता है, जो गैर-विंडोज सिस्टम के लिए एक WinAPI आवरण को लागू करता है, अर्थात वाइन के एक हल्के समकक्ष के रूप में कार्य करता है। ऊपर दिए गए कोड में मेमोरी रिसाव होता है: गेटकवर्ड फ़ंक्शन द्वारा आवंटित मेमोरी केवल विशेष-केस शाखाओं में जारी की जाती है। इसे ठीक करने के लिए, लेखकों को कॉल के बाद कॉल को मुफ्त में जोड़ना चाहिए।

सीमा सूचकांक को सीमा से बाहर

V557 ऐरे ओवररन संभव है। 'इवेंट-> EventHandlerCount' इंडेक्स का मान 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++; } .... } 

इस उदाहरण में, एक नया तत्व सूची में तब भी जोड़ा जाएगा जब उत्तरार्द्ध पहले से ही अधिकतम तत्वों तक पहुंच गया हो। यह बग केवल <= ऑपरेटर को <के साथ प्रतिस्थापित करके तय किया जा सकता है।

विश्लेषक ने इस प्रकार का एक और बग पाया:

• V557 ऐरे ओवररन संभव है। 'IBitmapFormat' सूचकांक का मान 8. आदेशों 2623 तक पहुँच सकता है

लेखन त्रुटियां

स्निपेट 1

V547 अभिव्यक्ति 'पाइप!> में' हमेशा गलत है। 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; .... 

}

हम यहां जो देख रहे हैं वह एक साधारण टाइपो है: पहली और दूसरी स्थिति दोनों एक ही चर की जांच करते हैं। यह खराब कॉपी-पेस्ट के उत्पाद की तरह दिखता है।

स्निपेट 2

V760 पाठ के दो समान खंड पाए गए। दूसरा ब्लॉक लाइन 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); .... } 

एक अन्य टाइपो: टिप्पणी कहती है कि हमें स्ट्रीम से पढ़ने के लिए मामूली वेरिएबल वेरिएबल की उम्मीद करनी चाहिए, जबकि वैल्यू को वेरिएबल वेरिएशन में पढ़ा जाता है। मैं परियोजना के साथ अच्छी तरह से परिचित नहीं हूँ, हालांकि यह सुनिश्चित करने के लिए पर्याप्त है।

स्निपेट 3

V524 यह अजीब है कि 'trio_index_last' फ़ंक्शन का शरीर '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); } 

जैसा कि टिप्पणी से पता चलता है, trio_index फ़ंक्शन स्ट्रिंग में पहला वर्ण घटना पाता है, जबकि trio_index_last फ़ंक्शन अंतिम घटना पाता है। फिर भी इन दोनों कार्यों के शरीर बिल्कुल समान हैं! यह एक टाइपो होना चाहिए और trio_index_last फ़ंक्शन को संभवतः स्ट्रैच के बजाय स्ट्र्रच को वापस करना चाहिए - उस स्थिति में, प्रोग्राम अपेक्षा के अनुरूप व्यवहार करेगा।

स्निपेट 4

V769 अभिव्यक्ति में 'डेटा' सूचक nullptr के बराबर है। इस सूचक पर अंकगणितीय संक्रियाओं का परिणामी मूल्य संवेदनहीन है और इसका उपयोग नहीं किया जाना चाहिए। 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; .... } 

डेवलपर ने गलती से निगेटिव ऑपरेटर को छोड़ दिया होगा ! डेटा से पहले। मुझे आश्चर्य है कि पहले किसी ने इस पर ध्यान क्यों नहीं दिया।

स्निपेट 5

V517 'if (A) {...} का उपयोग if (A) {...}' पैटर्न का पता चला। तार्किक त्रुटि उपस्थिति की संभावना है। चेक लाइन: 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); } .... } 

अंतिम दो शर्तें समान हैं: प्रोग्रामर को कॉपी बदलना भूल गया होगा। कोड के तर्क को देखते हुए, अंतिम भाग चार-बाइट मानों को संभालता है, इसलिए हम मान सकते हैं कि अंतिम स्थिति को जांचना चाहिए कि क्या मूल्य <= 0x3FFFFFFF है ।

इस प्रकार का एक और बग:

• V517 'if (A) {...} का उपयोग if (A) {...}' पैटर्न का पता चला। तार्किक त्रुटि उपस्थिति की संभावना है। चेक लाइनें: 169, 173. file.c 169

इनपुट डेटा की जाँच करना

स्निपेट 1

V547 अभिव्यक्ति 'स्ट्रैट (लक्ष्य, स्रोत)! = NULL' हमेशा सत्य होता है। 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); } 

फ़ंक्शन की वापसी मान की जांच दोषपूर्ण है। स्ट्रैकट फ़ंक्शन लक्ष्य स्ट्रिंग के लिए एक संकेतक लौटाता है, अर्थात् पहला पैरामीटर, जो इस मामले में लक्ष्य है । लेकिन अगर यह NULL के बराबर होता है, तो इसे जांचने में बहुत देर हो चुकी है क्योंकि यह पहले ही स्ट्रैकट फंक्शन में डिरेल हो चुका होगा।

स्निपेट 2

V547 एक्सप्रेशन 'कैश' हमेशा सच होता है। 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) { .... } .... } 

इस स्निपेट में, कैश वेरिएबल को स्टैटिक ऐरे ग्लाइफेचे-> ग्लिफ़कैच का पता सौंपा जाता है । चेक (यदि) कैश कर सकते हैं, इसलिए, हटाया जा सकता है।

संसाधन प्रबंधन त्रुटि

V1005 संसाधन 'CreateFileA' फ़ंक्शन का उपयोग करके हासिल किया गया था लेकिन असंगत 'fclose' फ़ंक्शन का उपयोग करके जारी किया गया था। प्रमाण पत्र। ४४ 44

 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; } .... } 

CreateFile फ़ंक्शन द्वारा बनाई गई फ़ाइल का fp हैंडल फ़ंक्शन बंदहैंडल के बजाय मानक लायब्रेरी से फ़ेकोज़ फ़ंक्शन को कॉल करके गलती से बंद हो गया था।

पहचान की स्थिति

V581 एक-दूसरे के साथ स्थित 'if' स्टेटमेंट्स के सशर्त भाव समान हैं। चेक लाइनें: 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); } .... } 

यह स्निपेट सही हो सकता है, लेकिन यह संदेह है कि दोनों स्थितियों में समान संदेश शामिल हैं - उनमें से एक संभवतः अनावश्यक है।

नि: शुल्क अशक्त संकेत

V575 null पॉइंटर को 'फ्री' फंक्शन में पास किया गया है। पहले तर्क का निरीक्षण करें। 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); .... } 

नि : शुल्क फ़ंक्शन को एक शून्य सूचक पर बुलाया जा सकता है, और पीवीएस-स्टूडियो जानता है कि। लेकिन अगर पॉइंटर हमेशा शून्य पाया जाता है, जैसे कि इस स्निपेट में, विश्लेषक एक चेतावनी जारी करेगा।

MszGroupsA पॉइंटर को शुरू में NULL पर सेट किया गया है और इसे कहीं और इनिशियलाइज़ नहीं किया गया है। एकमात्र शाखा जहां इसे आरंभ किया जा सकता था, वह उपलब्ध नहीं है।

इस प्रकार की अन्य चेतावनियों की एक जोड़ी:

• V575 null पॉइंटर को 'फ्री' फंक्शन में पास किया गया है। पहले तर्क का निरीक्षण करें। लाइसेंस। 790
• V575 null पॉइंटर को 'फ्री' फंक्शन में पास किया गया है। पहले तर्क का निरीक्षण करें। rdpsnd_alsa.c 575

परित्यक्त चर जैसे कि रिफैक्टिंग के बाद छोड़े गए अवशेष प्रतीत होते हैं और उन्हें हटाया जा सकता है।

संभावित अतिप्रवाह

V1028 संभावित अतिप्रवाह। कास्टिंग ऑपरेंड पर विचार करें, परिणाम नहीं। 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)); .... } 

मूल्यांकन को लंबे समय तक करने से परिणाम अतिप्रवाह नहीं होगा क्योंकि मूल्यांकन अभी भी किया जा रहा है, जबकि यह अभी भी टाइप इंट का है ।

आरंभीकरण पर सूचक को निष्क्रिय करना

V595 nullptr के खिलाफ सत्यापित होने से पहले 'संदर्भ' पॉइंटर का उपयोग किया गया था। चेक लाइनें: 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; .... } 

संदर्भ सूचक को इसके प्रारंभ के दौरान, चेक से पहले, अर्थात

इस प्रकार के अन्य कीड़े:

• V595 'ntlm' पॉइंटर को nullptr के खिलाफ सत्यापित होने से पहले उपयोग किया गया था। चेक लाइनें: 236, 255. ntlm.c 236
• V595 nullptr के खिलाफ सत्यापित होने से पहले 'संदर्भ' पॉइंटर का उपयोग किया गया था। चेक लाइनें: 1003, 1007. rfx.c 1003
• V595 ndptr के खिलाफ सत्यापित होने से पहले 'rdpei' पॉइंटर का उपयोग किया गया था। चेक लाइनें: 176, 180. rdpei_main.c 176
• V595 nullptr के खिलाफ सत्यापित होने से पहले 'gdi' पॉइंटर का उपयोग किया गया था। चेक लाइनें: 121, 123. xf_gfx.c 121

अर्थहीन स्थिति

V547 अभिव्यक्ति 'rdp-> राज्य> = CONNECTION_STATE_ACTIVE' हमेशा सत्य होता है। कनेक्शन। १४.c ९

 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; } .... } .... } 

यह देखना आसान है कि पहली शर्त का कोई मतलब नहीं है क्योंकि प्रश्न में मूल्य पहले से ही सौंपा गया था।

गलत स्ट्रिंग हैंडलिंग

V576 गलत प्रारूप। 'Sscanf' फ़ंक्शन के तीसरे वास्तविक तर्क की जाँच करने पर विचार करें। अहस्ताक्षरित int प्रकार के लिए एक सूचक अपेक्षित है। छद्म 220

V560 सशर्त अभिव्यक्ति का एक हिस्सा हमेशा सच होता है: (आरसी> = 0)। छद्म 222

 static BOOL check_no_proxy(....) { .... int sub; int rc = sscanf(range, "%u", &sub); if ((rc == 1) && (rc >= 0)) { .... } .... } 

यह कोड एक बार में दो चेतावनियों को ट्रिगर करता है। % U प्लेसहोल्डर का उपयोग प्रकार अहस्ताक्षरित int के चर के लिए किया जाता है, जबकि उप चर प्रकार int का होता है । दूसरी चेतावनी एक संदिग्ध जाँच को इंगित करती है: सशर्त अभिव्यक्ति का सही हिस्सा समझ में नहीं आता है क्योंकि पहले से ही बाएं हिस्से में 1 के लिए चर की जाँच की गई थी। मैं लेखक के इरादों के बारे में निश्चित नहीं हूं, लेकिन इस कोड के साथ कुछ स्पष्ट रूप से गलत है।

गलत क्रम में जाँच करता है

V547 अभिव्यक्ति 'स्थिति == 0x00090314' हमेशा झूठी होती है। 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; .... } 

चिह्नित स्थितियां हमेशा झूठी होंगी क्योंकि दूसरी स्थिति को केवल तभी निष्पादित किया जा सकता है यदि स्थिति == SEC_E_OK । यह वही है जो सही संस्करण देख सकता है:

 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; } 

निष्कर्ष

चेक में बहुत सारे बग पाए गए, और ऊपर जिन लोगों ने चर्चा की, वे सबसे दिलचस्प हैं। प्रोजेक्ट डेवलपर्स को अपना स्वयं का चेक करने के लिए पीवीएस-स्टूडियो वेबसाइट पर एक अस्थायी लाइसेंस कुंजी के लिए एक फॉर्म जमा करने का स्वागत है। विश्लेषक ने कई झूठी सकारात्मकताएं भी उत्पन्न कीं, जिन्हें हम इसके प्रदर्शन को सुधारने के लिए ठीक करेंगे। ध्यान दें कि स्थैतिक विश्लेषण अपरिहार्य है यदि आपका लक्ष्य न केवल कोड की गुणवत्ता में सुधार करना है, बल्कि बग शिकार को कम समय लेने वाला बनाना है - और यही वह जगह है जहां पीवीएस-स्टूडियो काम में आएगा।