Apakah Anda ingin melihat kumpulan error baru yang ditemukan oleh penganalisa statis PVS-Studio untuk Java? Bergabunglah dengan membaca artikel! Kali ini, proyek Bouncy Castle menjadi objek verifikasi. Cuplikan kode yang paling menarik, seperti biasa, menunggu Anda di bawah ini.
Sedikit tentang PVS-Studio
PVS-Studio adalah alat untuk mengidentifikasi kesalahan dan potensi kerentanan dalam kode sumber program. Pada saat penulisan ini, analisis statik diimplementasikan untuk program yang ditulis dalam bahasa pemrograman C, C ++, C # dan Java.
Penganalisis untuk Java adalah lini termuda dari PVS-Studio. Meskipun demikian, ia tidak kalah dengan kakak laki-lakinya dalam menemukan cacat pada kode. Hal ini disebabkan oleh fakta bahwa penganalisis Java menggunakan kekuatan penuh mekanisme dari penganalisis C ++. Anda dapat membaca tentang gabungan unik Java dan C ++ di sini .
Saat ini, ada plugin untuk Gradle, Maven dan IntelliJ IDEA untuk penggunaan yang lebih nyaman . Jika Anda sudah familiar dengan platform jaminan kualitas berkelanjutan SonarQube, maka Anda mungkin tertarik untuk bermain denganintegrasi hasil analisis.
Sedikit tentang Bouncy Castle
Bouncy Castle adalah paket dengan implementasi algoritma kriptografi yang ditulis dalam bahasa pemrograman Java (ada juga implementasi di C #, tapi artikel ini tidak akan membahasnya). Library ini melengkapi Standard Cryptographic Extension (JCE) dan berisi API yang cocok untuk digunakan di lingkungan apa pun (termasuk J2ME).
Pemrogram bebas menggunakan semua fitur perpustakaan ini. Implementasi sejumlah besar algoritma dan protokol membuat proyek ini sangat menarik bagi para pengembang perangkat lunak yang menggunakan kriptografi.
Mari kita mulai memeriksa
Bouncy Castle adalah proyek yang cukup serius, karena kesalahan apa pun dalam pustaka semacam itu dapat mengurangi keandalan sistem enkripsi. Oleh karena itu, pada awalnya kami bahkan ragu apakah kami dapat menemukan setidaknya sesuatu yang menarik di perpustakaan ini, atau apakah semua kesalahan sudah ditemukan dan diperbaiki sebelum kami. Katakanlah segera bahwa penganalisis Java kami tidak mengecewakan kami :)
Biasanya, kami tidak dapat menjelaskan semua peringatan penganalisis dalam satu artikel, tetapi kami memiliki lisensi gratis untuk pengembang yang mengembangkan proyek open source . Jika mau, Anda dapat meminta lisensi ini dari kami dan menganalisis proyek secara mandiri menggunakan PVS-Studio.
Dan kami akan mulai melihat cuplikan kode paling menarik yang telah ditemukan.
Kode tidak dapat dijangkau
V6019 Kode tidak dapat dijangkau terdeteksi. Mungkin saja ada kesalahan. 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;
....
}
}
}Nilai variabel tinggi dalam metode tidak berubah, sehingga penghitung i di loop for tidak boleh lebih besar dari 1024 (1 << 10). Namun, dalam pernyataan switch, kasus kedua memeriksa i terhadap nilai 0x0822 (2082). Secara alami, tanda tangan [1] byte tidak akan pernah diverifikasi .
Karena ini adalah kode metode pengujian, tidak ada yang perlu dikhawatirkan. Hanya saja developer perlu memperhatikan fakta bahwa pengujian salah satu byte tersebut tidak pernah dilakukan.
Subekspresi identik
V6001 Ada sub-ekspresi identik 'tag == PacketTags.SECRET_KEY' di kiri dan kanan '||' 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;
}Dalam potongan kode ini, pernyataan sebagai gantinya , double-check tag == PacketTags.SECRET_KEY . Mirip dengan memeriksa kunci publik, pemeriksaan terakhir harus untuk kesetaraan antara tag dan PacketTags.SECRET_SUBKEY .
Kode identik di if / else
V6004 Pernyataan 'then' setara dengan pernyataan '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);
}
}Dalam contoh ini, metode mengembalikan instance kelas GenericKey yang sama , terlepas dari apakah kondisi di if terpenuhi atau tidak. Jelas bahwa kode di cabang if / else harus berbeda, jika tidak, centang di if sama sekali tidak masuk akal. Di sini programmer jelas dikecewakan oleh copy-paste.
Ekspresi selalu salah
V6007 Expression '! (NGroups <8)' selalu salah. 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();
}
}Di sini, variabel nGroups di blok kode if / else diberi nilai yang digunakan tetapi tidak berubah di mana pun. Ekspresi dalam pernyataan if akan selalu salah, karena semua kemungkinan nilai untuk nGroups : 2, 3, 4, 5 dan 6 kurang dari 8.
Penganalisis memahami bahwa metode panic () tidak akan pernah dijalankan, dan oleh karena itu meningkatkan alarm. Tapi di sini, kemungkinan besar, "pemrograman defensif" digunakan, dan tidak ada yang perlu dikhawatirkan.
Menambahkan elemen yang sama
V6033 Item dengan kunci yang sama 'PKCSObjectIdentifiers.pbeWithSHAAnd3_KeyTripleDES_CBC' telah ditambahkan. 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);
}
}Kesalahan ini sekali lagi karena salin-tempel. Dua elemen identik ditambahkan ke wadah desAlgs . Pengembang menyalin baris terakhir kode, tetapi lupa mengoreksi angka 3 dengan 2 di nama bidang.
Indeks di luar jangkauan
V6025 Mungkin indeks 'i' di luar batas. 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)));
}
}Menambahkan elemen ke koleksi lmsParameters dan lmOtsParameters dilakukan di loop for pertama , di cabang pernyataan if / else yang berbeda . Kemudian, di perulangan for kedua , item koleksi diakses pada indeks i . Ini hanya memeriksa bahwa indeks i kurang dari ukuran koleksi pertama, dan ukuran koleksi kedua tidak diperiksa di loop for . Jika ukuran koleksinya ternyata berbeda, kemungkinan Anda bisa mendapatkan IndexOutOfBoundsException... Namun, perlu dicatat bahwa ini adalah kode metode pengujian, dan peringatan ini tidak menimbulkan bahaya khusus, karena koleksi diisi dengan data uji dari file yang dibuat sebelumnya dan, tentu saja, setelah menambahkan elemen, koleksi memiliki ukuran yang sama.
Penggunaan sebelum pemeriksaan nol
V6060 Referensi 'params' digunakan sebelum diverifikasi terhadap 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;
}Baris pertama dari metode ini menyetel y ke params.getY () . Segera setelah penugasan, variabel params diperiksa untuk null . Jika diperbolehkan bahwa params bisa menjadi null dalam metode ini , Anda harus melakukan pemeriksaan ini sebelum menggunakan variabel.
Check in berlebihan jika / lain
V6003 Penggunaan pola 'if (A) {...} else if (A) {...}' terdeteksi. Ada kemungkinan kehadiran kesalahan logis. 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;
} ....
}
}Dalam pernyataan if / else, nilai dari string args diperiksa dua kali untuk persamaan dengan string "- keyStoreType ". Secara alami, pemeriksaan kedua berlebihan dan tidak masuk akal. Namun, ini tidak terlihat seperti kesalahan, karena tidak ada parameter lain dalam teks bantuan argumen baris perintah yang tidak diproses di blok if / else . Kemungkinan besar ini adalah kode yang berlebihan yang harus dihapus.
Metode ini mengembalikan nilai yang sama
V6014 Aneh bahwa metode ini selalu mengembalikan satu dan nilai yang sama. 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;
}
}
}Penganalisis mengeluarkan peringatan karena metode ini selalu mengembalikan yang sama. Komentar metode mengatakan bahwa bergantung pada apakah tanda tangan dibuat atau tidak, kunci yang dihasilkan terakhir harus dikembalikan atau yang berikutnya. Rupanya, metode ini tampak mencurigakan bagi penganalisis karena suatu alasan.
Mari kita simpulkan
Seperti yang Anda lihat, kami masih berhasil menemukan kesalahan dalam proyek Bouncy Castle, dan ini hanya menegaskan sekali lagi bahwa tidak ada dari kami yang menulis kode yang sempurna. Berbagai faktor dapat bekerja: pengembang lelah, lalai, seseorang mengalihkan perhatiannya ... Selama kode ditulis oleh seseorang, kesalahan akan selalu terjadi.
Saat proyek berkembang, menemukan masalah dalam kode menjadi semakin sulit. Oleh karena itu, di dunia modern, analisis kode statis bukan hanya metodologi lain, tetapi kebutuhan nyata. Bahkan jika Anda menggunakan tinjauan kode, TDD atau analisis dinamis, ini tidak berarti bahwa Anda dapat menolak analisis statis. Ini adalah metodologi yang sangat berbeda yang saling melengkapi dengan sempurna.
Dengan membuat analisis statis sebagai salah satu tahap pengembangan, Anda berkesempatan untuk menemukan kesalahan hampir seketika, pada saat menulis kode. Akibatnya, pengembang tidak perlu menghabiskan waktu berjam-jam untuk men-debug kesalahan ini. Penguji harus mereproduksi bug yang jauh lebih sedikit yang sulit dipahami. Pengguna akan menerima program yang lebih andal dan stabil.
Secara umum, pastikan untuk menggunakan analisis statis dalam proyek Anda! Kami melakukannya sendiri, dan kami merekomendasikannya kepada Anda :)
Jika Anda ingin berbagi artikel ini dengan audiens berbahasa Inggris, silakan gunakan tautan terjemahan: Irina Polynkina. Unicorn Bersiap untuk Keselamatan Anda: Menjelajahi Kode Istana Bouncy .