بناءً على طلب المطورين المضمنين: اكتشاف الأخطاء في Amazon FreeRTOS

أي شخص يقوم ببرامج ميكروكنترولر ربما يعرف عن FreeRTOS ، أو على الأقل سمع عن نظام التشغيل هذا. قرر مطورو Amazon تعزيز قدرات نظام التشغيل هذا على العمل مع خدمات AWS Internet of Things. هذه هي الطريقة التي ظهرت أمازون FreeRTOS. طُلب منا ، مطورو محلل الشفرة الثابتة في PVS-Studio ، بالبريد وفي التعليقات للتحقق من هذه المشاريع. حسنا ، الآن الحصول على ما طلبت. استمر في القراءة لمعرفة ما خرج منه.

باختصار عن المشاريع

بادئ ذي بدء ، سوف أخبركم قليلاً عن رائد المشروع الجاري اختباره - FreeRTOS (الكود المصدري متاح هنا من خلال الرابط ). كما تنص ويكيبيديا ، فإن FreeRTOS هو نظام تشغيل متعدد المهام في الوقت الفعلي للأنظمة المدمجة.

هو مكتوب في C القديمة الجيدة ، وهو أمر غير مستغرب - يجب أن يعمل نظام التشغيل هذا في ظروف نموذجية من ميكروكنترولر: انخفاض قوة المعالجة ، وكمية صغيرة من ذاكرة الوصول العشوائي ، وما شابه ذلك. تسمح لك لغة C بالعمل مع الموارد بمستوى منخفض ولديها أداء عالٍ ، لذلك فهي الأنسب لتطوير نظام التشغيل هذا.

عاد الآن إلى منطقة Amazon ، التي تعمل دائمًا على تطوير اتجاهات واعدة مختلفة. على سبيل المثال ، تعمل Amazon على تطوير محرك Amazon Lumberyard AAA ، والذي فحصناه أيضًا .

أحد هذه الاتجاهات هو Internet of Things (IoT). للتطوير في هذا المجال ، قررت أمازون كتابة نظام التشغيل الخاص بهم - وأخذوا FreeRTOS الأساسية كأساس.

يتم وضع النظام الناتج ، Amazon FreeRTOS ، "لتوفير اتصال آمن لخدمات الويب من Amazon ، مثل AWS IoT Core أو AWS IoT Greengrass." الكود المصدري لهذا المشروع متاح على جيثب.

في هذه المقالة ، سنكتشف ما إذا كانت هناك أخطاء في FreeRTOS وكذلك مدى أمان نظام التشغيل Amazon من حيث تحليل الشفرة الثابتة.

مسار الشيك

تم إجراء الفحص باستخدام أداة البحث التلقائي عن الأخطاء - محلل الشفرة الثابتة في PVS-Studio. إنه قادر على اكتشاف الأخطاء في البرامج المكتوبة بلغات C و C ++ و C # و Java.

قبل التحليل ، يتعين علينا بناء المشروع. بهذه الطريقة ، سأكون واثقًا من أنني بحاجة إلى جميع التبعيات وأن المشروع جاهز للفحص. يمكن للمرء التحقق من المشروع بعدة طرق - على سبيل المثال ، باستخدام نظام مراقبة الترجمة. في هذه الحالة ، قمت بإجراء التحليل باستخدام المكون الإضافي لبرنامج Visual Studio - من الجيد أن تحتوي مستودعات كلا المشروعين على مجموعات من ملفات المشروع التي تسهل إنشاءها في نظام Windows.

كان عليّ فقط بناء مشاريع للتأكد من أن كل شيء جاهز للفحص. بعد ذلك ركضت التحليل و - فويلا! - لدي تقرير محلل جاهز أمامي.

يمكن أن تحتوي مكتبات الطرف الثالث المضمنة في هذه المشاريع أيضًا على أخطاء ، كما يمكنها بالطبع التأثير على البرنامج. ومع ذلك ، فقد استثنيتهم ​​من التحليل من أجل نقاء السرد.

لذلك ، يتم تحليل المشاريع ، تلقي التقارير ، وتسليط الضوء على أخطاء مثيرة للاهتمام. حان الوقت للحصول على استعراضهم!

ما يخفي FreeRTOS

