Clever Geek Handbook

ビットコインチェック

ビットコイン、PVS-Studio

この記事には何もありません。 PVS-Studioを使用して、ビットコインのソースコードを確認しました。 疑わしい場所をいくつか見つけました。 これは驚くことではありません。 怠け者だけがこれらのソースコードをチェックしなかったと思います。 しかし、一度確認したら、小さなメモを書くことにしました。 つまり、「ショーのために」。



すべては、オープンソースプロジェクトでPVS-StudioとClangを比較しているという考えから始まりました。 タスクは大きく、困難であり、私たちはすぐにそれをしないと思います。 複雑さは、次の点で表されます。

  1. GCCを使用してビルドされたプロジェクトを見つける必要がありますが、Clangを使用してビルドできます。 すでにClang指向のプロジェクトをチェックすると、不正直になります。 エラーはすでに修正されているため、Clangは当然それらの中に何も見つけません。 そして、PVS-Studioはそれを見つけます。
  2. PVS-Studioアナライザーは、「Linux」と呼ばれる外部フィールドでプレイする必要があります。 ClangとVisual Studioを使用して同時にビルドできるプロジェクトはほとんどありません。 理論的には、Clangは既にVisual Studioと十分に互換性があります。 実際には、これは真実ではありません。 多くのプロジェクトを収集して検証することはできません。 一方、PVS-StudioはLinuxプロジェクトのチェックが不十分です。 その結果、両方のツールが同等に機能するプロジェクトを選択する必要があります。


比較のために選択されたプロジェクトの1つはビットコインでした。 どちらのアナライザーもほとんど何も見つかりませんでした。 これは驚くことではありません。 このプロジェクトはすでに多くのツールでテストされていると思います。 したがって、このプロジェクトは比較されない可能性が最も高いです。 したがって、少なくともこの短いメモは残しておきます。



プロジェクト分析



ビットコインが必要ではないものを説明してください。 ソースコードは次を使用して取得されます。



git clone https://github.com/bitcoin/bitcoin.git



分析は、 PVS-Studioバージョン5.17を使用して実行されました。



奇妙なサイクル



私の意見では、興味深い場所は1つだけでした。 これは、暗号に関連するある種の機能です。 彼女は何をしているのか分かりません 多分、EPIC FAILを見つけました。 現在、セキュリティに関連する壮大なバグを見つけることが流行しています。 ただし、ほとんどの場合、これは意図された軽微な間違いであるか、まったくありません。 

bool CCryptoKeyStore::Unlock(const CKeyingMaterial& vMasterKeyIn) { { LOCK(cs_KeyStore); if (!SetCrypted()) return false; CryptedKeyMap::const_iterator mi = mapCryptedKeys.begin(); for (; mi != mapCryptedKeys.end(); ++mi) { const CPubKey &vchPubKey = (*mi).second.first; const std::vector<unsigned char> &vchCryptedSecret = (*mi).second.second; CKeyingMaterial vchSecret; if(!DecryptSecret(vMasterKeyIn, vchCryptedSecret, vchPubKey.GetHash(), vchSecret)) return false; if (vchSecret.size() != 32) return false; CKey key; key.Set(vchSecret.begin(), vchSecret.end(), vchPubKey.IsCompressed()); if (key.GetPubKey() == vchPubKey) break; return false; } vMasterKey = vMasterKeyIn; } NotifyStatusChanged(this); return true; }


PVS-Studio警告：V612ループ内の無条件の「戻り」。 crypter.cpp 169



ループに注意してください。 いくつかのキーをその中でソートする必要があります。 ただし、ループの本体は1回だけ実行されます。 ループの最後に「return false;」ステートメントがあります。 サイクルは、「break;」ステートメントを使用して途中で中断することもできます。 同時に、ループ本体には単一の「continue;」演算子はありません。



疑わしいシフト 



 static int64_t set_vch(const std::vector<unsigned char>& vch) { if (vch.empty()) return 0; int64_t result = 0; for (size_t i = 0; i != vch.size(); ++i) result |= static_cast<int64_t>(vch[i]) << 8*i; // If the input vector's most significant byte is 0x80, // remove it from the result's msb and return a negative. if (vch.back() & 0x80) return -(result & ~(0x80 << (8 * (vch.size() - 1)))); return result; }


PVS-Studio警告：V629「0x80 <<（8 *（vch.size（）-1））」式の検査を検討してください。 32ビット値のビットシフトと、それに続く64ビットタイプへの拡張。 script.h 169



この関数は、64ビット値を形成します。 1つのシフトは正しく行われ、2番目のシフトは正しく行われない場合があります。



正しく： 

 static_cast<int64_t>(vch[i]) << 8*i


最初に、変数はint64_tに展開され、その後でのみシフトが発生します。



疑わしい： 

 0x80 << (8 * (vch.size() - 1))


定数0x80は 'int'型です。 これは、シフト中にオーバーフローが発生する可能性があることを意味します。 この関数は64ビット値を生成するため、エラーが疑われます。 「 フォードを知らない、水に入らない-パート3 」の記事で、シフトについて詳しく説明しました。



安全なオプション： 

 0x80ull << (8 * (vch.size() - 1))


危険なクラス 



 class CKey { .... // Copy constructor. This is necessary because of memlocking. CKey(const CKey &secret) : fValid(secret.fValid), fCompressed(secret.fCompressed) { LockObject(vch); memcpy(vch, secret.vch, sizeof(vch)); } .... };


PVS-Studio警告：V690「CKey」クラスはコピーコンストラクターを実装しますが、「=」演算子がありません。 そのようなクラスを使用するのは危険です。 key.h 175



コメントテキストからわかるように、コピーコンストラクターは同期に必要です。 ただし、コピーコンストラクターを使用するだけでなく、演算子=を使用してオブジェクトをコピーすることもできます。 ただし、この演算子は実装されていません。 operator =が現在どこでも使用されていなくても、潜在的に危険です。



同様に、さらにいくつかのクラスに注意する価値があります。

おわりに



静的アナライザーを定期的に使用すると、膨大な時間と神経細胞を節約できます。 主なことは、それが便利であることです。 たとえば、PVS-Studioでインクリメンタル分析を試してください。 あなたはコードを書くだけで、それが起こったら彼らはあなたを止めます。 あなたはすぐに良いものに慣れます。



この記事は英語です。



この記事を英語圏の聴衆と共有したい場合は、翻訳へのリンクを使用してください：Andrey Karpov。 http://www.viva64.com/en/b/0268/



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



More articles:


All Articles