Möchten Sie eine neue Reihe von Fehlern sehen, die vom statischen Analysator PVS-Studio für Java gefunden wurden? Dann lesen Sie den Artikel! Dieses Mal wurde das Bouncy Castle-Projekt Gegenstand der Überprüfung. Die interessantesten Codefragmente warten wie gewohnt unten auf Sie.
Ein bisschen über PVS-Studio
PVS-Studio ist ein Tool zum Erkennen von Fehlern und potenziellen Schwachstellen im Quellcode von Programmen. Zum Zeitpunkt dieses Schreibens ist eine statische Analyse für Programme implementiert, die in den Programmiersprachen C, C ++, C # und Java geschrieben sind.
Der Analysator für Java ist die jüngste Linie von PVS-Studio. Trotzdem ist er seinen älteren Brüdern nicht unterlegen, wenn es darum geht, Fehler im Code zu finden. Dies liegt an der Tatsache, dass der Java-Analysator die volle Leistung der Mechanismen des C ++ - Analysators nutzt. Über diese einzigartige Vereinigung von Java und C ++ können Sie hier lesen .
Momentan gibt es Plugins für Gradle, Maven und IntelliJ IDEA für eine bequemere Verwendung . Wenn Sie mit der SonarQube-Plattform zur kontinuierlichen Qualitätssicherung vertraut sind, könnte Sie die Idee des Spielens interessierenIntegration des Analyseergebnisses.
Ein bisschen über Hüpfburg
Bouncy Castle ist ein Paket mit der Implementierung kryptografischer Algorithmen, die in der Programmiersprache Java geschrieben sind (es gibt auch eine Implementierung in C #, aber in diesem Artikel wird nicht darauf eingegangen). Diese Bibliothek ergänzt die Standard Cryptographic Extension (JCE) und enthält eine API, die für die Verwendung in jeder Umgebung (einschließlich J2ME) geeignet ist.
Programmierer können alle Funktionen dieser Bibliothek nutzen. Die Implementierung einer Vielzahl von Algorithmen und Protokollen macht dieses Projekt für Softwareentwickler, die Kryptografie verwenden, sehr interessant.
Beginnen wir mit der Überprüfung
Bouncy Castle ist ein ziemlich ernstes Projekt, da jeder Fehler in einer solchen Bibliothek die Zuverlässigkeit des Verschlüsselungssystems beeinträchtigen kann. Daher haben wir zunächst sogar daran gezweifelt, ob wir in dieser Bibliothek zumindest etwas Interessantes finden könnten oder ob alle Fehler bereits vor uns gefunden und behoben wurden. Nehmen wir sofort an, unser Java-Analysator hat uns nicht enttäuscht :)
Natürlich können wir nicht alle Warnungen des Analysators in einem Artikel beschreiben, aber wir haben eine kostenlose Lizenz für Entwickler, die Open-Source- Projekte entwickeln. Wenn Sie möchten, können Sie diese Lizenz bei uns anfordern und das Projekt mit PVS-Studio unabhängig analysieren.
Und wir werden uns die interessantesten Code-Schnipsel ansehen, die entdeckt wurden.
Unerreichbarer Code
V6019 Nicht erreichbarer Code erkannt. Möglicherweise liegt ein Fehler vor. 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;
....
}
}
}Der Wert der Höhenvariablen in der Methode ändert sich nicht, sodass der i- Zähler in der for- Schleife nicht größer als 1024 (1 << 10) sein kann. In der switch-Anweisung prüft der zweite Fall i jedoch mit dem Wert 0x0822 (2082). Natürlich werden die Signaturen [1] nie überprüft .
Da es sich um einen Testmethodencode handelt, besteht kein Grund zur Sorge. Es ist nur so, dass Entwickler darauf achten müssen, dass das Testen eines der Bytes niemals durchgeführt wird.
Identische Unterausdrücke
V6001 Links und rechts vom '||' befinden sich identische Unterausdrücke 'tag == PacketTags.SECRET_KEY'. Operator. 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;
}In diesem Codeausschnitt ist die Anweisung im Gegenzug das doppelt überprüfte Tag == PacketTags.SECRET_KEY . Ähnlich wie beim Überprüfen des öffentlichen Schlüssels sollte die letzte Überprüfung auf Gleichheit zwischen Tag und PacketTags.SECRET_SUBKEY erfolgen .
Identischer Code in if / else
V6004 Die Anweisung 'then' entspricht der Anweisung '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);
}
}In diesem Beispiel gibt die Methode dieselben Instanzen der GenericKey- Klasse zurück , unabhängig davon, ob die Bedingung im if erfüllt ist oder nicht. Es ist klar, dass der Code in den if / else- Zweigen unterschiedlich sein muss, sonst macht die Prüfung im if überhaupt keinen Sinn. Hier wurde der Programmierer durch Kopieren und Einfügen deutlich enttäuscht.
Ausdruck ist immer falsch
V6007 Ausdruck '! (NGroups <8)' ist immer falsch. 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();
}
}Hier wird der Variablen nGroups in den if / else -Codeblöcken ein Wert zugewiesen, der verwendet wird, sich aber nirgendwo ändert. Der Ausdruck in der if-Anweisung ist immer falsch, weil Alle möglichen Werte für nGroups : 2, 3, 4, 5 und 6 sind kleiner als 8.
Der Analysator versteht, dass die Panic () -Methode niemals ausgeführt wird, und löst daher den Alarm aus. Aber hier wurde höchstwahrscheinlich "defensive Programmierung" verwendet, und es gibt nichts, worüber man sich Sorgen machen müsste.
Hinzufügen der gleichen Elemente
V6033 Ein Element mit demselben Schlüssel 'PKCSObjectIdentifiers.pbeWithSHAAnd3_KeyTripleDES_CBC' wurde bereits hinzugefügt. 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);
}
}Dieser Fehler ist erneut auf das Kopieren und Einfügen zurückzuführen. Dem desAlgs- Container werden zwei identische Elemente hinzugefügt. Der Entwickler hat die letzte Zeile des Codes kopiert, aber vergessen, die Zahl 3 mal 2 im Feldnamen zu korrigieren.
Index außerhalb des Bereichs
V6025 Möglicherweise ist der Index 'i' außerhalb der Grenzen. 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)));
}
}Das Hinzufügen von Elementen zur Auflistung lmsParameters und lmOtsParameters erfolgt in der ersten for- Schleife in verschiedenen Zweigen der if / else-Anweisung . In der zweiten for- Schleife wird dann am Index i auf die Sammlungselemente zugegriffen . Es wird nur überprüft, ob der Index i kleiner als die Größe der ersten Sammlung ist, und die Größe der zweiten Sammlung wird in der for- Schleife nicht überprüft. Wenn sich herausstellt, dass die Größe der Sammlungen unterschiedlich ist, können Sie wahrscheinlich eine IndexOutOfBoundsException erhalten... Es ist jedoch zu beachten, dass dies ein Testmethodencode ist und diese Warnung keine besondere Gefahr darstellt, da Die Sammlungen werden mit Testdaten aus einer vorab erstellten Datei gefüllt, und natürlich haben die Sammlungen nach dem Hinzufügen von Elementen dieselbe Größe.
Verwendung vor der Nullprüfung
V6060 Die 'params'-Referenz wurde verwendet, bevor sie gegen null verifiziert wurde. 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;
}In der ersten Zeile der Methode wird y auf params.getY () gesetzt . Unmittelbar nach der Zuweisung wird die Variable params auf null geprüft . Wenn es zulässig ist, dass Parameter in einer bestimmten Methode null sein können , sollten Sie diese Prüfung durchgeführt haben, bevor Sie die Variable verwenden.
Redundantes Einchecken, wenn / sonst
V6003 Die Verwendung des Musters 'if (A) {...} else if (A) {...}' wurde erkannt. Es besteht die Wahrscheinlichkeit eines logischen Fehlers. 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;
} ....
}
}In der if / else-Anweisung wird der Wert der args- Zeichenfolge zweimal auf Gleichheit mit der Zeichenfolge "- keyStoreType " überprüft . Die zweite Prüfung ist natürlich überflüssig und macht keinen Sinn. Dies sieht jedoch nicht nach einem Fehler aus, da Der Hilfetext des Befehlszeilenarguments enthält keine anderen Parameter, die nicht im if / else- Block verarbeitet werden . Dies ist höchstwahrscheinlich redundanter Code, der entfernt werden sollte.
Die Methode gibt denselben Wert zurück
V6014 Es ist seltsam, dass diese Methode immer ein und denselben Wert zurückgibt. 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;
}
}
}Der Analysator gibt eine Warnung aus, da diese Methode immer dieselbe zurückgibt. Der Methodenkommentar besagt, dass je nachdem, ob die Signatur generiert wurde oder nicht, entweder der zuletzt generierte Schlüssel oder der nächste zurückgegeben werden sollte. Anscheinend schien diese Methode dem Analysator aus einem bestimmten Grund verdächtig.
Fassen wir zusammen
Wie Sie sehen, haben wir es immer noch geschafft, Fehler im Bouncy Castle-Projekt zu finden, und dies bestätigt nur noch einmal, dass keiner von uns perfekten Code schreibt. Verschiedene Faktoren können funktionieren: Der Entwickler ist müde, unaufmerksam, jemand hat ihn abgelenkt ... Solange der Code von einer Person geschrieben wird, treten immer Fehler auf.
Wenn Projekte wachsen, wird es immer schwieriger, Probleme in Ihrem Code zu finden. In der modernen Welt ist die statische Code-Analyse daher nicht nur eine andere Methode, sondern eine echte Notwendigkeit. Selbst wenn Sie Codeüberprüfung, TDD oder dynamische Analyse verwenden, bedeutet dies nicht, dass Sie die statische Analyse ablehnen können. Dies sind völlig unterschiedliche Methoden, die sich perfekt ergänzen.
Wenn Sie die statische Analyse zu einer der Entwicklungsstufen machen, haben Sie die Möglichkeit, Fehler fast sofort zum Zeitpunkt des Schreibens des Codes zu finden. Infolgedessen müssen Entwickler keine Stunden mit dem Debuggen dieser Fehler verbringen. Tester müssen weit weniger schwer fassbare Fehler reproduzieren. Benutzer erhalten ein Programm, das zuverlässiger und stabiler ist.
Verwenden Sie im Allgemeinen statische Analysen in Ihren Projekten! Wir machen es selbst und empfehlen es Ihnen :)
Wenn Sie diesen Artikel einem englischsprachigen Publikum zugänglich machen möchten, verwenden Sie bitte den Übersetzungslink: Irina Polynkina. Einhörner auf der Hut für Ihre Sicherheit: Erkundung des Hüpfburg-Codes .