macOS用PVS-Studioリリース:Apple XNUカーネルの64の弱点

アップルのバグ PVS-Studio 6.23の新しいバージョンはmacOS上で実行され、CおよびC ++で記述されたプロジェクトをチェックできます。 私たちのチームは、このイベントのXNUカーネルチェックと一致することにしました。

macOS用PVS-Studio


macOS用のアナライザーバージョンのリリースにより、PVS-StudioはCおよびC ++のクロスプラットフォーム静的コードアナライザーと安全に呼ばれるようになりました。

最初は、Windows専用のバージョンがありました。 約2年前、私たちのチームはLinuxをサポートしていました。「LinuxでPVS-Studioを作成した方法 」。 また、私たちのブログの注意深い読者は、FreeBSDカーネルのチェックに関する記事( 第1記事第2記事 )を思い出してください 。 その後、アナライザーはPC-BSDおよびTrueOSで実行するように構築されました。 そして最後に、macOSに到達しました!

クロスプラットフォーム製品の開発は簡単ですか?

この問題には経済的および技術的な要素があります。

経済的な観点から、アナライザーをクロスプラットフォームにすることは正しい決断でした。 ソフトウェア開発は長い間この方向に向かっており、そのようなプロジェクトの開発者向けのツールは適切なはずです。 ただし、何か有用なものがあるとしても、すぐに始める価値があるという意味ではありません。 最初は、常に新しい方向に何かを実現するのに十分な力があることを確認し、それをサポートします。

技術的な面から見ると、プロジェクトがすぐにクロスプラットフォームとして考えられない限り、最初から困難です。 アナライザーをLinuxシステムに適応させるのに数ヶ月を費やしました。 新しいプラットフォーム用にプロジェクトをコンパイルするのに多くの時間はかかりませんでした。グラフィカルインターフェイスがなく、コードは実際にはシステムAPIの使用に関連付けられていません。 ほとんどの時間は、アナライザーを新しいコンパイラーに適合させ、分析の品質を向上させるために費やされました。 言い換えれば、多くの努力には検知の防止が必要です。

macOS開発はどうですか?

この時点で、CMake用のプロジェクトアナライザーファイルが既にあり、さまざまなオペレーティングシステムに簡単に適応できます。 さまざまなタイプのテストシステムもクロスプラットフォームでした。 これはすべて、macOSで非常に迅速に起動するのに役立ちました。

macOSのアナライザー開発機能は、Apple LLVM Compilerでした。 アナライザーはGCCを使用して構築され、完全に機能しましたが、今後もアナライザーとユーザーのコンピューターの互換性に影響を与える可能性があります。 潜在的なユーザーに問題を生じさせないために、Xcodeに付属しているこのコンパイラを使用した配布ビルドをサポートすることにしました。

C ++の開発は、クロスプラットフォームプロジェクトの作成と開発に大いに役立ちますが、さまざまなコンパイラがこのような機能を非常に不均等に追加するため、条件コンパイルはいくつかの場所で積極的に使用されています。

一般的に、すべてが簡単かつスムーズに進みました。 以前と同様に、ほとんどの時間は例外の確定、サイトの変更、テスト、その他の関連する問題に費やされました。 PVS-Studio for macOSでテストされた最初のプロジェクトとして、 XNU Kernel紹介します。

配布

こちらからmacOS用PVS-Studioをダウンロードしてインストールする方法を理解できます

XNUカーネル


このシステムのカーネルをチェックしない場合、macVS用PVS-Studioの機能のデモを開始する場所! したがって、アナライザーの新しいバージョンを使用してテストされた最初のプロジェクトはXNUカーネルでした。

XNUは、Appleが開発し、OS XファミリーのOS(macOS、iOS、tvOS、watchOS)で使用されるコンピューターオペレーティングシステムの中核です。 詳細

カーネルはCおよびC ++で記述されていると考えられていますが、実際はCです。1302* .cファイルと97 * .cppファイルのみをカウントしました。 コードベースのサイズは1929 KLOCです。 これは比較的小さなプロジェクトであることがわかりました。 比較のために、Chromiumプロジェクトのコードベースは15倍大きく、30 MLOCが含まれています。

ソースコードは、GitHub Webサイトのミラーxnuから簡単にダウンロードできます。

検証結果


XNU Kernelプロジェクトは比較的小さなものですが、アナライザーの警告だけを調べることは、多くの時間がかかる大きなタスクです。 アナライザーを事前に構成しなかったため、警告と誤報の調査が複雑になりました。 私の意見では、すぐに警告を実行し、興味のあるコードの断片を書きました。 これは、記事を書くのに十分であり、しかもサイズがしっかりしています。 PVS-Studioアナライザーは、多数の興味深いエラーを簡単に検出します。

XNUカーネル開発者への注意 。 できるだけ多くのエラーを見つける作業はありませんでした。 したがって、この記事を読んで修正することはできません。 まず、アラートをナビゲートする方法がないため、これは不便です。 PVS-Studioが生成できる形式の1つ、たとえば、ナビゲート機能を備えたHTMLレポート(Clangが生成できるものに似ています)を使用する方がはるかに優れていると確信しています。 第二に、レポートを表面的に研究したというだけの理由で、多くの間違いを逃しました。 開発者は、PVS-Studioを使用して、プロジェクトのより徹底的な分析を独自に実行することをお勧めします。

先ほど言ったように、誤検知は私を妨害しましたが、実際には問題ではありません。 アナライザーを構成する場合、誤検知の数を10-15%減らすことができます。 分析プロセスのセットアップと再起動にも時間がかかるため、この手順をスキップしました。記事なしで記事のエラーを収集するのは簡単です。 分析を慎重に実行することを計画している場合は、もちろん、時間をかけて構成する必要があります。

大部分の誤検知は、マクロと十分にマークされていない関数が原因で発生します。 たとえば、XNUカーネルでは、それらのほとんどはパニック機能の使用に関連付けられています。

これが、この関数の宣言方法です。

extern void panic(const char *string, ...) __attribute__((__format__ (__printf__, 1, 2))); 

