Chromium：不正確なデータの使用

Chromiumプロジェクトで見つかったエラーの例について、高品質のコードを作成するための推奨事項に関する一連の記事をご紹介します。 これは5番目の部分で、未検証または誤って検証されたデータの使用におけるエラーに専念します。 未検証のデータを使用しているため、非常に多くの脆弱性が存在し、このトピックは興味深く関連性があります。



実際、この脆弱性の原因は、通常のタイプミスであっても、ほとんどすべてのタイプのエラーである可能性があります。 実際、見つかったエラーがCommon Weakness Enumerationに従って分類されている場合、潜在的な脆弱性を意味しています。



バージョン6.21以降、PVS-Studioアナライザーは、Common Weakness Enumerationに従って検出されたエラーを分類し、対応するCWE IDを割り当てることを学習しました。



おそらく読者は、以前の記事で警告番号Vxxxに加えて、CWE IDも提供していることにすでに気付いていたでしょう。 これは、前述のエラーが理論的に脆弱性を引き起こす可能性があることを意味します。 これの可能性は小さいですが、そうです。 興味深いことに、PVS-Studioによって発行されたほぼすべての警告と1つまたは別のCWE IDを一致させることができました。 つまり、自分で計画することなく、多数の弱点を検出できるアナライザーを作成したということです:)。



おわりに PVS-Studioアナライザーは、多くの種類の脆弱性を事前に防ぐのに役立ちます。 このトピックに関する出版物：「 PVS-Studioは脆弱性の検索にどのように役立ちますか？ 」。



この記事では、セキュリティの問題につながる可能性のあるエラーをまとめました。 エラーの選択は非常に条件付きで主観的であることを警告します。 ある種の脆弱性は、以前の記事の1つでささいなタイプミスと呼んだエラーとして偽装されていることがわかります。



それでは、Chromiumプロジェクト用にPVS-Studioによって発行されたレポートの解析中に気づいたセキュリティ上の欠陥を見てみましょう。 入門記事で書いたように、レポートを非常に流fluentに見たので、気づいていない他のエラーがあるかもしれません。 この記事の目的は、いくつかのエラーが、プログラムが不正確または未検証のデータの処理を開始するという事実にどのようにつながるかを示すことです。 そのようなデータに名前を付ける最良の方法はまだ決まっていないので、今のところは「偽データ」という用語を使用します。

エラーの例

Chromiumプロジェクト。