في البداية ، كنت أتوقع أن أكتب مقالتين منفصلتين: واحدة لكل نظام تشغيل. كنت بالفعل فرك يدي؟ وأنا على استعداد لكتابة مقال جيد حول FreeRTOS. أتوقع اكتشاف ما لا يقل عن اثنين من الخلل العصير (مثل CWE-457 ) ، كنت أبحث من خلال تحذيرات متفرقة للمحلل ، و ... لم أجد شيئًا. لم أجد أي خطأ مثير للاهتمام.

العديد من التحذيرات الصادرة عن المحلل لم تكن ذات صلة بـ FreeRTOS. على سبيل المثال ، كانت هذه التحذيرات عبارة عن عيوب 64 بت مثل casting size_t إلى uint32_t . إنه مرتبط بحقيقة أن FreeRTOS يهدف إلى العمل على أجهزة بحجم مؤشر لا يزيد عن 32 بت.

لقد راجعت بدقة جميع التحذيرات V1027 التي تشير إلى الصب بين المؤشرات إلى الهياكل غير ذات الصلة. إذا كانت الهياكل المصبوبة لها نفس المحاذاة ، فإن مثل هذا الخطأ يكون خطأ. ولم أجد صبًا خطيرًا واحدًا!

تم ربط جميع الأماكن المشبوهة الأخرى بأسلوب الترميز أو تم تزويدها بتعليق يشرح سبب القيام بذلك بهذه الطريقة ولماذا لم يكن خطأً.

لذلك أود أن أناشد مطوري FreeRTOS. الرجال ، أنت رائع! بالكاد رأينا مشاريع نظيفة وعالية الجودة مثل مشاريعك. وكان من دواعي سروري قراءة الكود النظيف والأنيق والموثق جيدًا. القبعات قبالة يا رفاق.

على الرغم من أنني لم أتمكن من العثور على أي أخطاء مثيرة للاهتمام في ذلك اليوم ، كنت أعرف أنني لن أتوقف عند هذا الحد. كنت ذاهبًا إلى المنزل بثقة راسخة في أن نسخة أمازون ستحظى بنسبة 100٪ بشيء مثير للاهتمام ، وأنه غداً سألتقط بالتأكيد أخطاء كافية لهذه المقالة. كما كنت قد خمنت ، كنت على حق.

ما يخفي أمازون FreeRTOS

تحولت نسخة أمازون من النظام إلى ... بعبارة ملطفة ، أسوأ قليلاً. ظل تراث FreeRTOS نظيفًا في حين أخفيت التحسينات الجديدة الكثير من المشكلات المثيرة للاهتمام.

بعض الأجزاء كانت منطق البرنامج مكسورة ، بعض المؤشرات التي تم التعامل معها بشكل غير صحيح. في بعض الأماكن ، يمكن أن تؤدي الشفرة إلى سلوك غير محدد ، وكانت هناك حالات لم يعرف فيها المبرمج ببساطة عن نمط الخطأ الذي ارتكبه. لقد وجدت العديد من نقاط الضعف المحتملة الخطيرة.

يبدو أنني شددت مع مقدمة. لنبدأ في اكتشاف الأخطاء!

كسر منطق البرنامج

لنبدأ بمشاكل الأماكن التي تشير بوضوح إلى أن البرنامج لا يعمل بالطريقة التي توقعها المبرمج. التعامل مع مجموعة مشبوهة سيأتي أولاً:

/** * @brief Pool of request and associated response buffers, * handles, and configurations. */ static _requestPool_t _requestPool = { 0 }; .... static int _scheduleAsyncRequest(int reqIndex, uint32_t currentRange) { .... /* Set the user private data to use in the asynchronous callback context. */ _requestPool.pRequestDatas[reqIndex].pConnHandle = &_connHandle; _requestPool.pRequestDatas[reqIndex].pConnConfig = &_connConfig; _requestPool.pRequestDatas[reqIndex].reqNum = reqIndex; _requestPool.pRequestDatas[reqIndex].currRange = currentRange; _requestPool.pRequestDatas[reqIndex].currDownloaded = 0; _requestPool.pRequestDatas[reqIndex].numReqBytes = numReqBytes; .... _requestPool.pRequestDatas->scheduled = true; .... } 

أصدر PVS-Studio تحذيرين لهذا الجزء من الكود:

• V619 يتم استخدام الصفيف '_requestPool.pRequestDatas' كمؤشر لكائن مفرد. iot_demo_https_s3_download_async.c 973
• V574 يتم استخدام مؤشر '_requestPool.pRequestDatas' في نفس الوقت كصفيف وكمؤشر لكائن واحد. خطوط التحقق: 931 ، 973. iot_demo_https_s3_download_async.c 973