この関数には注釈が付けられているため、入力引数はprintf関数の引数との類推によって解釈されます。 これにより、コンパイラーおよびアナライザーは、誤ったストリング形式のエラーを見つけることができます。 ただし、関数は制御を返さないとマークされていません。 その結果、次のコードは誤検知につながります。

 if (!ptr) panic("zzzzzz"); memcpy(ptr, src, n); 

ここで、アナライザーは、nullポインターを逆参照できるという警告を出します。 彼の観点から、 パニック関数を呼び出した後、 memcpy関数が呼び出されます。

このような誤検知を回避するには、 __attribute __((noreturn))を追加して関数の注釈を変更する必要があります。

 extern __attribute__((noreturn)) void panic(const char *string, ...) __attribute__((__format__ (__printf__, 1, 2))); 

ここで、XNU Kernelコードで何に興味を持ったかを見てみましょう。 合計で、64個のエラーを書き、この美しい数にこだわることにしました。 多くの人はこの分類に精通しているため、 Common Weakness Enumerationに従って欠陥をグループ化し、特定の章で説明されているエラーを理解しやすくなります。

CWE-570 / CWE-571:式は常にFalse / Trueです


非常に多様なエラーは、 CWE-570 / CWE-571につながる可能性があります。 条件または条件の一部が常にfalse / trueである状況。 XNU Kernelプロジェクトの場合、これらのエラーはすべて、私の意見では、タイプミスに関連しています。 PVS-Studioは通常、タイプミスを非常によく識別します。

フラグメントN1

 int key_parse( struct mbuf *m, struct socket *so) { .... if ((m->m_flags & M_PKTHDR) == 0 || m->m_pkthdr.len != m->m_pkthdr.len) { ipseclog((LOG_DEBUG, "key_parse: invalid message length.\n")); PFKEY_STAT_INCREMENT(pfkeystat.out_invlen); error = EINVAL; goto senderror; } .... } 

PVS-Studioの警告:V501 CWE-570 '!='演算子の左側と右側には、同一の部分式 'm-> M_dat.MH.MH_pkthdr.len'があります。 key.c 9442

タイプミスにより、クラスのメンバーはそれ自身と比較されます:

 m->m_pkthdr.len != m->m_pkthdr.len 

条件の一部は常にfalseであり、その結果、メッセージの長さが誤ってチェックされます。 プログラムが誤ったデータを処理し続けることが判明しました。 恐らくこれは恐ろしいことではありませんが、多くの脆弱性は、特定の入力データが未チェックであるか、十分な方法で検証されなかったことが原因です。 したがって、コード内のこの場所は、開発者の注目に値します。

フラグメントN2、N3

 #define VM_PURGABLE_STATE_MASK 3 kern_return_t memory_entry_purgeable_control_internal(...., int *state) { .... if ((control == VM_PURGABLE_SET_STATE || control == VM_PURGABLE_SET_STATE_FROM_KERNEL) && (((*state & ~(VM_PURGABLE_ALL_MASKS)) != 0) || ((*state & VM_PURGABLE_STATE_MASK) > VM_PURGABLE_STATE_MASK))) return(KERN_INVALID_ARGUMENT); .... } 

PVS-Studio警告:V560 CWE-570条件式の一部が常にfalseです:((* state&3)> 3)。 vm_user.c 3415

式のこの部分をさらに詳しく考えてみましょう。

 (*state & VM_PURGABLE_STATE_MASK) > VM_PURGABLE_STATE_MASK 

マクロ値を置換すると、次のようになります:

 (*state & 3) > 3 

ビット単位のAND演算の結果は、値0、1、または2のみになります。0、1、または2が3より大きいかどうかをチェックすることは意味がありません。

前のケースと同様に、特定のステータスが誤ってチェックされるため、誤った(誤った)データが処理される可能性があります。

vm_map.cファイルにもまったく同じエラーが見つかります。 どうやら、コードの一部はCopy-Pasteを使用して書かれたようです。 警告:V560 CWE-570条件式の一部は常にfalseです:((* state&3)> 3)。 vm_map.c 15809

フラグメントN4

 void pat_init(void) { boolean_t istate; uint64_t pat; if (!(cpuid_features() & CPUID_FEATURE_PAT)) return; istate = ml_set_interrupts_enabled(FALSE); pat = rdmsr64(MSR_IA32_CR_PAT); DBG("CPU%d PAT: was 0x%016llx\n", get_cpu_number(), pat); /* Change PA6 attribute field to WC if required */ if ((pat & ~(0x0FULL << 48)) != (0x01ULL << 48)) { mtrr_update_action(CACHE_CONTROL_PAT); } ml_set_interrupts_enabled(istate); } 

PVS-Studio警告:V547 CWE-571式は常に真です。 mtrr.c 692

タイプミスが侵入した可能性が高い無意味なチェックを見てみましょう。

 (pat & ~(0x0FULL << 48)) != (0x01ULL << 48) 

いくつかの式を計算します:
(pat&[0xFFF0FFFFFFFFFFFFFF])の値は0x0001000000000000になりません。 条件は常に真です。 その結果、 mtrr_update_action関数が常に呼び出されます。

フラグメントN5

今、私の意見では、非常に美しいタイプミスがあります: ==の代わりに= 書きました。

 typedef enum { CMODE_WK = 0, CMODE_LZ4 = 1, CMODE_HYB = 2, VM_COMPRESSOR_DEFAULT_CODEC = 3, CMODE_INVALID = 4 } vm_compressor_mode_t; void vm_compressor_algorithm_init(void) { .... assertf(((new_codec == VM_COMPRESSOR_DEFAULT_CODEC) || (new_codec == CMODE_WK) || (new_codec == CMODE_LZ4) || (new_codec = CMODE_HYB)), "Invalid VM compression codec: %u", new_codec); .... } 

PVS-Studio警告:V768 CWE-571式「new_codec = CMODE_HYB」は列挙型です。 ブール型の式として使用されるのは奇妙です。 vm_compressor_algorithms.c 419

条件をチェックするプロセスでは、変数new_codecに値2 割り当てられます。その結果、条件は常にtrueになり、アサートマクロは実際には何もチェックしません。