InstallUtil::ConditionalDeleteResult InstallUtil::DeleteRegistryValueIf(....) { .... ConditionalDeleteResult delete_result = NOT_FOUND; .... if (....) { LONG result = key.DeleteValue(value_name); if (result != ERROR_SUCCESS) { .... delete_result = DELETE_FAILED; } delete_result = DELETED; } return delete_result; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studio警告： V519 CWE-563 'delete_result'変数には連続して2回値が割り当てられます。 おそらくこれは間違いです。 行を確認してください：381、383。install_util.cc 383



関数は無効なステータスを返します。 その結果、プログラムの他の部分は、関数が特定の値を正常に削除したと想定します。 エラーは、ステータスDELETE_FAILEDが常にステータスDELETEDに置き換えられることです。



エラーは、 elseキーワードを追加することで修正できます。

 if (result != ERROR_SUCCESS) { .... delete_result = DELETE_FAILED; } else { delete_result = DELETED; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

おそらく、考慮されたエラーは、誤ったデータの本質をあまりよく反映していません。 この機能では、偽のデータの作成が行われ、検証や使用は行われません。 それでは、より適切な別のエラーを見てみましょう。



PDFiumライブラリ（Chromiumで使用）。

 CPVT_WordRange Intersect(const CPVT_WordRange& that) const { if (that.EndPos < BeginPos || that.BeginPos > EndPos || EndPos < that.BeginPos || BeginPos > that.EndPos) { return CPVT_WordRange(); } return CPVT_WordRange(std::max(BeginPos, that.BeginPos), std::min(EndPos, that.EndPos)); }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studioの警告：

• V501 CWE-570「||」の左側と右側に同一の副次式「that.BeginPos> EndPos」があります。 演算子。 cpvt_wordrange.h 46
• V501 CWE-570「||」の左側と右側に同一の副次式「that.EndPos <BeginPos」があります。 演算子。 cpvt_wordrange.h 46

条件のスペルが間違っています。 エラーに気付きやすくするために、条件を減らします。

 if (E2 < B1 || B2 > E1 || E1 < B2 || B1 > E2)
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

（E2 <B1）と（B1> E2）は同一であることに注意してください。 同様に（B2> E1）、これは（E1 <B2）と同じです。



必要なすべてのチェックが実行されるわけではなく、間違った範囲が生成される可能性があり、その結果、プログラムの機能に影響することがわかります。



次に、RE2正規表現ライブラリ（Chromiumで使用）の大きく複雑なコードを見てみましょう。 正直に言うと、ここで何が起こっているのかさえわかりませんが、コードに間違いなく異常なチェックがあります。



まず、いくつかの型がどのように宣言されているかを示します。 これが行われない場合、コードはあまり明確になりません。

 typedef signed int Rune; enum { UTFmax = 4, Runesync = 0x80, Runeself = 0x80, Runeerror = 0xFFFD, Runemax = 0x10FFFF, };
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

そして今、異常を持つ機能。

 char* utfrune(const char *s, Rune c) { long c1; Rune r; int n; if(c < Runesync) /* not part of utf sequence */ return strchr((char*)s, c); for(;;) { c1 = *(unsigned char*)s; if(c1 < Runeself) { /* one byte rune */ if(c1 == 0) return 0; if(c1 == c) // <= return (char*)s; s++; continue; } n = chartorune(&r, s); if(r == c) return (char*)s; s += n; } return 0; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studioアナライザーは、コメント「// <=」でメモした行に警告を生成します。 メッセージ： V547 CWE-570式 'c1 == c'は常にfalseです。 rune.cc 247



条件が常に偽である理由を理解してみましょう。 まず、次の行に注意してください。

 if(c < Runesync) return strchr((char*)s, c);
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

変数c <0x80の場合、関数は作業を停止します。 関数が処理を完了せずに続行する場合、変数c> = 0x80であることを確認できます。



次に、条件を見てみましょう。

 if(c1 < Runeself)
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

コメント「// <=」でマークされた条件（c1 == c）は 、 c1 <0x80の場合にのみ満たされます。



したがって、変数値について知っていることは次のとおりです。

• c> = 0x80
• c1 <0x80

したがって、条件c1 == cは常にfalseです。 しかし、これは非常に疑わしいです。 正規表現ライブラリのutfrune関数が計画どおりに機能しないことがわかりました。 このようなエラーの結果は予測できません。



LibVPXビデオコーデック（Chromiumで使用）。

 #define VP9_LEVELS 14 extern const Vp9LevelSpec vp9_level_defs[VP9_LEVELS]; typedef enum { .... LEVEL_MAX = 255 } VP9_LEVEL; static INLINE int log_tile_cols_from_picsize_level( uint32_t width, uint32_t height) { int i; const uint32_t pic_size = width * height; const uint32_t pic_breadth = VPXMAX(width, height); for (i = LEVEL_1; i < LEVEL_MAX; ++i) { if (vp9_level_defs[i].max_luma_picture_size >= pic_size && vp9_level_defs[i].max_luma_picture_breadth >= pic_breadth) { return get_msb(vp9_level_defs[i].max_col_tiles); } } return INT_MAX; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studioの警告：

• V557 CWE-119配列オーバーランが可能です。 「i」インデックスの値は254に達する可能性があります。vp9_encoder.h 931
• V557 CWE-119配列オーバーランが可能です。 「i」インデックスの値は254に達する可能性があります。vp9_encoder.h 932
• V557 CWE-119配列オーバーランが可能です。 「i」インデックスの値は254に達する可能性があります。vp9_encoder.h 933

vp9_level_defs配列は14個の要素で構成されています。 ループでは、配列のインデックスとして使用される変数iが0から254に変わります。結果：配列は境界を越えます。



まあ、このコードがアクセス違反につながる場合は。 しかし、実際には、ほとんどの場合、 vp9_level_defs配列の後にあるランダムデータの処理が開始されます。



SQLiteライブラリ（Chromiumで使用）で、配列外のデータを使用する別の同様のエラーが発生しました。



まず、 yy_shift_ofst配列には455個の要素が含まれていることに注意してください。

 static const short yy_shift_ofst[] = { /* 0 */ 355, 888, 1021, 909, 1063, 1063, 1063, 1063, 20, -19, .... /* 450 */ 1440, 1443, 1538, 1542, 1562, }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

2つのマクロも重要です。

 #define YY_SHIFT_COUNT (454) #define YY_MIN_REDUCE 993
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

YY_SHIFT_COUNTマクロは、 yy_shift_ofst配列の要素にアクセスするために使用できる最大インデックスを定義します。 要素の番号は0から始まるため、455ではなく454です。



マクロYY_MIN_REDUCEは993に等しく、 yy_shift_ofst配列のサイズとは関係ありません。



弱い検証を含む関​​数：

 static unsigned int yy_find_shift_action(....) { int i; int stateno = pParser->yytos->stateno; if( stateno>=YY_MIN_REDUCE ) return stateno; // <= assert( stateno <= YY_SHIFT_COUNT ); do { i = yy_shift_ofst[stateno]; // <= .... }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studio警告：V557 CWE-125アレイのオーバーランが発生する可能性があります。 'stateno'インデックスの値は992に達する可能性があります。sqlite3.c 138802



関数は、配列にアクセスするときのインデックスが特定の値を超えないように保護されています。 入力ミスのため、または別の理由で、間違った定数が使用されています。 定数454を使用する必要がありますが、代わりにインデックス値が993と比較されます。



その結果、海外のアレイにアクセスし、任意の偽データを読み取ることができます。



ご注意 以下に正しいassertを示しますが、リリースバージョンでは役に立ちません。



ほとんどの場合、チェックは次のように書き換える必要があります。

 if (stateno > YY_SHIFT_COUNT) { assert(false); return stateno; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

ICUプロジェクト（Chromiumで使用）。

 UVector* ZoneMeta::createMetazoneMappings(const UnicodeString &tzid) { UVector *mzMappings = NULL; .... if (U_SUCCESS(status)) { .... if (U_SUCCESS(status)) { .... while (ures_hasNext(rb)) { .... if (mzMappings == NULL) { mzMappings = new UVector( deleteOlsonToMetaMappingEntry, NULL, status); if (U_FAILURE(status)) { delete mzMappings; uprv_free(entry); break; } } .... } .... } } ures_close(rb); return mzMappings; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studio警告： V774 CWE-416メモリが解放された後、「mzMappings」ポインターが使用されました。 zonemeta.cpp 713



コードは複雑です。ここに本当の間違いがあるかどうかを確実に言うのは難しいと思います。 しかし、私が理解しているように、関数が既に解放されたメモリブロックへのポインタを返す場合、状況は可能です。 正しい無効なステータスハンドラは、ポインタを無効にする必要があります。

 if (U_FAILURE(status)) { delete mzMappings; mzMappings = nullptr; uprv_free(entry); break; }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

これで、関数が解放されたメモリへのポインタを返したことがわかりました。 このメモリは何でも構いません。この無効なポインタを使用すると、プログラムの動作が未定義になります。



Chromiumプロジェクトの次の機能は、負の値に対する保護を誤って実装しています。

 void AXPlatformNodeWin::HandleSpecialTextOffset(LONG* offset) { if (*offset == IA2_TEXT_OFFSET_LENGTH) { *offset = static_cast<LONG>(GetText().length()); } else if (*offset == IA2_TEXT_OFFSET_CARET) { int selection_start, selection_end; GetSelectionOffsets(&selection_start, &selection_end); if (selection_end < 0) *offset = 0; *offset = static_cast<LONG>(selection_end); } }
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

PVS-Studio警告：V519 CWE-563「*オフセット」変数には、連続して2回値が割り当てられます。 おそらくこれは間違いです。 行を確認してください：3543、3544。ax_platform_node_win.cc 3544



selection_end変数の値が負の場合、関数は0を返します 。 ただし、タイプミスにより、 0は正しい場所に書き込まれません。 正しいコードは次のようになります。

 if (selection_end < 0) selection_end = 0; *offset = static_cast<LONG>(selection_end);
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

このエラーにより、関数は負の数を返すことがありますが、そうではありません。 これは負の数であり、チェックを通じて「漏れる」可能性があり、不正確なデータがあります。

その他のバグ

正直に言うと、この記事の前のセクションで挙げた例はあまり好きではありません。 それらのいくつかはありますが、誤ったデータを使用する際のエラーの本質をあまりよく反映していません。 時間が経つにつれて、エラーのより鮮明な例を示し、さまざまなオープンプロジェクトからエラーを収集する別の記事を作成すると思います。



ちなみに、この記事にはもっと多くのエラーの例を含めることができますが、以前の記事を書いているときにすでにそれらを「使い切った」のですが、繰り返したいとは思いません。 たとえば、「Chromium：typos」という記事には、次のような断片がありました。

  if(!posX->hasDirtyContents() || !posY->hasDirtyContents() || !posZ->hasDirtyContents() || !negX->hasDirtyContents() || !negY->hasDirtyContents() || // <= !negY->hasDirtyContents()) // <=
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

このタイプミスのため、 negZポインターによって参照されるオブジェクトはチェックされません。 その結果、プログラムは誤ったデータを処理します。



また、この記事では、 malloc関数が返すポインターチェックがないために不正確な（破損した）データが発生する状況については考慮しませんでした。 malloc関数が NULLを 返す場合、これはNULLポインターの逆参照のエラーのみが可能なことを意味するものではありません。 もっと陰湿な状況があります。 概略的には、次のようになります。

 int *ptr = (int *)malloc(100 * sizeof(int)); ptr[1234567] = 42;
      
      

        
        
        
      

    
        
        
        
      
      

        
        
        
      

    
     

nullポインターの逆参照はありません。 ここではデータが記録されますが、どこでデータが破壊されたかは明確ではありません。



これは興味深い話であり、次の別の記事に専念します。

推奨事項

さまざまなエラーが発生すると、信頼性の低い（未検証、破損した）データが出現して使用されます。 ここに普遍的なアドバイスはありません。 もちろん、次のように記述できます。コードを間違えないでください！ しかし、そのようなアドバイスには意味がありません:)。



では、なぜこの記事を書いてこのタイプのエラーを強調したのですか？



あなたはそれらについて知っています。 問題の存在自体を知ることは、それを防ぐのに役立ちます。 誰かが問題について知らない場合、それはそこにないという意味ではありません。 この写真は良い例です：

まだアドバイスできること：

1. プロジェクトで使用されているライブラリを更新します。 新しいバージョンでは、脆弱性であるさまざまなエラーを修正できます。 ただし、この脆弱性は新しいバージョンだけでなく古いバージョンにも現れる可能性があることを認める必要があります。 それでも、より良い解決策はライブラリをアップグレードすることです。 新しい脆弱性よりも古い脆弱性について多くの人が知っています。
2. すべての入力、特に外部からの入力を慎重に確認してください。 たとえば、ネットワーク上のどこかから来るすべてのデータは、非常に慎重にチェックする必要があります。
3. さまざまなコード検証ツールを使用します。 たとえば、ChromiumプロジェクトにはPVS-Studio静的アナライザーの使用が明らかに欠けています:)。
4. 同僚に「 単純なコーディングエラーがそれほど恐れのないエラーではない 」ことを説明します。 チームが責任あるアプリケーションを開発する場合は、コードの品質に焦点を当て、無害に見えるエラーも含めてすべてを破壊する必要があります。

PVS-Studioに関する注意

前述したように、PVS-Studioアナライザーは、コードの記述段階でもエラーを検出することにより、すでに脆弱性を防ぐのに役立ちます。 しかし、私たちはさらに多くを望み、「未検証のデータを使用する」という概念をデータフロー分析に導入することで、PVS-Studioをまもなく真剣に改善します。



この重要な診断のために、V1010という特別な番号をすでに予約しています。 診断を使用すると、信頼性の低いソース（たとえば、ネットワーク経由で送信された）からデータを受信し、適切な検証なしで使用した場合のエラーを識別できます。 多くの場合、必要なすべての入力チェックの欠如がアプリケーションの脆弱性検出の原因です。 これについては、最近記事「 PVS-Studio 2018：CWE、Java、RPG、macOS、Keil、IAR、MISRA 」で詳しく説明しました（セクション「潜在的な脆弱性、CWE」を参照）。



新しい診断により、潜在的な脆弱性を特定するアナライザーが大幅に強化されます。 ほとんどの場合、V1010の診断は識別子CWE-20 （不適切な入力検証）に対応します。

おわりに

あなたとあなたの同僚に、私たちのウェブサイトの記事「 42の推奨事項 」を読むことを勧めます。 その後、プログラマはセキュリティの専門家になることはありませんが、多くの新しい有用な情報を学びます。 特に、これらの記事は、C言語またはC ++言語を習得したばかりで、ウサギの穴がどれだけ深く掘り下げられているかを疑わない開発者にとって有用です。



42のヒントを更新し、それらを50のヒントに変える予定です。 したがって、このTwitterのその他の興味深い記事を見逃さないように、Twitter @Code_AnalysisとRSSフィードを購読することをお勧めします。

この記事を英語圏の聴衆と共有したい場合は、翻訳へのリンクを使用してください：Andrey Karpov。 Chromium：信頼できないデータの使用 。