2017年のC ++プロジェクトのトップ10エラー

ほぼ3か月間、2018年はすでに窓の外にありました。つまり、過去1年間にC ++プロジェクトでPVS-Studioアナライザーによって検出された上位10個のエラーをコンパイルするときが来ました（少し遅れています）。 それでは始めましょう！

ご注意 さらに興味がある場合は、まずコードスニペットでエラーを見つけてから、アナライザーの警告と説明を読んでください。 もっと面白いと思います。

10位

ソース： メモ帳++：5年後のコードチェック

最も有名なテキストエディタの1つであるNotepad ++のチェック中にエラーが発見されました。

エラーを含むコードスニペット：

TCHAR GetASCII(WPARAM wParam, LPARAM lParam) { int returnvalue; TCHAR mbuffer[100]; int result; BYTE keys[256]; WORD dwReturnedValue; GetKeyboardState(keys); result = ToAscii(static_cast<UINT>(wParam), (lParam >> 16) && 0xff, keys, &dwReturnedValue, 0); returnvalue = (TCHAR) dwReturnedValue; if(returnvalue < 0){returnvalue = 0;} wsprintf(mbuffer, TEXT("return value = %d"), returnvalue); if(result!=1){returnvalue = 0;} return (TCHAR)returnvalue; } 

PVS-Studio警告 ： V560条件式の一部は常に真です：0xff。 babygrid.cpp 711

アナライザーは式（lParam >> 16）&& 0xffが疑わしいと判断しました 。 ToAscii関数に渡される2番目の引数の値は常に0または1であり、結果の値は左の部分式- （lParam >> 16）のみに依存します。 明らかに、&&演算子の代わりに＆演算子を使用する必要がありました。

9位

出典： Yandexの開発者に挨拶を送ります

9位は、Yandexが開発したClickHouseプロジェクトのエラーです。

 bool executeForNullThenElse(....) { .... const ColumnUInt8 * cond_col = typeid_cast<const ColumnUInt8 *>(arg_cond.column.get()); .... if (cond_col) { .... } else if (cond_const_col) { .... } else throw Exception( "Illegal column " + cond_col->getName() + " of first argument of function " + getName() + ". Must be ColumnUInt8 or ColumnConstUInt8.", ErrorCodes::ILLEGAL_COLUMN); .... } 

PVS-Studio警告 ： V522ヌルポインター 'cond_col'の逆参照が行われる場合があります。 FunctionsConditional.h 765

このコードでは、例外をスローする必要があるときにエラー状況が誤って処理されます。 cond_colポインターに注意してください 。 そのために、 ifステートメントはポインターがゼロ以外であることをチェックします。 例外がスローされるelseブランチに制御が到達した場合、 cond_colポインターは正確にnullです。 ただし、例外メッセージを生成する場合、 cond_colは式cond_col-> getName（）で間接参照されます。

8位

出典： Firebird、MySQL、PostgreSQLのコード品質比較

8位は、Firebird、MySQL、PostgreSQLのコードの品質を比較したときに、MySQLプロジェクトで見つかったエラーの1つです。

エラーを含むメソッドのコード：

 mysqlx::XProtocol* active() { if (!active_connection) std::runtime_error("no active session"); return active_connection.get(); } 

PVS-Studio警告 ： V596オブジェクトは作成されましたが、使用されていません。 「throw」キーワードが欠落している可能性があります：throw runtime_error（FOO）; mysqlxtest.cc 509

アクティブな接続（ ！Active_connection ）がない場合は、 std :: runtime_errorタイプの例外オブジェクトが作成されます...それだけです。 作成後、メソッドは単に削除されますが、メソッドの実行は継続されます。 明らかに、開発者は例外をスローするためにthrowキーワードを忘れていました。

7位

ソース： FreeBSDコードの56の潜在的な脆弱性を一晩で見つける方法

夕方に56の潜在的な脆弱性を見つける方法？ もちろん、静的解析では！