فقط في حالة اسمح لي أن أذكركم: اسم الصفيف هو المؤشر إلى العنصر الأول. هذا إذا كان _requestPool.pRequestDatas عبارة عن صفيف من الهياكل ، فإن _requestPool.pRequestDatas [i] .scheduled هو تقييم للعضو المجدول في بنية الصفيف i. وإذا كتبنا _requestPool.pRequestDatas-> مجدولة ، فسوف يتحول ذلك إلى سيتم الوصول إليهم من هيكل الصفيف الأول.

في مقتطف من الكود أعلاه ، هذا ما يحدث. في السطر الأخير ، يتم تغيير قيمة عضو بنية الصفيف الأول فقط. في حد ذاته ، يكون الوصول مشبوهًا بالفعل ، لكن الحالة هنا أكثر وضوحًا: يتم تقييم صفيف _requestPool.pRequestDatas بالفهرس في كامل نص الوظيفة. ولكن في النهاية تم نسيان عملية الفهرسة.

كما أفهمها ، يجب أن يبدو السطر الأخير كما يلي:

 _requestPool.pRequestDatas[reqIndex].scheduled = true; 

الخطأ التالي يكمن في وظيفة صغيرة ، لذلك سأعطيه بالكامل:

 /* Return true if the string " pcString" is found * inside the token pxTok in JSON file pcJson. */ static BaseType_t prvGGDJsoneq( const char * pcJson, const jsmntok_t * const pxTok, const char * pcString ) { uint32_t ulStringSize = ( uint32_t ) pxTok->end - ( uint32_t ) pxTok->start; BaseType_t xStatus = pdFALSE; if( pxTok->type == JSMN_STRING ) { if( ( uint32_t ) strlen( pcString ) == ulStringSize ) { if( ( int16_t ) strncmp( &pcJson[ pxTok->start ], // <= pcString, ulStringSize ) == 0 ) { xStatus = pdTRUE; } } } return xStatus; } 

تحذير PVS-Studio: V642 [CWE-197] يعد حفظ نتيجة دالة strncmp داخل متغير النوع "القصير" غير مناسب. يمكن أن تضيع البتات كبيرة كسر منطق البرنامج. aws_greengrass_discovery.c 637

دعونا نلقي نظرة على تعريف وظيفة strncmp:

 int strncmp( const char *lhs, const char *rhs, size_t count ); 

في المثال ، يتم تحويل نتيجة int int ، والتي يبلغ حجمها 32 بت في متغير من النوع int16_t . باستخدام هذا التحويل "الضيق" ، سيتم فقد وحدات البتات الأقدم من القيمة التي تم إرجاعها. على سبيل المثال ، إذا كانت دالة strncmp تُرجع 0x00010000 ، فستفقد الوحدة أثناء التحويل ، وسيتم تنفيذ الشرط.

من الغريب أن نرى مثل هذا الصب في الحالة. لماذا هو مطلوب من أي وقت مضى هنا ، إذا كان يمكن مقارنة كثافة العمليات العادية مع الصفر؟ من ناحية أخرى ، إذا أراد مبرمج أن تعود هذه الوظيفة إلى حقيقة في بعض الأحيان حتى لو لم يكن الأمر كذلك ، فلماذا لا يدعم هذا السلوك الصعب مع التعليق؟ ولكن بهذه الطريقة هو نوع من الباب الخلفي. على أي حال ، أنا أميل إلى الاعتقاد بأنه خطأ. ما رايك

سلوك غير محدد ومؤشرات

هنا يأتي مثال كبير. إنها تغطي dereference مؤشر المحتملة فارغة:

 static void _networkReceiveCallback(....) { IotHttpsReturnCode_t status = IOT_HTTPS_OK; _httpsResponse_t* pCurrentHttpsResponse = NULL; IotLink_t* pQItem = NULL; .... /* Get the response from the response queue. */ IotMutex_Lock(&(pHttpsConnection->connectionMutex)); pQItem = IotDeQueue_PeekHead(&(pHttpsConnection->respQ)); IotMutex_Unlock(&(pHttpsConnection->connectionMutex)); /* If the receive callback is invoked * and there is no response expected, * then this a violation of the HTTP/1.1 protocol. */ if (pQItem == NULL) { IotLogError(....); fatalDisconnect = true; status = IOT_HTTPS_NETWORK_ERROR; goto iotCleanup; } .... iotCleanup : /* Report errors back to the application. */ if (status != IOT_HTTPS_OK) { if ( pCurrentHttpsResponse->isAsync && pCurrentHttpsResponse->pCallbacks->errorCallback) { pCurrentHttpsResponse->pCallbacks->errorCallback(....); } pCurrentHttpsResponse->syncStatus = status; } .... } 

تحذير PVS-Studio: V522 [CWE-690] قد يكون هناك إلغاء مرجعية لمؤشر فارغ محتمل "pCurrentHttpsResponse". iot_https_client.c 1184

آخر كتلة إذا كان يحتوي على dereferences إشكالية. دعونا معرفة ما يحدث هنا.

تبدأ الوظيفة بمتغيرات pCurrentHttpsResponse و pQItem التي تمت تهيئتها بالقيمة NULL ويتم تهيئة متغير الحالة بواسطة قيمة IOT_HTTPS_OK ، مما يعني أن الأمر كله صحيح.

يتم تعيين قيمة pQItem أخرى ، يتم إرجاعها من دالة IotDeQueue_PeekHead ، والتي تُرجع المؤشر إلى بداية قائمة الانتظار المزدوجة الارتباط.

ماذا يحدث إذا كانت قائمة الانتظار فارغة؟ في هذه الحالة ، ترجع الدالة IotDeQueue_PeekHead NULL:

 static inline IotLink_t* IotDeQueue_PeekHead (const IotDeQueue_t* const pQueue) { return IotListDouble_PeekHead(pQueue); } .... static inline IotLink_t* IotListDouble_PeekHead (const IotListDouble_t* const pList) /* @[declare_linear_containers_list_double_peekhead] */ { IotLink_t* pHead = NULL; if (pList != NULL) { if (IotListDouble_IsEmpty(pList) == false) { pHead = pList->pNext; } } return pHead; } 

علاوة على ذلك ، ستصبح الحالة pQItem == NULL صحيحة وسيتم تمرير تدفق التحكم بواسطة الانتقال إلى الجزء السفلي من الوظيفة. بحلول هذا الوقت ، سيظل مؤشر pCurrentHttpsResponse خاليًا ، في حين أن الحالة لن تكون مساوية لـ IOT_HTTPS_OK . في النهاية ، سنصل إلى نفس الشيء إذا كان الفرع ، و ... الطفرة! حسنا ، أنت تعرف عن عواقب مثل هذا dereference.

حسنا. لقد كان مثالًا صعبًا بعض الشيء. الآن أقترح عليك إلقاء نظرة على إلغاء تسجيل محتمل بسيط للغاية ومفهوم:

 int PKI_mbedTLSSignatureToPkcs11Signature (uint8_t * pxSignaturePKCS, uint8_t * pxMbedSignature ) { int xReturn = 0; uint8_t * pxNextLength; /* The 4th byte contains the length of the R component */ uint8_t ucSigComponentLength = pxMbedSignature[ 3 ]; // <= if( ( pxSignaturePKCS == NULL ) || ( pxMbedSignature == NULL ) ) { xReturn = FAILURE; } .... } 

تحذير PVS-Studio: V595 [CWE-476] تم استخدام مؤشر "pxMbedSignature" قبل أن يتم التحقق منه ضد nullptr. خطوط التحقق: 52 ، 54. iot_pki_utils.c 52

تستقبل هذه الوظيفة مؤشرات إلى uint8_t . يتم التحقق من كلا المؤشرين لـ NULL ، وهي ممارسة جيدة - يجب حل هذه المواقف على الفور.

ولكن هذه هي المشكلة: بحلول الوقت الذي يتم فيه تحديد pxMbedSignature ، سيتم بالفعل إلغاء تأجيلها حرفيًا سطر واحد أعلاه. تا-DAA!

مثال آخر على كود المضاربة:

 CK_RV vAppendSHA256AlgorithmIdentifierSequence ( uint8_t * x32ByteHashedMessage, uint8_t * x51ByteHashOidBuffer ) { CK_RV xResult = CKR_OK; uint8_t xOidSequence[] = pkcs11STUFF_APPENDED_TO_RSA_SIG; if( ( x32ByteHashedMessage == NULL ) || ( x51ByteHashOidBuffer == NULL ) ) { xResult = CKR_ARGUMENTS_BAD; } memcpy( x51ByteHashOidBuffer, xOidSequence, sizeof( xOidSequence ) ); memcpy( &x51ByteHashOidBuffer[ sizeof( xOidSequence ) ], x32ByteHashedMessage, 32 ); return xResult; } 

تحذيرات PVS-Studio:

• V1004 [CWE-628] تم استخدام مؤشر 'x51ByteHashOidBuffer' بطريقة غير آمنة بعد أن تم التحقق منه ضد nullptr. خطوط التحقق: 275 ، 280. iot_pkcs11.c 280
• V1004 [CWE-628] تم استخدام مؤشر 'x32ByteHashedMessage' بطريقة غير آمنة بعد أن تم التحقق منه ضد nullptr. خطوط التحقق: 275 ، 281. iot_pkcs11.c 281

يحذر المحلل من أن المعلمات الدالة التي هي مؤشرات ، يتم استخدامها بشكل غير آمن بعد التحقق من NULL . في الواقع ، يتم فحص الحجج. ولكن في حالة إذا لم يكن أي منهم فارغًا ، فلن يتم اتخاذ أي إجراء باستثناء الكتابة في xResult. هذا القسم من الكود من نوع يقول: "نعم ، لذلك تبين أن الحجج سيئة. سنلاحظ ذلك الآن ، وأنت - تستمر ، استمر. "

النتيجة: سيتم تمرير NULL إلى memcpy. ما يمكن أن يأتي منه؟ أين سيتم نسخ القيم وأيها؟ في الواقع ، لن يساعد التخمين ، حيث ينص المعيار بوضوح على أن مثل هذه الدعوة تؤدي إلى سلوك غير محدد (انظر القسم 1).

هناك أمثلة أخرى لمؤشرات غير صحيحة يتم التعامل معها في تقرير المحلل الموجود في Amazon FreeRTOS ، لكنني أعتقد أن الأمثلة المقدمة كافية لإظهار قدرات PVS-Studio في اكتشاف مثل هذه الأخطاء. دعنا نلقي نظرة على شيء جديد.

صحيح! = 1

كان هناك العديد من الأخطاء المتعلقة بالنمط ، والتي ، للأسف ، غالباً ما يتم تجاهله.

الحقيقة هي أن نوع bool (من C ++) يختلف عن نوع BOOL (يشيع استخدامه في C). الأول يمكن أن يحتوي فقط على قيمة صحيحة أو خاطئة . النوع الثاني هو typedef لنوع عدد صحيح ( int ، long ، وغيرها). القيمة 0 هي "false" لها ، وأي قيمة أخرى مختلفة عن الصفر هي "true".

نظرًا لعدم وجود نوع منطقي مدمج في C ، يتم تعريف هذه الثوابت للراحة:

 #define FALSE 0 #define TRUE 1 

لنلقِ نظرة على المثال.

 int mbedtls_hardware_poll(void* data, unsigned char* output, size_t len, size_t* olen) { int lStatus = MBEDTLS_ERR_ENTROPY_SOURCE_FAILED; HCRYPTPROV hProv = 0; /* Unferenced parameter. */ (void)data; /* * This is port-specific for the Windows simulator, * so just use Crypto API. */ if (TRUE == CryptAcquireContextA( &hProv, NULL, NULL, PROV_RSA_FULL, CRYPT_VERIFYCONTEXT)) { if (TRUE == CryptGenRandom(hProv, len, output)) { lStatus = 0; *olen = len; } CryptReleaseContext(hProv, 0); } return lStatus; } 

تحذيرات PVS-Studio:

• V676 [CWE-253] من الخطأ مقارنة متغير نوع BOOL مع TRUE. aws_entropy_hardware_poll.c 48
• V676 [CWE-253] من الخطأ مقارنة متغير نوع BOOL مع TRUE. التعبير الصحيح هو: 'FALSE! = CryptGenRandom (hProv ، len ، الإخراج)'. aws_entropy_hardware_poll.c 51

هل وجدت خطأ؟ لا شك ، إنها هنا :) الدالتان CryptAcquireContextA و CryptGenRandom هي وظائف قياسية من رأس wincrypt.h . إذا نجحت ، فإنها تُرجع القيمة غير الصفرية. اسمحوا لي أن أؤكد أنه ليس صفرا . لذلك ، من الناحية النظرية ، يمكن أن تكون أي قيمة مختلفة عن الصفر: 1 ، 314 ، 42 ، 420 .

على ما يبدو ، لم يكن المبرمج الذي كان يكتب الوظيفة من المثال ، يفكر في ذلك ، وفي النهاية ، تتم مقارنة القيم الناتجة مع واحدة.

ما مدى احتمال عدم استيفاء شرط TRUE == CryptGenRandom (....) ؟ من الصعب القول. ربما ، قد يُرجع CryptGenRandom 1 أكثر من القيم الأخرى ، لكن ربما قد يرجع 1 فقط. لا يمكننا معرفة هذا بالتأكيد: تنفيذ هذه الوظيفة المشفرة مخفي عن أعين المبرمجين البشر :)

من المهم أن نتذكر أن هذه المقارنات قد تكون خطيرة. بدلا من:

 if (TRUE == GetBOOL()) 

استخدم نسخة أكثر أمانًا من الكود:

 if (FALSE != GetBOOL()) 

مشاكل التحسين

ارتبطت عدة تحذيرات من محلل لهياكل التشغيل ببطء. على سبيل المثال:

 int _IotHttpsDemo_GetS3ObjectFileSize(....) { .... pFileSizeStr = strstr(contentRangeValStr, "/"); .... } 

تحذير PVS-Studio: V817 يعد البحث عن حرف "/" أكثر فعالية من سلسلة. iot_demo_https_common.c 205

انها قصيرة وبسيطة ، أليس كذلك؟ يتم استخدام وظيفة strstr هنا للبحث عن حرف واحد فقط ، يتم تمريره في المعلمة كسلسلة (يكون في علامات اقتباس مزدوجة).

يمكن تحسين هذا المكان عن طريق استبدال strstr بـ strchr :

 int _IotHttpsDemo_GetS3ObjectFileSize(....) { .... pFileSizeStr = strchr(contentRangeValStr, '/'); .... } 

بهذه الطريقة ، سيعمل البحث بشكل أسرع قليلاً. شيء صغير ، لكن لطيف.

حسنًا ، تعد مثل هذه التحسينات جيدة ، لكن المحلل قد وجد أيضًا مكانًا آخر ، والذي يمكن تحسينه بطريقة أكثر وضوحًا:

 void vRunOTAUpdateDemo(void) { .... for (; ; ) { .... xConnectInfo.cleanSession = true; xConnectInfo.clientIdentifierLength = (uint16_t)strlen(clientcredentialIOT_THING_NAME); xConnectInfo.pClientIdentifier = clientcredentialIOT_THING_NAME; .... } } 

تحذير PVS-Studio: V814 انخفاض الأداء. تم استدعاء الوظيفة "strlen" عدة مرات داخل جسم الحلقة. aws_iot_ota_update_demo.c 235

هم ... داخل الحلقة ، مع كل strlen يسمى تكرار الذي يقيم طول نفس الخط في كل مرة. ليست العملية الأكثر فعالية :)

دعونا نلقي نظرة على تعريف clientcredentialIOT_THING_NAME :

 /* * @brief Host name. * * @todo Set this to the unique name of your IoT Thing. */ #define clientcredentialIOT_THING_NAME "" 

يطلب من المستخدم إدخال اسم أجهزتهم هنا. بشكل افتراضي ، إنه فارغ ، وفي هذه الحالة ، كل شيء على ما يرام. ماذا لو أراد المستخدم إدخال اسم طويل وجميل هناك؟ على سبيل المثال ، أحب أن أسمي من بنات أفكاري " آلة القهوة المتحمسة والمتطورة BarBarista-N061E The Ultimate Edition ." هل يمكنك أن تتخيل ما سيكون عليه المفاجأة إذا بدأت آلة القهوة الجميلة في العمل بشكل أبطأ قليلاً بعد ذلك؟ ازعاج!

لتصحيح الخطأ ، يجدر أخذ strlen خارج حلقة الجسم. بعد كل شيء ، لا يتغير اسم الجهاز أثناء عمل البرنامج. أوه ، سوف constexpr من C ++ تناسب تماما هنا ...

حسنًا ، حسنًا ، لا نذهب إلى الزنبق. كما لاحظ زميلي أندري كاربوف ، يعرف المترجمون العصريون ما هو شاق ، وشاهدهم شخصياً باستخدام ثابت في الكود الثنائي إذا حصلوا على أن طول الخط لا يمكن أن يتغير. لذلك هناك فرصة جيدة في وضع إصدار الإصدار ، بدلاً من تقييم طول الخط الحقيقي ، سيتم استخدام القيمة التي تم تقييمها مسبقًا. ومع ذلك ، لا يعمل هذا دائمًا ، لذا فإن كتابة هذه التعليمات البرمجية ليست ممارسة جيدة.

بضع كلمات عن ميسرا

يحتوي محلل PVS-Studio على مجموعة كبيرة من القواعد للتحقق من شفرتك للتأكد من امتثالها لمعايير MISRA C و MISRA C. ما هي هذه المعايير؟

MISRA هو معيار الترميز للأنظمة المدمجة عالية المسؤولية. أنه يحتوي على مجموعة من القواعد والإرشادات الصارمة لكتابة التعليمات البرمجية وإعداد عملية التطوير. هذه القواعد كثيرة جدًا ، وهي لا تهدف فقط إلى القضاء على الأخطاء الخطيرة ، ولكن أيضًا إلى "روائح الكود" المختلفة. ويهدف أيضًا إلى كتابة الكود الأكثر قابلية للفهم وقراءة.

وبالتالي ، فإن معيار MISRA التالي لا يساعد فقط في تجنب الأخطاء ونقاط الضعف ، ولكن أيضًا يقلل بشكل كبير من احتمال ظهورها في الكود الموجود بالفعل.

يستخدم MISRA في الصناعات الجوية والطبية والعسكرية ، حيث تعتمد حياة البشر على جودة البرامج المدمجة.

على ما يبدو ، يعرف مطورو Amazon FreeRTOS بهذا المعيار ، ويتابعونه في معظم الأحيان. مثل هذا النهج معقول تمامًا: إذا كتبت نظام تشغيل عريض القاعدة للأنظمة المدمجة ، فيجب عليك التفكير في الأمان.

ومع ذلك ، فقد وجدت الكثير من الانتهاكات لمعيار MISRA. لن أعطي أمثلة على قواعد مثل "لا تستخدم الاتحاد" أو "الوظيفة يجب أن يكون لها عائد واحد فقط في نهاية الجسم" - لسوء الحظ ، فهي ليست مذهلة ، كما هي الحال مع معظم قواعد MISRA. أفضل أن أعطيك أمثلة على الانتهاكات التي قد تؤدي إلى عواقب وخيمة.

لنبدأ مع وحدات الماكرو:

 #define FreeRTOS_ms_to_tick(ms) ( ( ms * configTICK_RATE_HZ + 500 ) / 1000 ) 

 #define SOCKETS_htonl( ulIn ) ( ( uint32_t ) \ ( ( ( ulIn & 0xFF ) << 24 ) | ( ( ulIn & 0xFF00 ) << 8 ) \ | ( ( ulIn & 0xFF0000 ) >> 8 ) | ( ( ulIn & 0xFF000000 ) >> 24 ) ) ) 

 #define LEFT_ROTATE( x, c ) ( ( x << c ) | ( x >> ( 32 - c ) ) ) 

تحذيرات PVS-Studio:

• V2546 [MISRA C 20.7] يجب وضع الماكرو ومعلماته بين قوسين. النظر في فحص المعلمة "مللي ثانية" من الماكرو "FreeRTOS_ms_to_tick". FreeRTOS_IP.h 201
• V2546 [MISRA C 20.7] يجب وضع الماكرو ومعلماته بين قوسين. النظر في فحص المعلمة 'ulIn' من الماكرو 'SOCKETS_htonl'. iot_secure_sockets.h 512
• V2546 [MISRA C 20.7] يجب وضع الماكرو ومعلماته بين قوسين. ضع في اعتبارك فحص المعلمات 'x' و 'c' للماكرو 'LEFT_ROTATE'. iot_device_metrics.c 90

نعم ، هذا بالضبط ما تفكر فيه. معلمات وحدات الماكرو هذه ليست بين قوسين. إذا كان شخص ما يكتب عن طريق الخطأ شيء من هذا القبيل

 val = LEFT_ROTATE(A[i] | 1, B); 

سيتم توسيع "استدعاء" ماكرو إلى:

 val = ( ( A[i] | 1 << B ) | ( A[i] | 1 >> ( 32 - B ) ) ); 

تذكر أولويات العمليات؟ أولاً ، يتم إجراء تحول bitwise ، وفقط بعد ذلك - "bitwise" أو "bit". لذلك ، سيتم كسر منطق البرنامج. مثال أبسط: ماذا سيحدث إذا تم تمرير التعبير " x + y " في الماكرو FreeRTOS_ms_to_tick ؟ أحد الأهداف الرئيسية لـ MISRA هو منع مثل هذه الحالات.

قد يجادل البعض ، "إذا كان لديك مبرمجين لا يعرفون ذلك ، فلن يساعدك أي معيار!" لن أتفق مع ذلك. المبرمجون هم أيضًا أشخاص ، وبغض النظر عن مدى خبرة أي شخص ، يمكن أيضًا أن يتعبوا ويخطئوا في نهاية اليوم. هذا هو أحد الأسباب التي تجعل MISRA توصي بشدة باستخدام أدوات التحليل التلقائي لاختبار مشروع للامتثال.

اسمحوا لي أن أتطرق إلى مطوري Amazon FreeRTOS: لقد عثر PVS-Studio على 12 وحدة ماكرو أخرى غير آمنة ، لذا عليك أن تكون حريصًا معهم :)

انتهاك MISRA آخر مثير للاهتمام:

 /** * @brief Callback for an asynchronous request to notify * that the response is complete. * * @param[in] 0pPrivData - User private data configured * with the HTTPS Client library request configuration. * @param[in] respHandle - Identifier for the current response finished. * @param[in] rc - Return code from the HTTPS Client Library * signaling a possible error. * @param[in] status - The HTTP response status. */ static void _responseCompleteCallback(void* pPrivData, IotHttpsResponseHandle_t respHandle, IotHttpsReturnCode_t rc, uint16_t status) { bool* pUploadSuccess = (bool*)pPrivData; /* When the remote server response with 200 OK, the file was successfully uploaded. */ if (status == IOT_HTTPS_STATUS_OK) { *pUploadSuccess = true; } else { *pUploadSuccess = false; } /* Post to the semaphore that the upload is finished. */ IotSemaphore_Post(&(_uploadFinishedSem)); } 

يمكنك أن تجد علة نفسك؟

تحذير PVS-Studio: V2537 [MISRA C 2.7] يجب ألا تحتوي الوظائف على معلمات غير مستخدمة. النظر في فحص المعلمة: 'rc'. iot_demo_https_s3_upload_async.c 234

ألقِ نظرة فاحصة: لا تستخدم المعلمة rc في أي مكان في الجسم الوظيفي. بينما يقول تعليق الوظيفة بوضوح أن هذه المعلمة هي رمز إرجاع لوظيفة أخرى ، وأنه يمكن أن يشير إلى وجود خطأ. لماذا لا يتم التعامل مع هذه المعلمة بأي شكل من الأشكال؟ هناك خطأ ما بوضوح هنا.

ومع ذلك ، حتى بدون هذه التعليقات ، غالبًا ما تشير المعلمات غير المستخدمة إلى المنطق المعطوب للبرنامج. وإلا ، لماذا تحتاجها في توقيع الوظيفة؟

لقد قدمت هنا وظيفة صغيرة جيدة للحصول على مثال في المقال. بالإضافة إلى ذلك ، لقد وجدت 10 المعلمات غير المستخدمة الأخرى. يتم استخدام الكثير منها في وظائف أكبر ، وليس من السهل اكتشافها.

بشكل مثير للريبة ، لم يتم العثور عليها من قبل. بعد كل شيء ، بسهولة مترجمين اكتشاف مثل هذه الحالات.

استنتاج

لم تكن هذه جميع المشكلات التي وجدها المحلل ، ولكن تبين أن المقالة كبيرة جدًا بالفعل. آمل أن يتمكن مطورو amazon FreeRTOS بفضل ذلك من تصحيح بعض أوجه القصور ، وقد يرغبون في تجربة برنامج PVS-Studio لوحدهم. بهذه الطريقة سيكون أكثر ملاءمة للتحقيق بدقة التحذيرات. في واقع الأمر - العمل مع واجهة مريحة أسهل بكثير من النظر في تقرير نصي.

شكرا لقراءة مقالاتنا! نراكم في المنشور التالي: د

ملاحظة: لقد حدث هذا المقال في 31 أكتوبر. هابي هالوين ، شباب وبنات!