PVS-Studio विश्लेषक के साथ FreeRDP की जाँच करना

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

थोड़ा सा इतिहास

फ्रीआरडीपी परियोजना माइक्रोसॉफ्ट द्वारा अपने मालिकाना आरडीपी प्रोटोकॉल के विनिर्देशों का अनावरण करने के बाद आई। उस समय, एक 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 आवरण को लागू करता है, अर्थात। यह वाइन का एक हल्का प्रतिरूप है। यहां एक रिसाव है: गेटकॉव फ़ंक्शन द्वारा आवंटित मेमोरी को केवल विशेष मामलों के दौरान मुक्त किया जाता है। त्रुटि को ठीक करने के लिए, memcpy के बाद मुफ्त में कॉल जोड़ें।

किसी व्यूह की सीमा से परे जाना

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 तक पहुँच सकता है

शुद्धिपत्र

टुकड़ा करना १

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

यहां हम सामान्य टाइपो देखते हैं: दूसरी स्थिति में, उसी चर को पहले की तरह जांचा जाता है। सबसे अधिक संभावना है, कोड की असफल प्रतिलिपि के परिणामस्वरूप त्रुटि दिखाई दी।

टुकड़ा करना २

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

एक अन्य टाइपो: कोड पर एक टिप्पणी का तात्पर्य है कि मामूली विचलन धारा से आना चाहिए, हालांकि, पठन को वेरिएशन नामक एक चर में जगह मिलती है । हालांकि, मैं प्रोटोकॉल से परिचित नहीं हूं, इसलिए यह सिर्फ एक धारणा है।

टुकड़े करना ३

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_last अंतिम है जब trio_index फ़ंक्शन एक स्ट्रिंग में एक चरित्र का पहला मैच पाता है। लेकिन इन कार्यों के शरीर समान हैं! सबसे अधिक संभावना है, यह एक टाइपो है, और trio_index_last फ़ंक्शन में आपको strrr के बजाय strrchr का उपयोग करने की आवश्यकता है । तब व्यवहार अपेक्षित होगा।

टुकड़े करना ४

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

लगता है कि वे गलती से नकार ऑपरेटर को याद किया ! डेटा के बगल में। यह अजीब है कि यह किसी का ध्यान नहीं गया।

टुकड़ा ५

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

इनपुट सत्यापन

टुकड़ा करना १

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 है , तो यह जांचने में बहुत देर हो चुकी है, क्योंकि strcat function में इसे dereferenced जाएगा।

टुकड़ा करना २

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

सबसे अधिक संभावना है, इस तरह के भूल गए चर refactoring के दौरान उत्पन्न होते हैं और बस हटाए जा सकते हैं।

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

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)) { .... } .... } 

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

अनियंत्रित चेक

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

निष्कर्ष

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

यदि आप इस लेख को अंग्रेजी बोलने वाले दर्शकों के साथ साझा करना चाहते हैं, तो कृपया अनुवाद के लिंक का उपयोग करें: सर्गेई लारिन। पीवीएस-स्टूडियो के साथ फ्रीआरडीपी की जाँच