FreeBSDコードで見つかった問題の1つ：

 int mlx5_core_create_qp(struct mlx5_core_dev *dev, struct mlx5_core_qp *qp, struct mlx5_create_qp_mbox_in *in, int inlen) { .... struct mlx5_destroy_qp_mbox_out dout; .... err_cmd: memset(&din, 0, sizeof(din)); memset(&dout, 0, sizeof(dout)); din.hdr.opcode = cpu_to_be16(MLX5_CMD_OP_DESTROY_QP); din.qpn = cpu_to_be32(qp->qpn); mlx5_cmd_exec(dev, &din, sizeof(din), &out, sizeof(dout)); return err; } 

PVS-Studio警告 ： V597コンパイラは、「dout」オブジェクトのフラッシュに使用される「memset」関数呼び出しを削除できました。 プライベートデータを消去するには、memset_s（）関数を使用する必要があります。 mlx5_qp.c 159

式memset（＆dout、0、sizeof（dout））に注意してください。 開発者は、 doutに対応するメモリブロック内のデータを「消去」して、値をゼロに設定したいと考えていました。 通常、このアプローチは、メモリに「ハング」しないようにプライベートデータをクリアする必要がある場合に使用されます。

ただし、 doutはそれ以上使用されません（ sizeof（dout）はカウントされません）。これにより、コンパイラは上記のmemset関数呼び出しを削除できます。 このような最適化は、C / C ++の観点から見たプログラムの動作には影響しません。 その結果、クリアされるべきデータがメモリに残る場合があります。

このトピックをさらに深く掘り下げるには、次の記事を読むことをお勧めします。

• プライベートデータを安全に消去します 。
• 診断ルールV597のドキュメント 。
• C / C ++の世界で最も危険な関数 。

6位

出典： 待望のCryEngine Vテスト

このトップで取り上げるコード-CryEngineV。

 int CTriMesh::Slice(....) { .... bop_meshupdate *pmd = new bop_meshupdate, *pmd0; pmd->pMesh[0]=pmd->pMesh[1] = this; AddRef();AddRef(); for(pmd0=m_pMeshUpdate; pmd0->next; pmd0=pmd0->next); pmd0->next = pmd; .... } 

PVS-Studio警告 ： V529奇数セミコロン ';' 「for」演算子の後。 boolean3d.cpp 1314

このフラグメントがこのように書き出されていない場合-短縮され、コードの残りの部分から分離されている場合、アナライザーが見つけた疑わしいセクション-forループを終了するシンボル ';'を見つけるのはそれほど簡単ではないことに同意します。 同時に、コードをフォーマットする（次の式をシフトする）ことも、シンボル ';'を示唆しています。 ここは不要です。式pmd0-> next = pmd; サイクルの本体でなければなりません。 しかし、 forループのロジックから判断すると、混乱を招くのはコードの誤ったフォーマットであり、論理的なエラーではありません。 ちなみに、CryEngineコードでは、コードのフォーマットが修正されています。

5位

出典： Unreal Engine開発プロセスの一部としての静的分析。

Unreal EngineゲームエンジンのコードでPVS-Studioが検出したエラーの修正作業中に、次のエラーが発見されました。

 for(int i = 0; i < SelectedObjects.Num(); ++i) { UObject* Obj = SelectedObjects[0].Get(); EdObj = Cast<UEditorSkeletonNotifyObj>(Obj); if(EdObj) { break; } } 

PVS-Studio警告 ： V767ループ内の定数インデックスによる「SelectedObjects」配列の要素への疑わしいアクセス。 skeletonnotifydetails.cpp 38

ループでは、すべての要素を調べて、その中でUEditorSkeletonNotifyObj型の最初の要素を見つけたいと考えました。 しかし、ループカウンターiの代わりにSelectedObjects [0] .Get（）式で定数インデックス0を使用することにより、不幸なミスを犯しました。 その結果、最初の要素のみが常にチェックされます。

4位

出典： Tizenオペレーティングシステムの27,000エラー

Tizenオペレーティングシステムとその中で使用されているサードパーティのコンポーネントをチェックしているときにエラーが見つかりました。 この記事は大きく、多くの興味深いエラー例が含まれています-読むことを強くお勧めします。

