Atas perintah pengembang Tertanam: mencari bug di Amazon FreeRTOS

Setiap orang yang memprogram pengendali mikrokontroler mungkin tahu tentang FreeRTOS, atau setidaknya mendengar tentang sistem operasi ini. Orang-orang Amazon memutuskan untuk memperluas kemampuan sistem operasi ini untuk bekerja dengan layanan AWS Internet of Things - ini adalah bagaimana Amazon FreeRTOS muncul. Kami, pengembang alat analisis kode PVS-Studio, diminta memeriksa proyek-proyek ini melalui pos dan dalam komentar di bawah artikel. Nah, Anda bertanya - kami melakukannya. Apa yang terjadi - baca terus.

Gambar 3

Sedikit tentang proyek


Untuk memulai, saya akan memberi tahu Anda sedikit tentang "ayah" dari proyek yang diperiksa - FreeRTOS (Anda dapat menemukan kode sumber di sini ). Seperti yang dikatakan Wikipedia, FreeRTOS adalah sistem operasi waktu-nyata multi-tasking untuk sistem tertanam.

Itu ditulis dalam C tua yang baik, yang tidak mengejutkan - sistem operasi ini harus bekerja dalam kondisi khas mikrokontroler: daya komputasi rendah, sejumlah kecil RAM, dan sejenisnya. Bahasa C memungkinkan Anda untuk bekerja dengan sumber daya pada tingkat rendah dan memiliki kinerja tinggi, sehingga sangat cocok untuk pengembangan OS semacam itu.

Sekarang kembali ke Amazon, yang tidak tinggal diam dan berkembang di berbagai bidang yang menjanjikan. Sebagai contoh, Amazon sedang mengembangkan mesin AAA game Amazon Lumberyard, yang juga kami uji .

Salah satu bidang tersebut adalah Internet of Things (Internet of Things, IoT). Untuk mengembangkan di bidang ini, Amazon memutuskan untuk menulis sistem operasinya sendiri - dan mereka mengambil kernel FreeRTOS sebagai basis.

Sistem yang dihasilkan - Amazon FreeRTOS - diposisikan sebagai "menyediakan kemampuan untuk terhubung dengan aman ke Amazon Web Services, seperti AWS IoT Core atau AWS IoT Greengrass." Kode sumber untuk proyek ini disimpan di Github.

Pada artikel ini, kita akan memeriksa apakah ada kesalahan dalam FreeRTOS, serta seberapa aman sistem operasi dari Amazon dalam hal analisis kode statis.

Bagaimana ceknya


Kode diperiksa menggunakan alat pencarian kesalahan otomatis: Penganalisis kode statis PVS-Studio. Itu mampu mendeteksi kesalahan dalam program yang ditulis dalam C, C ++, C # dan Java.

Sebelum memulai analisis, Anda perlu merakit proyek - jadi saya akan yakin bahwa saya memiliki semua dependensi yang diperlukan dan semuanya sesuai dengan proyek. Ada beberapa cara untuk memverifikasi proyek - misalnya, menggunakan sistem pemantauan kompilasi. Saya melakukan analisis menggunakan plug-in untuk Visual Studio - ada baiknya repositori dari kedua proyek memiliki satu set file proyek yang membuatnya mudah dibangun di bawah Windows.

Yang saya butuhkan hanyalah mengumpulkan proyek untuk memastikan bahwa ada semua yang diperlukan untuk verifikasi. Selanjutnya, saya meluncurkan analisis dan voila! - di depan saya laporan analisa yang sudah jadi.

Perpustakaan pihak ketiga yang termasuk dalam proyek-proyek ini juga dapat mengandung kesalahan, dan mereka, tentu saja, juga dapat mempengaruhi operasi program. Namun, saya mengecualikan mereka dari analisis demi kemurnian narasi.

Jadi, proyek dianalisis, laporan diterima, kesalahan menarik ditulis. Saatnya beralih ke analisis mereka!

Apa yang menyembunyikan FreeRTOS


Awalnya, saya berharap untuk menulis dua artikel terpisah: satu untuk setiap sistem operasi. Saya sudah menggosok tangan saya, bersiap untuk menulis artikel bagus tentang FreeRTOS. Mengantisipasi deteksi bahkan beberapa kesalahan berair (seperti CWE-457 ), saya bersemangat melihat beberapa peringatan penganalisa, dan ... dan tidak ada. Saya tidak menemukan kesalahan yang menarik.