間違いは無害です。 まあ、アサートマクロは何かをチェックしませんでした-それは重要ではありません。 ただし、これに加えて、デバッグバージョンも正しく機能しません。 結局、 new_codec変数の値は損なわれ、必要なコーデックは使用されません。

フラグメントN6、N7

 void pbuf_copy_back(pbuf_t *pbuf, int off, int len, void *src) { VERIFY(off >= 0); VERIFY(len >= 0); VERIFY((u_int)(off + len) <= pbuf->pb_packet_len); if (pbuf->pb_type == PBUF_TYPE_MBUF) m_copyback(pbuf->pb_mbuf, off, len, src); else if (pbuf->pb_type == PBUF_TYPE_MBUF) { if (len) memcpy(&((uint8_t *)pbuf->pb_data)[off], src, len); } else panic("%s: bad pb_type: %d", __func__, pbuf->pb_type); } 

PVS-Studio警告:V517 CWE-570「if(A){...} else if(A){...}」パターンの使用が検出されました。 論理エラーが存在する可能性があります。 行を確認してください:340、343。pf_pbuf.c 340

明確にするために、本質を強調します。

 if (A) foo(); else if (A) Unreachable_code; else panic(); 

条件Aが真の場合、最初のifステートメントの本文真です。 そうでない場合は、再確認しても意味がなく、 パニック機能がすぐに呼び出されます。 通常、コードの一部は達成できません。

ここでは、ロジックの何らかの種類のエラー、またはいずれかの条件のタイプミスがあります。

同じファイル内のpbuf_copy_data関数は、 ほとんどの場合、Copy-Pasteを使用して記述されており、同じエラーが含まれています。 警告:V517 CWE-570「if(A){...} else if(A){...}」パターンの使用が検出されました。 論理エラーが存在する可能性があります。 行を確認してください:358、361。pf_pbuf.c 358

CWE-670:常に正しくない制御フローの実装


CWE-670の欠陥は、おそらくコード内で、プログラマが意図したとおりに何かが機能しないことを示唆しています。