ただし、特定の警告に戻ります。

 int _read_request_body(http_transaction_h http_transaction, char **body) { .... *body = realloc(*body, new_len + 1); .... memcpy(*body + curr_len, ptr, body_size); body[new_len] = '\0'; curr_len = new_len; .... } 

PVS-Studio警告 ： V527 「\ 0」値が「char」型ポインターに割り当てられているのは奇妙です。 おそらく意味：* body [new_len] = '\ 0'。 http_request.c 370

エラーは式本体[new_len] = '\ 0'にあります。 bodyパラメーターはそれぞれchar **型であり、式body [new_len]の型はchar *であることに注意してください。 しかし、開発者は大失敗し、別の逆参照を忘れて、ポインターに値「\ 0」を書き込もうとしました（nullポインターに変換されます）。

これは2つの問題につながります。

• どこか（ body [new_len] ）nullポインターが書き込まれます。
• 終端のゼロは行末に書き込まれません。

正しいコードは次のとおりです。

 (*body)[new_len] = '\0'; 

3位

出典： PVS-Studioは脆弱性の検索にどのように役立ちますか？

だから私たちはトップ3に到達しました。 次のコードは、「PVS-StudioはCVE検索にどのように対処しますか？」という質問への回答を検索中に見つかりました（回答については、上記の記事を参照してください）。 illumos-gateプロジェクトのコード。

 static int devzvol_readdir(....) { .... char *ptr; .... ptr = strchr(ptr + 1, '/') + 1; rw_exit(&sdvp->sdev_contents); sdev_iter_datasets(dvp, ZFS_IOC_DATASET_LIST_NEXT, ptr); .... } 

PVS-Studio 警告 ： V769 「strchr（ptr + 1、」/「）+ 1」式の「strchr（ptr + 1、」/「）」ポインターはnullptrである可能性があります。 そのような場合、結果の値は無意味になり、使用しないでください。

strchr関数は、最初の引数で指定された文字列内の2番目の引数で指定された文字の最初の出現を指すポインターを返します。 そのような文字が見つからない場合、 strchrはNULLを返します 。 ただし、この事実は考慮されず、値「1」が常に戻り値に追加されます。 その結果、 ptrポインターは常にゼロ以外になります。つまり、 ptr！= NULLという形式をさらにチェックしても、ポインターの有効性に関する情報は得られません。 その結果、特定の条件下で、このコードによりカーネルパニックが発生しました。

識別子CVE-2014-9491はこのエラーに割り当てられました：illumosのdevzvol_readdir関数はstrchr呼び出しの戻り値をチェックしません。これにより、リモートの攻撃者が不特定のベクトルを介してサービス拒否（ NULLポインターの参照解除とパニック）を引き起こすことができます 。

CVE自体は2014年に発見されたという事実にもかかわらず、私たち自身の研究の過程で、2017年にこのエラーを発見したため、このトップになりました。

2位

出典： Unreal Engine開発プロセスの一部としての静的分析。

2番目にあるエラーが発見されました...はい、再びUnreal Engineで発見されました。 とても面白かったので、抵抗できず、書きませんでした。

ご注意 実際、アンリアルエンジンに関する上記の記事からさらに2、3のエラーを記述しますが、それでも同じプロジェクトにあまり頻繁にアクセスしたくありません。 したがって、上記の記事、特に警告V714およびV709を自分で確認することを強くお勧めします。

その後、多くのコードがありますが、問題の本質を理解する必要があります。

 bool FCreateBPTemplateProjectAutomationTests::RunTest( const FString& Parameters) { TSharedPtr<SNewProjectWizard> NewProjectWizard; NewProjectWizard = SNew(SNewProjectWizard); TMap<FName, TArray<TSharedPtr<FTemplateItem>> >& Templates = NewProjectWizard->FindTemplateProjects(); int32 OutMatchedProjectsDesk = 0; int32 OutCreatedProjectsDesk = 0; GameProjectAutomationUtils::CreateProjectSet(Templates, EHardwareClass::Desktop, EGraphicsPreset::Maximum, EContentSourceCategory::BlueprintFeature, false, OutMatchedProjectsDesk, OutCreatedProjectsDesk); int32 OutMatchedProjectsMob = 0; int32 OutCreatedProjectsMob = 0; GameProjectAutomationUtils::CreateProjectSet(Templates, EHardwareClass::Mobile, EGraphicsPreset::Maximum, EContentSourceCategory::BlueprintFeature, false, OutMatchedProjectsMob, OutCreatedProjectsMob); return ( OutMatchedProjectsDesk == OutCreatedProjectsDesk ) && ( OutMatchedProjectsMob == OutCreatedProjectsMob ); } 

