PVS-Studio for Java静的アナライザーによって検出されたエラーの新しいバッチを確認しますか？その後、記事の読書に参加してください！今回は、弾む城プロジェクトが検証の対象となりました。いつものように、最も興味深いコードスニペットが以下であなたを待っています。
PVS-Studioについて少し
PVS-Studioは、プログラムのソースコードのエラーと潜在的な脆弱性を特定するためのツールです。この記事の執筆時点では、静的分析は、C、C ++、C＃、およびJavaプログラミング言語で記述されたプログラムに対して実装されています。
Java用のアナライザーはPVS-Studioの最年少ラインです。それにもかかわらず、彼はコードの欠陥を見つける点で兄に劣っていません。これは、JavaアナライザーがC ++アナライザーのメカニズムの全機能を使用しているためです。 JavaとC ++のこのユニークな結合については、ここで読むことができます。
現時点では、より便利に使用できるGradle、Maven、IntelliJIDEAのプラグインがあります。 SonarQubeの継続的な品質保証プラットフォームに精通している場合は、で遊ぶというアイデアに興味があるかもしれません分析結果の統合。
弾む城について少し
Bouncy Castleは、Javaプログラミング言語で記述された暗号化アルゴリズムを実装したパッケージです（C＃にも実装がありますが、この記事ではそれについては説明していません）。このライブラリは、Standard Cryptographic Extension（JCE）を補完し、あらゆる環境（J2MEを含む）での使用に適したAPIを含みます。
プログラマーは、このライブラリのすべての機能を自由に使用できます。多数のアルゴリズムとプロトコルの実装により、このプロジェクトは暗号化を使用するソフトウェア開発者にとって非常に興味深いものになっています。
チェックを始めましょう
Bouncy Castleはかなり深刻なプロジェクトです。そのようなライブラリに間違いがあると、暗号化システムの信頼性が低下する可能性があるためです。したがって、最初は、このライブラリで少なくとも何か面白いものを見つけることができるかどうか、またはすべてのエラーがすでに検出されて修正されているかどうかさえ疑問に思いました。私たちのJavaアナライザーが私たちを失望させなかったとすぐに言いましょう:)
当然、1つの記事ですべてのアナライザーの警告を説明することはできませんが、オープンソースプロジェクトを開発する開発者向けの無料ライセンスがあります。必要に応じて、このライセンスを当社にリクエストし、PVS-Studioを使用してプロジェクトを独自に分析することができます。
そして、発見された最も興味深いコードスニペットを見ていきます。
到達不能なコード
V6019到達不能なコードが検出されました。エラーが存在する可能性があります。XMSSTest.java（170）
public void testSignSHA256CompleteEvenHeight2() {
....
int height = 10;
....
for (int i = 0; i < (1 << height); i++) {
byte[] signature = xmss.sign(new byte[1024]);
switch (i) {
case 0x005b:
assertEquals(signatures[0], Hex.toHexString(signature));
break;
case 0x0822:
assertEquals(signatures[1], Hex.toHexString(signature));
break;
....
}
}
}メソッドのheight 変数の値は変更されないため、forループのiカウンターは1024（1 << 10）を超えることはできません。ただし、switchステートメントでは、 2番目のケースはiを値0x0822（2082）に対してチェックします。当然、signatures [1]バイトは検証されません。 これはテストメソッドコードなので、心配する必要はありません。開発者は、バイトの1つのテストが実行されないという事実に注意を払う必要があるだけです。
同一の部分式
V6001「||」の左側と右側に同一のサブ式「tag == PacketTags.SECRET_KEY」があります。オペレーター。PGPUtil.java（212）、PGPUtil.java（212）
public static boolean isKeyRing(byte[] blob) throws IOException {
BCPGInputStream bIn = new BCPGInputStream(new ByteArrayInputStream(blob));
int tag = bIn.nextPacketTag();
return tag == PacketTags.PUBLIC_KEY || tag == PacketTags.PUBLIC_SUBKEY
|| tag == PacketTags.SECRET_KEY || tag == PacketTags.SECRET_KEY;
}このコードスニペットでは、返されるステートメントは、タグ== PacketTags.SECRET_KEYをダブルチェックしました。公開キーのチェックと同様に、最後のチェックは、タグとPacketTags.SECRET_SUBKEYが等しいかどうかを確認する必要があります。
if / elseの同一コード
V6004「then」ステートメントは「else」ステートメントと同等です。BcAsymmetricKeyUnwrapper.java（36）、BcAsymmetricKeyUnwrapper.java（40）
public GenericKey generateUnwrappedKey(....) throws OperatorException {
....
byte[] key = keyCipher.processBlock(....);
if (encryptedKeyAlgorithm.getAlgorithm().equals(....)) {
return new GenericKey(encryptedKeyAlgorithm, key);
} else {
return new GenericKey(encryptedKeyAlgorithm, key);
}
}この例では、ifの条件が満たされているかどうかに関係なく、メソッドはGenericKeyクラスの同じインスタンスを返します。if / elseブランチのコードは異なっている必要があることは明らかです。そうでない場合、ifのチェックはまったく意味がありません。ここで、プログラマーはコピーアンドペーストによって明らかに失望しました。
式は常に偽です
V6007式 '！（NGroups <8）'は常にfalseです。CBZip2OutputStream.java（753）
private void sendMTFValues() throws IOException {
....
int nGroups;
....
if (nMTF < 200) {
nGroups = 2;
} else if (nMTF < 600) {
nGroups = 3;
} else if (nMTF < 1200) {
nGroups = 4;
} else if (nMTF < 2400) {
nGroups = 5;
} else {
nGroups = 6;
}
....
if (!(nGroups < 8)) {
panic();
}
}ここで、if / elseコードブロックのnGroups変数には、使用されるがどこでも変更されない値が割り当てられます。ifステートメントの式は常にfalseになります。nGroupsのすべての可能な値：2、3、4、5、および6は8未満です。 アナライザーは、panic（）メソッドが実行されないことを理解しているため、アラームを発生させます。しかし、ここでは、おそらく「防御プログラミング」が使用されており、心配する必要はありません。
同じ要素を追加する
V6033同じキー「PKCSObjectIdentifiers.pbeWithSHAAnd3_KeyTripleDES_CBC」のアイテムがすでに追加されています。PKCS12PBEUtils.java（50）、PKCS12PBEUtils.java（49）
class PKCS12PBEUtils {
static {
....
keySizes.put(PKCSObjectIdentifiers.pbeWithSHAAnd3_KeyTripleDES_CBC,
Integers.valueOf(192));
keySizes.put(PKCSObjectIdentifiers.pbeWithSHAAnd2_KeyTripleDES_CBC,
Integers.valueOf(128));
....
desAlgs.add(PKCSObjectIdentifiers.pbeWithSHAAnd3_KeyTripleDES_CBC);
desAlgs.add(PKCSObjectIdentifiers.pbeWithSHAAnd3_KeyTripleDES_CBC);
}
}このエラーも、コピーと貼り付けが原因です。2つの同一の要素がdesAlgsコンテナに追加されます。開発者はコードの最後の行をコピーしましたが、フィールド名の番号3 x2を修正するのを忘れていました。
インデックスが範囲外
V6025インデックス「i」が範囲外である可能性があります。HSSTests.java（384）
public void testVectorsFromReference() throws Exception {
List<LMSigParameters> lmsParameters = new ArrayList<LMSigParameters>();
List<LMOtsParameters> lmOtsParameters = new ArrayList<LMOtsParameters>();
....
for (String line : lines) {
....
if (line.startsWith("Depth:")) {
....
} else if (line.startsWith("LMType:")) {
....
lmsParameters.add(LMSigParameters.getParametersForType(typ));
} else if (line.startsWith("LMOtsType:")) {
....
lmOtsParameters.add(LMOtsParameters.getParametersForType(typ));
}
}
....
for (int i = 0; i != lmsParameters.size(); i++) {
lmsParams.add(new LMSParameters(lmsParameters.get(i),
lmOtsParameters.get(i)));
}
}lmsParametersおよびlmOtsParametersコレクション への要素の追加は、最初のforループで、if / elseステートメントのさまざまなブランチで行われます。次に、2番目のforループで、コレクションアイテムはインデックスiでアクセスされます。インデックスiが最初のコレクションのサイズよりも小さいことを確認するだけで、forループでは2番目のコレクションのサイズはチェックされません。コレクションのサイズが異なることが判明した場合は、IndexOutOfBoundsExceptionが発生する可能性があります。..。ただし、これはテストメソッドコードであり、この警告は特に危険をもたらすものではないことに注意してください。コレクションには、事前に作成されたファイルからのテストデータが入力されます。もちろん、要素を追加した後、コレクションのサイズは同じになります。
nullチェック前の使用法
V6060'params '参照は、nullに対して検証される前に使用されていました。BCDSAPublicKey.java（54）、BCDSAPublicKey.java（53）
BCDSAPublicKey(DSAPublicKeyParameters params) {
this.y = params.getY();
if (params != null) {
this.dsaSpec = new DSAParameterSpec(params.getParameters().getP(),
params.getParameters().getQ(),
params.getParameters().getG());
} else {
this.dsaSpec = null;
}
this.lwKeyParams = params;
}メソッドの最初の行は、yをparams.getY（）に設定します。割り当ての直後に、params変数のnullがチェックされます。特定のメソッドでparamsをnullにすることが許可されている場合は、変数を使用する前にこのチェックを行う必要があります。
冗長チェックインif / else
V6003'if（A）{...} else if（A）{...} 'パターンの使用が検出されました。論理エラーが存在する可能性があります。EnrollExample.java（108）、EnrollExample.java（113）
public EnrollExample(String[] args) throws Exception {
....
for (int t = 0; t < args.length; t++) {
String arg = args[t];
if (arg.equals("-r")) {
reEnroll = true;
} ....
else if (arg.equals("--keyStoreType")) {
keyStoreType = ExampleUtils.nextArgAsString
("Keystore type", args, t);
t += 1;
} else if (arg.equals("--keyStoreType")) {
keyStoreType = ExampleUtils.nextArgAsString
("Keystore type", args, t);
t += 1;
} ....
}
}もし/ else文のチェックの値引数の文字列を二回列と平等のための「 - keystoreTypeキー」。当然、2番目のチェックは冗長であり、意味がありません。ただし、これはエラーのようには見えません。コマンドライン引数のヘルプテキストには、if / elseブロックで処理されない他のパラメータはありません。これはおそらく、削除する必要のある冗長なコードです。
メソッドは同じ値を返します
V6014このメソッドが常に1つの同じ値を返すのは奇妙です。XMSSSigner.java（129）
public AsymmetricKeyParameter getUpdatedPrivateKey() {
// if we've generated a signature return the last private key generated
// if we've only initialised leave it in place
// and return the next one instead.
synchronized (privateKey) {
if (hasGenerated) {
XMSSPrivateKeyParameters privKey = privateKey;
privateKey = null;
return privKey;
} else {
XMSSPrivateKeyParameters privKey = privateKey;
if (privKey != null) {
privateKey = privateKey.getNextKey();
}
return privKey;
}
}
}このメソッドは常に同じものを返すため、アナライザーは警告を発行します。メソッドのコメントによると、署名が生成されたかどうかに応じて、最後に生成されたキーを返すか、次のキーを返す必要があります。どうやら、この方法は理由のために分析者に疑わしいように見えました。
まとめましょう
ご覧のとおり、Bouncy Castleプロジェクトでエラーを見つけることができました。これは、完璧なコードを書いている人がいないことをもう一度確認するだけです。さまざまな要因が機能する可能性があります。開発者が疲れている、注意を怠っている、誰かが気を散らしている...コードが人によって書かれている限り、エラーは常に発生します。
プロジェクトが成長するにつれて、コード内の問題を見つけることはますます困難になります。したがって、現代の世界では、静的コード分析は単なる別の方法ではなく、本当に必要なものです。コードレビュー、TDD、または動的分析を使用している場合でも、これは静的分析を拒否できることを意味するものではありません。これらは完全に異なる方法であり、互いに完全に補完し合っています。
静的分析を開発段階の1つにすることで、コードの作成時にエラーをほぼ即座に見つけることができます。その結果、開発者はこれらのエラーのデバッグに何時間も費やす必要がなくなります。テスターは、とらえどころのないバグをはるかに少なく再現する必要があります。ユーザーは、より信頼性が高く安定したプログラムを受け取ります。
一般に、プロジェクトでは必ず静的分析を使用してください。私たちは自分たちでそれをします、そして私たちはあなたにそれをお勧めします:)
この記事を英語を話す聴衆と共有したい場合は、翻訳リンクを使用してください：IrinaPolynkina。あなたの安全を守るためのユニコーン：弾む城のコードを探る。