フラグメントN8、N9、N10

 static void in_ifaddr_free(struct ifaddr *ifa) { IFA_LOCK_ASSERT_HELD(ifa); if (ifa->ifa_refcnt != 0) { panic("%s: ifa %p bad ref cnt", __func__, ifa); /* NOTREACHED */ } if (!(ifa->ifa_debug & IFD_ALLOC)) { panic("%s: ifa %p cannot be freed", __func__, ifa); /* NOTREACHED */ } if (ifa->ifa_debug & IFD_DEBUG) { .... } 

PVS-Studio警告:V646 CWE-670アプリケーションのロジックの検査を検討してください。 「else」キーワードが欠落している可能性があります。 in.c 2010

おそらく、このコードにはエラーはありません。 ただし、この場所は非常に疑わしいようです。

 } if (!(ifa->ifa_debug & IFD_ALLOC)) { 

例外は、文章が受け入れられないことです。 新しい行からの場合は、書き込みを開始する方が論理的です。 コード作成者はこの場所を確認する必要があります。 おそらくelseキーワードがここにありません。コードは次のようになります。

 } else if (!(ifa->ifa_debug & IFD_ALLOC)) { 

または、改行を追加するだけで、このコードがアナライザーまたはこのコードに関係する同僚によって混同されないようにすることができます。

同様の不審な場所はここにあります:


フラグメントN11、N​​12、N13、N14

 int dup2(proc_t p, struct dup2_args *uap, int32_t *retval) { .... while ((fdp->fd_ofileflags[new] & UF_RESERVED) == UF_RESERVED) { fp_drop(p, old, fp, 1); procfdtbl_waitfd(p, new); #if DIAGNOSTIC proc_fdlock_assert(p, LCK_MTX_ASSERT_OWNED); #endif goto startover; } .... startover: .... } 

PVS-Studio警告:V612 CWE-670ループ内の無条件の「goto」。 kern_descrip.c 628

これは非常に奇妙なコードです。 whileステートメントの本体はgotoステートメントで終了することに注意してください 。 この場合、ループ文のどこでもcontinueステートメントは使用されません。 これは、サイクルの本体が複数回実行されないことを意味します。

それでもループを複数回実行しないのに、なぜループを作成する必要があったのですか? ifステートメントを使用する方が良いでしょう、そしてこれは問題を提起しません。 これは間違いであり、ループ内で何かが間違って記述されていると思います。 たとえば、 gotoステートメントの前に条件が欠落している可能性があります。

同様の「ワンタイム」サイクルがさらに3回発生します。

ヌルポインターの逆参照:CWE-476、CWE-628、CWE-690


nullポインターの逆参照が発生する可能性があるさまざまな理由があり、PVS-Studioアナライザーは状況に応じて異なるCWE-IDを割り当てる場合があります。
記事を書くとき、このタイプのすべてのエラーを1つのセクションに集めるのが妥当であると考えました。

フラグメントN15

複雑で優れた機能から始めます。 まず、関数netagent_send_error_responseを検討します。この関数では、 セッション引数で渡されたポインターが逆参照されます。

 static int netagent_send_error_response( struct netagent_session *session, u_int8_t message_type, u_int32_t message_id, u_int32_t error_code) { int error = 0; u_int8_t *response = NULL; size_t response_size = sizeof(struct netagent_message_header); MALLOC(response, u_int8_t *, response_size, M_NETAGENT, M_WAITOK); if (response == NULL) { return (ENOMEM); } (void)netagent_buffer_write_message_header(.....); if ((error = netagent_send_ctl_data(session->control_unit, (u_int8_t *)response, response_size))) { NETAGENTLOG0(LOG_ERR, "Failed to send response"); } FREE(response, M_NETAGENT); return (error); } 

セッションポインターは、事前の検証なしに、 session-> control_unit式で間接参照されることに注意してください。 NULLポインターが逆参照されるかどうかは、この関数に渡される実際の引数によって異なります。

ここで、上記のnetagent_send_error_response関数がnetagent_handle_unregister_message関数でどのように使用されるかを見てみましょう。

 static void netagent_handle_unregister_message( struct netagent_session *session, ....) #pragma unused(payload_length, packet, offset) u_int32_t response_error = NETAGENT_MESSAGE_ERROR_INTERNAL; if (session == NULL) { NETAGENTLOG0(LOG_ERR, "Failed to find session"); response_error = NETAGENT_MESSAGE_ERROR_INTERNAL; goto fail; } netagent_unregister_session_wrapper(session); netagent_send_success_response(session, .....); return; fail: netagent_send_error_response( session, NETAGENT_MESSAGE_TYPE_UNREGISTER, message_id, response_error); } 

PVS-Studio警告:V522 CWE-628ヌルポインター「セッション」の逆参照が行われる場合があります。 ヌルポインターは 'netagent_send_error_response'関数に渡されます。 最初の引数を調べます。 行を確認してください:427、972。network_agent.c 427

ここでは、PVS-Studioに実装されたデータフロー分析が現れます。 アナライザーは、 セッションポインターがNULLである場合、ログに何らかの情報が書き込まれ、その後、 失敗ラベルへの遷移が発生することを観察します。

以下は、 netagent_send_error_response関数の呼び出しです。

 fail: netagent_send_error_response( session, NETAGENT_MESSAGE_TYPE_UNREGISTER, message_id, response_error); 

NULLである不運なセッションポインターは、実際の引数として関数に渡されることに注意してください。

知っているように、 netagent_send_error_response関数は、この場合の保護はなく、nullポインターが逆参照されます。

フラグメントN16

次の状況は前の状況と似ています。 関数コードは短くなりますが、ゆっくりと詳細に対処する必要があります。

 void * pf_lazy_makewritable(struct pf_pdesc *pd, pbuf_t *pbuf, int len) { void *p; if (pd->lmw < 0) return (NULL); VERIFY(pbuf == pd->mp); p = pbuf->pb_data; if (len > pd->lmw) { .... } 

最初にNULLをチェックせずにpbufポインター逆参照されることに注意してください。 コードには「VERIFY(pbuf == pd-> mp)」というチェックがあります。 ただし、 pd-> mpNULLになる場合があるため、検証はNULLに対する保護とは見なされません。

ご注意 私はXNUカーネルコードに完全に不慣れであり、間違っている可能性があることに注意してください。 おそらく、 pd-> mpNULL値を格納することはありません。 それから私の推論はすべて間違っており、ここに間違いはありません。 それでも、そのようなコードはもう一度確認する方が良いでしょう。

続けて、考慮されるpf_lazy_makewritable関数がどのように使用されるかを見てみましょう

 static int pf_test_state_icmp(....) { .... if (pf_lazy_makewritable(pd, NULL, off + sizeof (struct icmp6_hdr)) == NULL) return (PF_DROP); .... } 

PVS-Studio警告:V522 CWE-628ヌルポインター「pbuf」の逆参照が行われる場合があります。 NULLポインターは「pf_lazy_makewritable」関数に渡されます。 2番目の引数を調べます。 行を確認:349、7460。pf.c 349

2番目の実引数として、 pf_lazy_makewritable関数にNULLが渡されます。 これは非常に奇妙です。

プログラマーが、プログラムが「VERIFY(pbuf == pd-> mp)」をヌルポインターから保護すると考えているとします。 その後、疑問が生じます。なぜそのようなコードを書いたのですか? 明らかに無効な引数を渡して関数を呼び出すのはなぜですか?

したがって、実際には、 pf_lazy_makewritable関数はnullポインターを受け入れ、何らかの方法でこのケースを特別な方法で処理できるはずですが、そうではありません。 このコードはプログラマーによる最も徹底的な検証に値し、PVS-Studioアナライザーは間違いなく正しいので、注意を払っています。

フラグメントN17

少しリラックスできます。簡単なケースを考えてみましょう。

 typedef struct vnode * vnode_t; int cache_lookup_path(...., vnode_t dp, ....) { .... if (dp && (dp->v_flag & VISHARDLINK)) { break; } if ((dp->v_flag & VROOT) || dp == ndp->ni_rootdir || dp->v_parent == NULLVP) break; .... } 

PVS-Studio警告:V522 CWE-690潜在的なヌルポインター 'dp'の逆参照がある可能性があります。 vfs_cache.c 1449

チェックを見てください:

 if (dp && (dp->v_flag & VISHARDLINK)) 

dpポインターがnullである可能性があることがわかります。 ただし、事前検証なしでポインターが逆参照されます。

 if ((dp->v_flag & VROOT) || ....) 

フラグメントN18

前の例では、参照を解除する前にポインターがチェックされ、コード内でコードが忘れられるという状況に遭遇しました。 しかし、多くの場合、最初にポインターが逆参照されてからしかチェックされないという状況に遭遇する可能性があります。 XNU Kernelプロジェクトコードも例外ではありませんでした。 私たちが話していることをよりよく理解するために、合成例を考えてみましょう:

 p[n] = 1; if (!p) return false; 

次に、このようなエラーが実際にどのように見えるかを見てみましょう。 名前比較機能から始めましょう。 比較関数は非常に陰湿です :)。

 bool IORegistryEntry::compareName(....) const { const OSSymbol * sym = copyName(); bool isEqual; isEqual = sym->isEqualTo( name ); // <= if( isEqual && matched) { name->retain(); *matched = name; } if( sym) // <= sym->release(); return( isEqual ); } 

PVS-Studio警告:V595 CWE-476 nullptrに対して検証される前に、 'sym'ポインターが使用されました。 行を確認:889、896。IORegistryEntry.cpp 889

「// <=」という形式のコメントで、興味のあるコード行を強調表示しました。 ご覧のとおり、最初はポインターが逆参照されています。 さらにコードには、 nullptrポインターが等しいかどうかのチェックがあります。 しかし、ポインターがnullの場合、nullポインターが逆参照され、実際、関数はそのような状況に対応する準備ができていないことがすぐにわかります。

フラグメントN19

タイプミスにより、次のエラーが発生しました。

 static int memorystatus_get_priority_list( memorystatus_priority_entry_t **list_ptr, size_t *buffer_size, size_t *list_size, boolean_t size_only) { .... *list_ptr = (memorystatus_priority_entry_t*)kalloc(*list_size); if (!list_ptr) { return ENOMEM; } .... } 

PVS-Studio警告:V595 CWE-476 nullptrに対して検証される前に、 'list_ptr'ポインターが使用されました。 行を確認してください:7175、7176。kern_memorystatus.c 7175

アナライザーは、最初に変数が間接参照されていることを確認し、次の行でnullptrの等価性をチェックします。 プログラマが文字「*」を書くのを忘れたため、この興味深いエラーが発生しました。 実際、コードは次のようになっているはずです。

 *list_ptr = (memorystatus_priority_entry_t*)kalloc(*list_size); if (!*list_ptr) { return ENOMEM; } 

エラーは間接的に検出されたと言えます。 ただし、これは重要ではありません。最も重要なことは、アナライザーが異常なコードに注意を払い、エラーが発生したことです。

フラグメントN20-N35

XNUカーネルコードでは、V595診断を通じて多くのエラーが検出されます。 しかし、それらすべてを考えるのは退屈です。 したがって、もう1つだけケースを分析し、エラーを示すメッセージのリストを提供します。

 inline void inp_decr_sndbytes_unsent(struct socket *so, int32_t len) { struct inpcb *inp = (struct inpcb *)so->so_pcb; struct ifnet *ifp = inp->inp_last_outifp; if (so == NULL || !(so->so_snd.sb_flags & SB_SNDBYTE_CNT)) return; if (ifp != NULL) { if (ifp->if_sndbyte_unsent >= len) OSAddAtomic64(-len, &ifp->if_sndbyte_unsent); else ifp->if_sndbyte_unsent = 0; } } 

PVS-Studio警告:V595 CWE-476 nullsoに対して検証される前に「so」ポインターが使用されました。 行を確認してください:3450、3453。in_pcb.c 3450

soポインターの運命を独立して追跡し、コードが正しく記述されていないことを確認することを読者に提案します。

その他のエラー:


フラグメントN36、N37

そして、nullポインターの使用に関する最後のいくつかの間違い。

 static void feth_start(ifnet_t ifp) { .... if_fake_ref fakeif; .... if (fakeif != NULL) { peer = fakeif->iff_peer; flags = fakeif->iff_flags; } /* check for pending TX */ m = fakeif->iff_pending_tx_packet; .... } 

PVS-Studio警告:V1004 CWE-476 nullptrに対して検証された後、「fakeif」ポインターが安全に使用されませんでした。 行を確認してください:566、572。if_fake.c 572

このコードにコメントする必要はないと思います。 fakeifポインターがどのようにチェックされ、使用されるかを確認してください。

最後の同様のケース:V1004 CWE-476 nullptrに対して検証された後、「rt-> rt_ifp」ポインターが安全に使用されませんでした。 行を確認してください:138、140。netsrc.c 140

CWE-119:メモリバッファーの境界内での操作の不適切な制限


バッファの境界を越えることに関連するエラーがいくつかありました。 XNU Kernelのような重要なプロジェクトの非常に不快な間違い。

海外に行くためのさまざまなオプションは、さまざまなCWE IDによって分類できますが、この場合、アナライザーはCWE-119を選択しました。

フラグメントN38

最初に、いくつかのマクロがどのように宣言されているかを見てみましょう。

 #define IFNAMSIZ 16 #define IFXNAMSIZ (IFNAMSIZ + 8) #define MAX_ROUTE_RULE_INTERFACES 10 

次のことを覚えておくことが重要です。
次に、 snprintf関数とmemset関数を使用するときに、バッファーの境界を越えることができる関数を考えます。 つまり 2つのエラーが発生します。

 static inline const char * necp_get_result_description(....) { .... char interface_names[IFXNAMSIZ][MAX_ROUTE_RULE_INTERFACES]; .... for (index = 0; index < MAX_ROUTE_RULE_INTERFACES; index++) { if (route_rule->exception_if_indices[index] != 0) { ifnet_t interface = ifindex2ifnet[....]; snprintf(interface_names[index], IFXNAMSIZ, "%s%d", ifnet_name(interface), ifnet_unit(interface)); } else { memset(interface_names[index], 0, IFXNAMSIZ); } } .... } 

PVS-Studioの警告:
interface_namesの 2次元配列がどのように宣言されているかに注目してください。

 char interface_names[IFXNAMSIZ][MAX_ROUTE_RULE_INTERFACES]; //  : char interface_names[24][10]; 

ただし、この配列は次のように使用されます。

 char interface_names[MAX_ROUTE_RULE_INTERFACES][IFXNAMSIZ]; //  : char interface_names[10][24]; 

データから何らかの混乱が生じていることがわかります。

考えることなく、誰かが心配する必要はないと言うかもしれません。なぜなら、両方の配列が同じバイト数を占有しているからです。

いいえ、すべてが悪いです。 interface_names [10..23] [....]配列の要素は、ループ内のインデックス変数が値[0..9]をとるため、使用されません。 しかし、 interface_names [0..9] [....]の要素は、互いに「重複」し始めます。 つまり 一部のデータは他のデータを上書きします。

結果は完全なでたらめです。 配列の一部は初期化されずに残り、他の部分には既に記録されたデータの上にデータが書き込まれたときの混乱が含まれています。

フラグメントN39

同じファイルnecp_client.cの下には、非常によく似たエラーを含む別の関数があります

 #define IFNAMSIZ 16 #define IFXNAMSIZ (IFNAMSIZ + 8) #define NECP_MAX_PARSED_PARAMETERS 16 struct necp_client_parsed_parameters { .... char prohibited_interfaces[IFXNAMSIZ] [NECP_MAX_PARSED_PARAMETERS]; .... }; static int necp_client_parse_parameters(...., struct necp_client_parsed_parameters *parsed_parameters) { .... u_int32_t length = ....; .... if (length <= IFXNAMSIZ && length > 0) { memcpy(parsed_parameters->prohibited_interfaces[ num_prohibited_interfaces], value, length); parsed_parameters->prohibited_interfaces[ num_prohibited_interfaces][length - 1] = 0; .... } 

PVS-Studio警告:
すべて同じです。配列:

 char prohibited_interfaces[IFXNAMSIZ][NECP_MAX_PARSED_PARAMETERS]; 

次のように処理されます。

 char prohibited_interfaces[NECP_MAX_PARSED_PARAMETERS][IFXNAMSIZ]; 

CWE-563:使用しない変数への割り当て


PVS-Studioによって検出されたCWE-563の欠陥は、多くの場合、タイプミスの結果です。ここで、このような美しいタイプミスを検討します。

フラグメントN40

 uint32_t gss_krb5_3des_unwrap_mbuf(....) { .... for (cflag = 1; cflag >= 0; cflag--) { *minor = gss_krb5_3des_token_get( ctx, &itoken, wrap, &hash, &offset, &length, reverse); if (*minor == 0) break; wrap.Seal_Alg[0] = 0xff; wrap.Seal_Alg[0] = 0xff; } .... } 

PVS-Studio警告:V519 CWE-563 'wrap.Seal_Alg [0]'変数には値が連続して2回割り当てられます。 おそらくこれは間違いです。行を確認してください:2070、2071。gss_krb5_mech.c 2071

2回、値0xffを配列の同じ要素に書き込みます私は隣のコードを見て、本当にここに書きたかったという結論に達しました:

 wrap.Seal_Alg[0] = 0xff; wrap.Seal_Alg[1] = 0xff; 

関数の名前から判断すると、ネットワーク認証プロトコルに関連付けられています。そして、そのような失態...なんて恐ろしいことでしょう。ここで

PVS-Studioを購入できます当社のアナライザーは、このような多くのエラーの防止に役立ちます!

フラグメントN41、N42、N43、N44

 static struct mbuf * pf_reassemble(struct mbuf *m0, struct pf_fragment **frag, struct pf_frent *frent, int mff) { .... m->m_pkthdr.csum_flags &= ~CSUM_PARTIAL; m->m_pkthdr.csum_flags = CSUM_DATA_VALID | CSUM_PSEUDO_HDR | CSUM_IP_CHECKED | CSUM_IP_VALID; .... } 

PVS-Studio警告:V519 CWE-563「m-> M_dat.MH.MH_pkthdr.csum_flags」変数には、値が連続して2回割り当てられます。 おそらくこれは間違いです。行を確認してください:758、759。pf_norm.c 759

行:

 m->m_pkthdr.csum_flags &= ~CSUM_PARTIAL; 

実用的ではありません。次の行では、変数m-> m_pkthdr.csum_flagsに新しい値が割り当てられます。正しいコードが実際にどのように見えるかはわかりませんが、「|」文字を忘れてしまったと思います。私の謙虚な意見では、コードは次のようになります。

 m->m_pkthdr.csum_flags &= ~CSUM_PARTIAL; m->m_pkthdr.csum_flags |= CSUM_DATA_VALID | CSUM_PSEUDO_HDR | CSUM_IP_CHECKED | CSUM_IP_VALID; 

同様のエラーを示す警告がさらに3つあります。

CWE-14:コンパイラーがコードを削除してバッファーをクリアする


デバッグバージョンでは見えない、非常に油断のならない欠陥。読者がそれをまだよく知らない場合は、読み続ける前に、次のリンクを読むことをお勧めします。
  1. 個人データの安全な消去
  2. V597コンパイラは、「Foo」バッファをフラッシュするために使用される「memset」関数呼び出しを削除できます。RtlSecureZeroMemory()関数を使用して、プライベートデータを消去する必要があります。
  3. CWE-14:コンパイラがバッファをクリアするコードを削除。

読者がメモリに保存されているプラ​​イベートデータを上書きする理由を理解していない場合は、記事「メモリを上書きする-なぜ?をお勧めします

そのため、メモリ内のプライベートデータを上書きすることは重要ですが、コンパイラは対応するコードを削除することがあります。これは、その観点から見ると余分なためです。XNU Kernelの興味深いところを見てみましょう。

フラグメントN45

 __private_extern__ void YSHA1Final(unsigned char digest[20], YSHA1_CTX* context) { u_int32_t i, j; unsigned char finalcount[8]; .... /* Wipe variables */ i = j = 0; memset(context->buffer, 0, 64); memset(context->state, 0, 20); memset(context->count, 0, 8); memset(finalcount, 0, 8); // <= #ifdef SHA1HANDSOFF YSHA1Transform(context->state, context->buffer); #endif } 

PVS-Studio警告:V597 CWE-14コンパイラーは、「finset」バッファーをフラッシュするために使用される「memset」関数呼び出しを削除できます。プライベートデータを消去するには、memset_s()関数を使用する必要があります。 sha1mod.c 188

リリースバージョンを最適化するために、コンパイラは、コメント「// <=」で書き留めたコード行を削除する権利を持っています。ほぼ間違いなく彼はそうするでしょう。

フラグメントN46

 __private_extern__ void YSHA1Transform(u_int32_t state[5], const unsigned char buffer[64]) { u_int32_t a, b, c, d, e; .... state[0] += a; state[1] += b; state[2] += c; state[3] += d; state[4] += e; /* Wipe variables */ a = b = c = d = e = 0; } 

PVS-Studio警告:V1001 CWE-563「a」変数が割り当てられていますが、関数の最後まで使用されません。sha1mod.c 120

コンパイラには、変数が関数で使用されなくなったため、変数をリセットするコードを生成しない権利があります。

PVS-Studioアナライザーがこの疑わしい状況をCWE-563と解釈したことに注意してください実際、同じ欠陥が異なるCWEとして解釈されることがよくあり、この場合、アナライザーはCWE-563を選択しました。ただし、このコードの何が問題なのかをより正確に説明するため、このコードは特にCWE-14に起因すると判断しました。

CWE-783:演算子優先ロジックエラー


CWE-783の欠陥は、プログラマーが操作の優先順位を混同し、意図したとおりに動作しないコードを記述した場合に発生します。多くの場合、このようなエラーは不注意または括弧の欠落により発生します。

フラグメントN47

 int getxattr(....) { .... if ((error = copyinstr(uap->attrname, attrname, sizeof(attrname), &namelen) != 0)) { goto out; } .... out: .... return (error); } 

PVS-Studio警告:V593 CWE-783「A = B!= C」の表現を検討することを検討してください。式は次のように計算されます: 'A =(B!= C)'。vfs_syscalls.c 10574

古典的なエラー。私はさまざまなプログラムでそのようなエラーに遭遇します(証拠)。根本的な原因は、なんらかの理由で一部のプログラマーが1行に多く詰め込む傾向があることです。

その結果、次の代わりに:

 Status s = foo(); if (s == Error) return s; 

彼らは書く:

 Status s; if (s = foo() == Error) return s; 

また、コードにエラーが発生します。
その結果、returnステートメントError定数に等しい値ではなく、1の無効なエラーステータスを返します

私は定期的にそのようなコードを批判しており、一度に複数のアクションを1行にプッシュしないことをお勧めします。「プッシュ」はコードのサイズを大幅には削減しませんが、さまざまなエラーを引き起こします。これについては、私のミニブック「プログラミング、リファクタリング、その他すべての主な問題」で詳しく読むことを提案します章を参照してください。
XNU Kernelのコードに戻りましょう。エラーの場合、getxattr関数は、実際のエラーコードではなく、値1を返します。

フラグメントN48-N52

 static void memorystatus_init_snapshot_vmstats( memorystatus_jetsam_snapshot_t *snapshot) { kern_return_t kr = KERN_SUCCESS; mach_msg_type_number_t count = HOST_VM_INFO64_COUNT; vm_statistics64_data_t vm_stat; if ((kr = host_statistics64(.....) != KERN_SUCCESS)) { printf("memorystatus_init_jetsam_snapshot_stats: " "host_statistics64 failed with %d\n", kr); memset(&snapshot->stats, 0, sizeof(snapshot->stats)); } else { + .... } 

PVS-Studio警告:V593 CWE-783「A = B!= C」の表現を検討することを検討してください。式は次のように計算されます: 'A =(B!= C)'。 kern_memorystatus.c 4554 kr

変数に割り当てることができる値は2つのみです。0または1です。このため、printf関数host_statistics64関数によって返される実際のステータスではなく、常に数値1を出力します記事は大きいです。私は彼女だけでなく、読者も退屈させると思います。したがって、この記事で検討するフラグメントの数を減らします。他の同様の欠陥は考慮するのに面白くないです、そして、私はメッセージのリストに自分自身を制限します:






CWE-758: Reliance on Undefined, Unspecified, or Implementation-Defined Behavior


CまたはC ++プログラムで未定義または未指定の動作を取得する方法はたくさんあります。したがって、PVS-Studioアナライザーは、V567、V610、V611、V681、V704、V708、V726、V736などの問題を特定することを目的とした多くの診断を実装しています。

XNUの場合、アナライザーは、負の数値のシフトによって引き起こされる未定義の動作に関連するCWE-758の 2つの弱点のみを検出しました

フラグメントN53、N54

 static void pfr_prepare_network(union sockaddr_union *sa, int af, int net) { .... sa->sin.sin_addr.s_addr = net ? htonl(-1 << (32-net)) : 0; .... } 

PVS-Studio警告:V610 CWE-758未定義の動作。シフト演算子「<<」を確認してください。左のオペランド '-1'は負です。pf_table.c 976

負の数を左にシフトすると、未定義の動作が発生します。実際には、このコードはプログラマが期待するとおりに動作する可能性があります。それでも、このコードは正しくないため、修正する必要があります。これは次のように実行できます。

 htonl((unsigned)(-1) << (32-net)) 

PVS-Studioアナライザーは、ここで別の変化を見つけます:V610 CWE-758未定義の動作。シフト演算子「<<」を確認してください。左のオペランド '-1'は負です。pf_table.c 983

CWE-401:最後の参照を削除する前のメモリの不適切なリリース(「メモリリーク」)


XNU Kernelの開発者は、アナライザーがメモリリーク(CWE-401)の問題を見つけることができなかったという事実を賞賛すべきです。削除演算子がオブジェクト初期化エラーで呼び出されない場合、疑わしい場所は3つだけです。ただし、これが正確に間違いであるかどうかはわかりません。

フラグメントN55、N56、N57

 IOService * IODTPlatformExpert::createNub(IORegistryEntry * from) { IOService * nub; nub = new IOPlatformDevice; if (nub) { if( !nub->init( from, gIODTPlane )) { nub->free(); nub = 0; } } return (nub); } 

V773 CWE-401「nub」ポインターには、メモリーを解放せずに値が2回割り当てられました。メモリリークが発生する可能性があります。IOPlatformExpert.cpp 1287 init

関数がオブジェクトを初期化できない場合、メモリリークが発生する可能性があります。私の意見では、削除演算子はここありません。次のように書かれているはずです。

 if( !nub->init( from, gIODTPlane )) { nub->free(); delete nub; nub = 0; } 

私が正しいかどうかはわかりません。おそらく、free関数自体が、「delete * this;」という操作を実行してオブジェクトを破棄します。これらの警告に到達する頃にはすでに疲れていたので、私は注意深く理解していませんでした。

同様のアナライザー警告:

CWE-129:配列インデックスの不適切な検証


CWE-129の欠陥は、配列内の要素にインデックスを付けるために使用される変数が誤って、または不十分にチェックされていることを示しています。その結果、配列のオーバーフローが発生する可能性があります。

フラグメントN58-N61

 IOReturn IOStateReporter::updateChannelValues(int channel_index) { .... state_index = _currentStates[channel_index]; if (channel_index < 0 || channel_index > (_nElements - state_index) / _channelDimension) { result = kIOReturnOverrun; goto finish; } .... } 

PVS-Studio警告:V781 CWE-129「channel_index」変数の値は、使用後にチェックされます。おそらく、プログラムロジックに間違いがあります。行を確認してください:852、855。IOStateReporter.cpp 852

負の値に対する保護が誤って実装されています。最初に、要素は常に配列から取得され、インデックスが負でないことの検証のみが行われます。

このコードは次のように書き直すべきだと思います。

 IOReturn IOStateReporter::updateChannelValues(int channel_index) { .... if (channel_index < 0) { result = kIOReturnOverrun; goto finish; } state_index = _currentStates[channel_index]; if (channel_index > (_nElements - state_index) / _channelDimension) { result = kIOReturnOverrun; goto finish; } .... } 

おそらく、channel_indexの値が配列のサイズを超えないようにするチェックも追加する必要があります私はコードに詳しくないので、XNU Kernel開発者の裁量に任せます。

同様のエラー:


CWE-480:不適切な演算子の使用


CWE-480の欠陥は、通常、式の何らかのタイプミスに関連付けられています。原則として、それらはあまり多くありませんが、非常に面白いです。そのようなエラーを見て、どのようにそれらを作成できるのか疑問に思います。それでも、記事で繰り返し示したように、高度な資格を持つプログラマーでさえ、そのようなエラーから免れません。

フラグメントN62

 #define NFS_UC_QUEUE_SLEEPING 0x0001 static void nfsrv_uc_proxy(socket_t so, void *arg, int waitflag) { .... if (myqueue->ucq_flags | NFS_UC_QUEUE_SLEEPING) wakeup(myqueue); .... } 

PVS-Studio警告:V617 CWE-480状態の検査を検討してください。「|」の「0x0001」引数 ビット単位演算にゼロ以外の値が含まれています。nfs_upcall.c 331

一部のエンティティは、必要以上に頻繁に「起動」します。むしろ、状態に関係なく、常に「目覚め」ます。ほとんどの場合、次のように記述する必要があります。

 if (myqueue->ucq_flags & NFS_UC_QUEUE_SLEEPING) wakeup(myqueue); 


CWE-665:不適切な初期化


PVS-Studioアナライザーは、CWEに従って次のエラーを分類できませんでした。私の意見では、私たちはCWE-665を扱っています。

フラグメントN63

 extern void bzero(void *, size_t); static struct thread thread_template, init_thread; struct thread { .... struct thread_qos_override { struct thread_qos_override *override_next; uint32_t override_contended_resource_count; int16_t override_qos; int16_t override_resource_type; user_addr_t override_resource; } *overrides; .... }; void thread_bootstrap(void) { .... bzero(&thread_template.overrides, sizeof(thread_template.overrides)); .... } 

PVS-Studio警告:V568 'sizeof()'演算子がクラスへのポインターのサイズを評価するのは奇妙ですが、 'thread_template.overrides'クラスオブジェクトのサイズは評価しません。thread.c 377

ポインターを格納する変数のアドレスを取得し、bzero関数を使用してこの変数をリセットします実際、彼らはポインタにnullptr書き込んだだけです。

関数bzeroを使用することは、変数を無効にする非常に奇妙な不自然な方法です。書くのがずっと簡単でした:

 thread_template.overrides = NULL; 

ここから、バッファをリセットしたが、誤ってポインタをリセットしたいと結論付けました。つまり、正しいコードは次のようになっているはずです。

 bzero(thread_template.overrides, sizeof(*thread_template.overrides)); 

CWE-691:不十分な制御フロー管理


CWE-691は、命令シーケンスの異常を示します。別の異常も考えられます-コードの設計が動作方法と一致しません。これは、XNU Kernelコードで出会ったまさにそのケースです。

Fragment N64

Hooray、最後のコードを考慮する必要がありました!アナライザーによって発行されたレポートを見たときに気づかなかった他のエラーがあるかもしれませんが、できるだけ多くのエラーを特定するという目標がなかったことを思い出します。いずれにせよ、XNU Kernelの開発者はプロジェクトコードに精通しているため、私よりもレポートを研究することができます。それでは、美しい数字の64について考えてみましょうこの数字は、サイトviva64の名前と一致しています。

ご注意「viva64」の由来に興味のある方は、「PVS-Studioプロジェクトが10年前にどのように始まった」という記事を読むことをお勧めします

 void vm_page_release_startup(vm_page_t mem); void pmap_startup( vm_offset_t *startp, vm_offset_t *endp) { .... // -debug code remove if (2 == vm_himemory_mode) { for (i = 1; i <= pages_initialized; i++) { .... } } else // debug code remove- /* * Release pages in reverse order so that physical pages * initially get allocated in ascending addresses. This keeps * the devices (which must address physical memory) happy if * they require several consecutive pages. */ for (i = pages_initialized; i > 0; i--) { if(fill) fillPage(....); vm_page_release_startup(&vm_pages[i - 1]); } .... } 

PVS-Studio警告:V705 CWE-691「else」ブロックが忘れられたりコメントアウトされたりして、プログラムの動作ロジックが変更された可能性があります。vm_resident.c 1248

おそらくここにはエラーはありません。しかし、elseキーワードは本当に私を混乱させますコードは、ループが常に実行されているように見えるように設計されています。実際、条件(2 == vm_himemory_mode)がfalseの場合にのみ、ループが実行されます。

おわりに


macOSの世界で、新しい強力な静的コードアナライザーであるPVS-Studioが、CおよびC ++プログラムのエラーと潜在的な脆弱性を検出できます。私たちのアナライザーをそれぞれのプロジェクトで試して、その機能を評価してください。

ご清聴ありがとうございました。PVS-StudioがmacOSで利用可能になったという情報を同僚と共有することを忘れないでください。



この記事を英語圏の聴衆と共有したい場合は、翻訳へのリンクを使用してください:Andrey Karpov。 PVS-StudioがmacOSで利用可能になりました:XNUカーネルの64の弱点

記事を読んで質問がありますか?
多くの場合、記事には同じ質問が寄せられます。 ここで回答を収集しました: PVS-Studioバージョン2015に関する記事の読者からの質問への回答 。 リストをご覧ください。

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


All Articles