問題を理解するために必要な次の重要な点に注意してください。 変数OutMatchedProjectsDesk 、 OutCreatedProjectsDesk 、およびOutMatchedProjectsMob 、 OutCreatedProjectsMobは、宣言時にゼロで初期化され、 CreateProjectSetメソッドに引数として渡されます。

その後、ステートメントステートメントreturnで変数が比較されます。 したがって、 CreateProjectSetメソッドは最後の2つの引数を初期化する必要があります。

それでは、エラーを含むCreateProjectSetメソッドを見てみましょう。

 static void CreateProjectSet(.... int32 OutCreatedProjects, int32 OutMatchedProjects) { .... OutCreatedProjects = 0; OutMatchedProjects = 0; .... OutMatchedProjects++; .... OutCreatedProjects++; .... } 

PVS-Studioの警告 ：

• V763パラメーター 'OutCreatedProjects'は、使用される前に常に関数本体で書き換えられます。 gameprojectautomationtests.cpp 88
• V763パラメーター 'OutMatchedProjects'は、使用される前に常に関数本体で書き換えられます。 gameprojectautomationtests.cpp 89

パラメーターOutCreatedProjectsおよびOutMatchedProjectsは参照を作成するのを忘れ、その結果、対応する引数の値が単純にコピーされます。 結果として、上記のRunTestメソッドの戻り値は常にtrueです 。これは、比較されるすべての変数が初期化中に指定された同じ値-0を持っているためです。

正しいコードは次のとおりです。

 static void CreateProjectSet(.... int32 &OutCreatedProjects, int32 &OutMatchedProjects) 

一位

ソース： 静的コード分析が大好き！

この間違いを見た途端、誰がトップをリードすべきかについて疑う余地はありませんでした。 一般的に、自分で見てください。 上記のコードスニペットでエラーが見つかるまで、問題の説明に決してアクセスしないでください。 ところで、プロジェクト-StarEngine-は再びゲームエンジンです。

 PUGI__FN bool set_value_convert( char_t*& dest, uintptr_t& header, uintptr_t header_mask, int value) { char buf[128]; sprintf(buf, "%d", value); return set_value_buffer(dest, header, header_mask, buf); } 

さて、間違いを見つける成功はどうですか？ :)

PVS-Studio警告 ： V614未初期化バッファー 'buf'が使用されました。 'printf'関数の最初の実引数を確認することを検討してください。 pugixml.cpp 3362

確かに質問があります：「 printf ？コードでsprintf関数の呼び出しのみがある場合、 printfアナライザーの警告はどこから来ますか？」

それが本質です！ sprintfは（！） std :: printfに展開されるマクロです！

 #define sprintf std::printf 

その結果、初期化されていないbufバッファーがフォーマット文字列として使用されます。 すごいですね。 この間違いは、当然のことながら、1位になったと思います。

マクロ宣言を使用してヘッダーファイルにリンクします 。

おわりに

収集されたバグをお楽しみください。 個人的には、彼らは私にとって十分に興味深いようでした。 しかし、もちろん、あなたのビジョンは私のビジョンとは異なる可能性があります。そのため、 ブログの記事を読むか、オープンソースプロジェクトでPVS-Studioが検出したエラーのリストを見て、「トップ10」を編集できます。

また、記事に記載されているすべてのエラー（ および他の多くのエラー）はPVS-Studioアナライザーを使用して検出されたことを思い出してください。プロジェクトで試してみることをお勧めします： ダウンロードページへのリンク 。

この記事を英語圏の聴衆と共有したい場合は、翻訳へのリンクを使用してください：セルゲイヴァシリエフ。 2017年のC ++プロジェクトのバグのトップ10