Banyak peringatan yang dikeluarkan analis tidak relevan untuk FreeRTOS. Sebagai contoh, peringatan tersebut adalah kekurangan 64-bit seperti casting size_t ke uint32_t . Ini karena fakta bahwa FreeRTOS dirancang untuk bekerja pada perangkat dengan ukuran pointer tidak lebih dari 32 bit.

Saya hati-hati memeriksa semua peringatan V1027 yang terkait dengan gips antara pointer ke struktur yang tidak terkait. Jika struktur yang dapat direduksi memiliki keselarasan yang sama, maka gips semacam itu bukan kesalahan. Dan saya tidak menemukan satu pun pemain berbahaya!

Semua tempat mencurigakan lainnya terkait dengan gaya pengkodean, atau dilengkapi dengan komentar yang menjelaskan mengapa ini dilakukan persis di sini dan mengapa ini bukan kesalahan.

Secara umum, saya ingin menghubungi pengembang FreeRTOS. Kalian benar-benar hebat! Kami hampir tidak pernah bertemu proyek yang bersih dan berkualitas tinggi seperti milik Anda. Dan saya sangat senang membaca kode yang bersih, rapi, dan terdokumentasi dengan baik. Angkat topi untukmu.

Meskipun saya tidak dapat menemukan kesalahan yang menarik hari itu, saya mengerti bahwa saya tidak akan berhenti di situ. Saya pulang dengan keyakinan kuat bahwa sesuatu yang menarik akan ditemukan dalam versi dari Amazon 100%, dan bahwa besok saya pasti akan mengumpulkan cukup banyak kesalahan untuk artikel tersebut. Seperti yang mungkin Anda tebak, saya benar.

Apa yang menyembunyikan Amazon FreeRTOS


Versi sistem Amazon ternyata ... dengan kata lain, sedikit lebih buruk. Warisan FreeRTOS tetap sama bersihnya, tetapi revisi baru ternyata cukup menarik.

Di beberapa tempat, logika program dilanggar, di suatu tempat salah bekerja dengan pointer. Di beberapa tempat, kode dapat menyebabkan perilaku yang tidak terdefinisi, tetapi di suatu tempat programmer tidak tahu tentang pola kesalahan yang ia buat. Saya bahkan menemukan beberapa potensi kerentanan serius.

Sesuatu yang saya tunda dengan pengantar. Ayo mulai parsing bug!

Pelanggaran logika program


Mari kita mulai dengan area masalah, yang dengan jelas menunjukkan bahwa program tidak berjalan persis seperti yang diharapkan oleh programmer. Tempat pertama seperti itu akan merupakan karya mencurigakan dengan sebuah array:

/** * @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 mengeluarkan dua peringatan untuk kode ini:

  • V619 Array '_requestPool.pRequestDatas' digunakan sebagai penunjuk ke objek tunggal. iot_demo_https_s3_download_async.c 973
  • V574 Pointer '_requestPool.pRequestDatas' digunakan secara bersamaan sebagai array dan sebagai pointer ke objek tunggal. Periksa baris: 931, 973. iot_demo_https_s3_download_async.c 973

Untuk jaga-jaga, izinkan saya mengingatkan Anda: nama array adalah pointer ke elemen pertama. Yaitu, jika _requestPool.pRequestDatas adalah array struktur, maka _requestPool.pRequestDatas [i] .scheduled adalah akses ke anggota terjadwal dari struktur ke- l array. Dan jika Anda menulis _requestPool.pRequestDatas-> terjadwal , ini akan berarti akses ke anggota terjadwal dari struktur pertama array.

Inilah yang terjadi pada cuplikan kode di atas. Baris terakhir selalu mengubah nilai hanya untuk anggota struktur pertama array. Dengan sendirinya, panggilan ini sudah mencurigakan, tetapi situasi di sini bahkan lebih jelas: seluruh seluruh fungsi, array _requestPool.pRequestDatas diakses oleh indeks, dan hanya pada akhir operasi pengindeksan yang lupa untuk diterapkan.

Seperti yang saya pahami, baris terakhir akan terlihat seperti ini:

 _requestPool.pRequestDatas[reqIndex].scheduled = true; 

Kesalahan berikut terletak pada fungsi kecil, jadi saya akan memberikannya secara keseluruhan:

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

Peringatan PVS-Studio: V642 [CWE-197] Menyimpan hasil fungsi 'strncmp' di dalam variabel tipe 'pendek' tidak tepat. Bit signifikan bisa hilang melanggar logika program. aws_greengrass_discovery.c 637

Mari kita lihat definisi fungsi strncmp:

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

Dalam contoh, hasil dari tipe int , yang ukurannya 32 bit, dikonversi ke variabel tipe int16_t . Dengan konversi "penyempitan" seperti itu, bit paling signifikan dari nilai pengembalian akan hilang. Misalnya, jika fungsi strncmp mengembalikan 0x00010000 , maka yang akan hilang selama konversi, dan kondisinya akan terpenuhi.

Bahkan, aneh melihat para pemain dalam kondisi seperti itu. Mengapa bahkan melakukannya jika Anda dapat membandingkan int normal dengan nol? Di sisi lain, jika programmer secara sadar ingin fungsi tersebut terkadang mengembalikan true , bahkan jika tidak, maka mengapa perilaku rumit ini tidak dijelaskan oleh komentar? Tapi kemudian ini sudah bookmark. Secara umum, saya cenderung percaya bahwa ini adalah kesalahan. Apa yang kamu pikirkan

Perilaku dan petunjuk yang tidak terdefinisi


Sekarang akan ada contoh yang agak besar. Itu menyembunyikan potensi referensi dari pointer nol:

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

Peringatan PVS-Studio: V522 [CWE-690] Mungkin ada dereferensi dari penunjuk null potensial 'pCurrentHttpsResponse'. iot_https_client.c 1184

Dereferensi masalah ada di bagian paling bawah jika . Mari kita lihat apa yang terjadi di sini.

Pada awal fungsi, variabel pCurrentHttpsResponse dan pQItem diinisialisasi ke NULL , dan variabel status diinisialisasi ke IOT_HTTPS_OK , yang berarti semuanya berjalan dengan lancar.

Selanjutnya, pQItem diberi nilai yang dikembalikan dari fungsi IotDeQueue_PeekHead , yang mengembalikan pointer ke awal antrian yang terhubung dua kali lipat.

Apa yang terjadi jika antrian kosong? Dalam hal ini, fungsi IotDeQueue_PeekHead akan mengembalikan 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; } 

Selanjutnya, kondisi pQItem == NULL puas, dan aliran kontrol berlanjut melalui goto ke bagian bawah dari fungsi body. Pada saat ini, pointer pCurrentHttpsResponse akan tetap nol, dan statusnya tidak lagi sama dengan IOT_HTTPS_OK . Akibatnya, kita akan jatuh ke cabang itu jika , dan ... lulus! Konsekuensi dereferencing ini, Anda sendiri tahu.

Baiklah Itu adalah contoh yang sedikit berornamen. Sekarang saya bawa perhatian Anda potensi dereferencing yang sangat sederhana dan dapat dipahami:

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

Peringatan PVS-Studio: V595 [CWE-476] Pointer 'pxMbedSignature' digunakan sebelum diverifikasi terhadap nullptr. Periksa baris: 52, 54. iot_pki_utils.c 52

Fungsi ini mendapat dua petunjuk untuk uint8_t . Kedua petunjuk diperiksa untuk NULL , yang merupakan praktik yang baik - situasi seperti itu harus segera diselesaikan.

Tapi ini sialnya: pada saat pxMbedSignature dicentang, itu sudah akan direferensikan secara harfiah satu baris di atas. Ta-daa!

Contoh lain dari kode spekulatif:

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

Peringatan PVS-Studio:

  • V1004 [CWE-628] Pointer 'x51ByteHashOidBuffer' digunakan secara tidak aman setelah diverifikasi terhadap nullptr. Periksa baris: 275, 280. iot_pkcs11.c 280
  • V1004 [CWE-628] Pointer 'x32ByteHashedMessage' digunakan secara tidak aman setelah diverifikasi terhadap nullptr. Periksa baris: 275, 281. iot_pkcs11.c 281

Penganalisa memperingatkan bahwa parameter fungsi yang pointer digunakan tidak aman setelah mereka diuji untuk NULL . Memang, argumen diperiksa, tetapi jika salah satu dari mereka ternyata NULL , tidak ada tindakan yang diambil, kecuali untuk menulis ke xResult . Sepotong kode ini sepertinya mengatakan: β€œYa, itu berarti argumennya ternyata buruk. Kami akan menuliskannya sekarang, selagi Anda melanjutkan, lanjutkan. "

Intinya: NULL akan diteruskan ke memcpy . Apa yang bisa terjadi dengan ini? Di mana nilai-nilai akan disalin dan yang mana? Bahkan, menebak tentang hal ini tidak layak, karena standar dengan jelas menyatakan bahwa panggilan semacam itu mengarah pada perilaku yang tidak terdefinisi (lihat paragraf 1).

Gambar 2


Laporan analisa masih berisi contoh operasi yang salah dengan pointer yang saya temukan di Amazon FreeRTOS, tapi saya pikir contoh yang diberikan sudah cukup untuk menunjukkan kepada Anda kemampuan PVS-Studio dalam mendeteksi kesalahan tersebut. Pertimbangkan sesuatu yang baru.

BENAR! = 1


Beberapa kesalahan yang saya temukan terkait dengan satu pola, yang, sayangnya, sering dilupakan.

Faktanya adalah bahwa tipe bool (dari C ++) berbeda dari tipe BOOL (biasanya digunakan dalam C). Yang pertama hanya bisa berisi benar atau salah . Yang kedua adalah typedef dari beberapa tipe integer ( int , long , etc.). Baginya, "false" adalah nilai 0 , dan "true" adalah nilai selain nol.

Karena tidak ada tipe Boolean bawaan di C, konstanta ini didefinisikan untuk kenyamanan:

 #define FALSE 0 #define TRUE 1 

Sekarang pertimbangkan sebuah contoh:

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

Peringatan PVS-Studio:

  • V676 [CWE-253] Tidak benar membandingkan variabel tipe BOOL dengan TRUE. aws_entropy_hardware_poll.c 48
  • V676 [CWE-253] Tidak benar membandingkan variabel tipe BOOL dengan TRUE. Ekspresi yang benar adalah: 'FALSE! = CryptGenRandom (hProv, len, output)'. aws_entropy_hardware_poll.c 51

Menemukan kesalahan? Dan itu :) Fungsi CryptAcquireContextA dan CryptGenRandom adalah fungsi standar dari header wincrypt.h . Jika berhasil, mereka mengembalikan nilai bukan nol. Saya tekankan - bukan nol . Jadi, secara teoritis, ini bisa berupa nilai selain nol: 1 , 314 , 42 , 420 .

Rupanya, programmer yang menulis fungsi dari contoh tidak memikirkannya, dan sebagai hasilnya, nilai yang diperoleh dibandingkan dengan kesatuan.

Dengan probabilitas apa kondisi TRUE == CryptGenRandom (....) tidak terpenuhi? Sulit dikatakan. Mungkin CryptGenRandom mengembalikan unit lebih sering daripada nilai-nilai lain, dan mungkin selalu mengembalikan hanya satu. Kita tidak tahu pasti: penerapan fungsi kriptografis ini disembunyikan dari mata programmer fana :)

Penting untuk diingat bahwa perbandingan semacam itu berpotensi berbahaya. Dan bukannya:

 if (TRUE == GetBOOL()) 

gunakan opsi yang lebih aman:

 if (FALSE != GetBOOL()) 

Masalah Optimasi


Beberapa peringatan penganalisa telah dikaitkan dengan konstruksi yang berjalan lambat. Sebagai contoh:

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

PVS-Studio Warning: V817 Lebih efisien untuk mencari karakter '/' daripada string. iot_demo_https_common.c 205

Secara singkat dan jelas, bukan? Fungsi strstr di sini digunakan untuk mencari hanya satu karakter, yang diteruskan ke parameter sebagai string (itu dilampirkan dalam tanda kutip ganda).

Tempat ini dapat berpotensi dioptimalkan dengan mengganti strstr dengan strchr :

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

Maka pencarian akan bekerja sedikit lebih cepat. Agak, tapi bagus.

Optimalisasi semacam itu, tentu saja, bagus, dan penganalisa telah menemukan tempat lain yang dapat dioptimalkan dengan lebih jelas:

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

Peringatan PVS-Studio: V814 Penurunan kinerja. Fungsi 'strlen' disebut beberapa kali di dalam tubuh loop. aws_iot_ota_update_demo.c 235

Hmmm ... Di dalam loop, pada setiap iterasi, strlen disebut, yang setiap kali menghitung panjang garis yang sama. Bukan operasi yang paling efisien :)

Mari kita lihat definisi clientcredentialIOT_THING_NAME :

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

Pengguna diminta untuk memasukkan nama perangkatnya di sini. Secara default, ini kosong, dan dalam hal ini, semuanya baik-baik saja. Tetapi bagaimana jika pengguna ingin memasukkan nama panjang dan indah di sana? Sebagai contoh, saya ingin menyebut gagasan saya " Mesin Kopi yang Bergairah dan Canggih BarBarista-N061E Edisi Tertinggi ". Bisakah Anda bayangkan betapa terkejutnya saya jika setelah itu mesin kopi saya yang indah mulai bekerja sedikit lebih lambat? Kekacauan!

Untuk memperbaiki kesalahan, strlen harus dikeluarkan dari badan loop. Lagi pula, nama perangkat tidak berubah saat program sedang berjalan. Ehhh, ini akan menjadi constexpr dari C ++ ...

Oke, oke, saya akui: di sini saya sedikit menebal. Seperti kolega saya Andrei Karpov catat, kompiler modern tahu apa strlen dan dia secara pribadi mengamati bagaimana mereka hanya menggunakan konstanta dalam kode biner, jika mereka mengerti bahwa panjang string tidak dapat berubah. Jadi ada probabilitas tinggi bahwa dalam mode build versi rilis, alih-alih perhitungan nyata dari panjang string, nilai yang dihitung sebelumnya hanya akan digunakan. Namun, ini tidak selalu berhasil, jadi menulis kode seperti itu bukan praktik yang baik.

Beberapa kata tentang MISRA


Alat analisis PVS-Studio memiliki seperangkat aturan besar yang memungkinkan Anda memeriksa kode Anda untuk kesesuaian dengan standar MISRA C dan MISRA C ++. Apa standar ini?

MISRA adalah standar pengkodean untuk sistem tertanam yang sangat responsif. Ini berisi seperangkat aturan ketat dan pedoman untuk menulis kode dan mengatur proses pengembangan. Ada beberapa aturan ini, dan mereka ditujukan tidak hanya untuk menghilangkan kesalahan serius, tetapi juga pada berbagai "bau kode", serta penulisan kode yang paling mudah dipahami dan dibaca.

Dengan demikian, mengikuti standar MISRA tidak hanya membantu menghindari kesalahan dan kerentanan, tetapi juga secara signifikan - signifikan! - mengurangi kemungkinan kemunculannya dalam kode yang ada.

MISRA digunakan dalam industri kedirgantaraan, medis, otomotif dan militer - di mana kehidupan manusia bergantung pada kualitas perangkat lunak yang disematkan.

Tampaknya, pengembang FreeRTOS Amazon mengetahui standar ini, dan sebagian besar mengikutinya. Itu benar: jika Anda menulis OS berbasis luas untuk sistem embedded, maka Anda harus memikirkan keamanan.

Namun, saya menemukan beberapa pelanggaran terhadap standar MISRA. Di sini saya tidak akan memberikan contoh aturan seperti "jangan gunakan serikat" atau "fungsi seharusnya hanya memiliki satu pengembalian di akhir tubuh" - sayangnya, mereka tidak spektakuler, seperti kebanyakan aturan MISRA. Saya lebih baik memberi Anda contoh-contoh pelanggaran yang berpotensi menyebabkan konsekuensi serius.

Mari kita mulai dengan makro:

 #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 ) ) ) 

Peringatan PVS-Studio:

  • V2546 [MISRA C 20.7] Makro dan parameternya harus dilampirkan dalam tanda kurung. Pertimbangkan untuk memeriksa parameter 'ms' dari makro 'FreeRTOS_ms_to_tick'. FreeRTOS_IP.h 201
  • V2546 [MISRA C 20.7] Makro dan parameternya harus dilampirkan dalam tanda kurung. Pertimbangkan untuk memeriksa parameter 'ulIn' dari makro 'SOCKETS_htonl'. iot_secure_sockets.h 512
  • V2546 [MISRA C 20.7] Makro dan parameternya harus dilampirkan dalam tanda kurung. Pertimbangkan untuk memeriksa parameter 'x', 'c' dari makro 'LEFT_ROTATE'. iot_device_metrics.c 90

Ya, ini persis seperti yang Anda pikirkan. Parameter makro ini tidak dibungkus dengan tanda kurung. Jika seseorang secara tidak sengaja menulis sesuatu seperti

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

maka makro "panggilan" semacam itu akan terbuka di:

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

Ingat prioritas operasi? Pertama, pergeseran bitwise dilakukan, dan hanya setelah itu bitwise β€œatau”. Karena itu, logika program akan dilanggar. Contoh yang lebih sederhana: apa yang terjadi jika ekspresi " x + y " dilewatkan ke makro FreeRTOS_ms_to_tick ? Salah satu tujuan utama MISRA adalah untuk mencegah terjadinya situasi seperti itu.

Seseorang mungkin keberatan: "jika Anda memiliki programmer yang tidak tahu tentang ini, maka tidak ada standar yang akan menyelamatkan Anda!" Dan saya tidak akan setuju dengan ini. Pemrogram juga manusia, dan tidak peduli seberapa berpengalaman seseorang, ia juga bisa lelah dan membuat kesalahan di akhir hari kerja. Ini adalah salah satu alasan MISRA sangat merekomendasikan penggunaan alat analisis otomatis untuk memvalidasi proyek terhadap standar.

Saya beralih ke pengembang Amazon FreeRTOS: PVS-Studio menemukan 12 makro yang lebih tidak aman, jadi Anda lebih berhati-hati di sana bersama mereka :)

Pelanggaran MISRA lain yang menarik:

 /** * @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)); } 

Bisakah Anda menemukan kesalahan sendiri?

Peringatan PVS-Studio: V2537 [MISRA C 2.7] Fungsi tidak boleh memiliki parameter yang tidak digunakan. Pertimbangkan memeriksa parameter: 'rc'. iot_demo_https_s3_upload_async.c 234

Lihatlah lebih dekat: parameter rc tidak digunakan di mana saja di badan fungsi. Selain itu, dalam komentar tentang fungsi itu secara eksplisit ditulis bahwa parameter ini adalah kode pengembalian fungsi lain, dan bahwa itu dapat menandakan kesalahan. Lalu mengapa parameter ini tidak diproses dengan cara apa pun? Jelas ada sesuatu yang salah di sini.

Namun, bahkan tanpa komentar tersebut, parameter yang tidak digunakan sering menunjukkan logika program rusak. Kalau tidak, mengapa mereka diperlukan dalam tanda tangan fungsi?

Di sini saya telah memberikan fungsi kecil yang cocok untuk contoh di artikel. Selain dia, saya menemukan 10 parameter yang tidak digunakan lagi. Banyak dari mereka yang digunakan dalam fungsi yang lebih besar, dan menemukannya bukanlah hal yang mudah.

Sangat mencurigakan bahwa mereka tidak ditemukan sebelumnya. Memang, kompiler dapat dengan mudah mendeteksi kasus-kasus seperti itu.

Gambar 1


Kesimpulan


Ini tidak semua bidang masalah yang ditemukan oleh penganalisa, tetapi artikel itu sudah cukup besar. Saya berharap bahwa berkat itu, pengembang Amazon FreeRTOS akan dapat memperbaiki beberapa kekurangan, dan mungkin bahkan ingin mencoba PVS-Studio sendiri. Dengan cara ini, dimungkinkan untuk mempelajari peringatan secara lebih menyeluruh, dan memang, bekerja dengan antarmuka yang nyaman jauh lebih mudah daripada melihat laporan teks.

Terima kasih telah membaca artikel kami! Sampai jumpa di edisi berikutnya: D

PS Kebetulan artikel ini diterbitkan pada tanggal 31 Oktober. Karena itu, saya berharap semua orang selamat Halloween!



Jika Anda ingin berbagi artikel ini dengan audiens yang berbahasa Inggris, silakan gunakan tautan ke terjemahan: George Gribkov. Atas permintaan Pengembang Tertanam: Mendeteksi Kesalahan di Amazon FreeRTOS .

Source: https://habr.com/ru/post/id473972/


All Articles