وحيد القرن في سلامتك: فحص كود القلعة النطاطة

هل ترغب في رؤية مجموعة جديدة من الأخطاء التي تم العثور عليها بواسطة PVS-Studio لمحلل Java الثابت؟ ثم انضم لقراءة المقال! هذه المرة ، أصبح مشروع Bouncy Castle موضوع التحقق. مقتطفات التعليمات البرمجية الأكثر إثارة للاهتمام ، كالعادة ، في انتظارك أدناه.

قليلا عن PVS-Studio

PVS-Studio هي أداة لتحديد الأخطاء ونقاط الضعف المحتملة في التعليمات البرمجية المصدر للبرامج. في وقت كتابة هذا التقرير ، تم تنفيذ التحليل الثابت للبرامج المكتوبة بلغات البرمجة C و C ++ و C # و Java.



يعد محلل Java هو أحدث خط في PVS-Studio. على الرغم من ذلك ، فهو ليس أدنى من إخوته الأكبر سنًا في اكتشاف العيوب في الكود. هذا يرجع إلى حقيقة أن محلل Java يستخدم القوة الكاملة للآليات من محلل C ++. يمكنك أن تقرأ عن هذا الاتحاد الفريد لـ Java و C ++ هنا .



في الوقت الحالي ، هناك مكونات إضافية لـ Gradle و Maven و IntelliJ IDEA لاستخدام أكثر ملاءمة . إذا كنت معتادًا على منصة SonarQube المستمرة لمراقبة الجودة ، فقد تكون مهتمًا بفكرة اللعب بهاتكامل نتيجة التحليل.

قليلا عن قلعة نطاط

Bouncy Castle هي حزمة مع تنفيذ خوارزميات التشفير ، مكتوبة بلغة برمجة Java (هناك أيضًا تطبيق في C # ، لكن هذه المقالة ليست عن ذلك). تكمل هذه المكتبة ملحق التشفير القياسي (JCE) وتحتوي على واجهة برمجة تطبيقات مناسبة للاستخدام في أي بيئة (بما في ذلك J2ME).



المبرمجون أحرار في استخدام جميع ميزات هذه المكتبة. إن تنفيذ عدد كبير من الخوارزميات والبروتوكولات يجعل هذا المشروع ممتعًا جدًا لمطوري البرامج الذين يستخدمون التشفير.

لنبدأ في التحقق

يعد Bouncy Castle مشروعًا جادًا إلى حد ما ، لأن أي خطأ في مثل هذه المكتبة يمكن أن يقلل من موثوقية نظام التشفير. لذلك ، شككنا في البداية في ما إذا كان بإمكاننا العثور على أي شيء مثير للاهتمام في هذه المكتبة ، أو ما إذا كان قد تم بالفعل العثور على جميع الأخطاء وإصلاحها أمامنا. دعنا نقول على الفور أن محلل Java الخاص بنا لم يخيب آمالنا :)



بطبيعة الحال ، لا يمكننا وصف جميع تحذيرات المحلل في مقال واحد ، ولكن لدينا ترخيصًا مجانيًا للمطورين الذين يطورون مشاريع مفتوحة المصدر . إذا كنت ترغب في ذلك ، يمكنك طلب هذا الترخيص منا وتحليل المشروع بشكل مستقل باستخدام 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;
            ....
        }
    }
}

لا تتغير قيمة متغير الارتفاع في الطريقة ، لذلك لا يمكن أن يكون العداد i في الحلقة for أكبر من 1024 (1 << 10). ومع ذلك ، في بيان التبديل ، تتحقق الحالة الثانية من i مقابل القيمة 0x0822 (2082). بطبيعة الحال ، لن يتم التحقق من التوقيعات [1] بايت .



نظرًا لأن هذا رمز طريقة اختبار ، فلا داعي للقلق. يحتاج المطورون فقط إلى الانتباه إلى حقيقة أن اختبار أحد البايتات لا يتم إجراؤه أبدًا.

تعبيرات متطابقة

V6001 توجد علامة تعبيرات فرعية متطابقة == 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". Bc AsymmetricKeyUnwrapper.java (36) ، قبل الميلاد AsymmetricKeyUnwrapper.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);
    }
}

في هذا المثال ، تقوم الطريقة بإرجاع نفس مثيلات فئة GenericKey ، بغض النظر عما إذا كان الشرط في if مستوفيا أم لا. من الواضح أن الكود الموجود في فرعي if / else يجب أن يكون مختلفًا ، وإلا فإن التحقق في if لا معنى له على الإطلاق. هنا تم التخلي عن المبرمج بشكل واضح عن طريق النسخ واللصق.

التعبير خاطئ دائمًا

تعبير V6007 '! (NGroups <8)' خاطئ دائمًا. 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();
    }
}

هنا ، يتم تعيين قيمة متغير nGroups في مقاطع التعليمات البرمجية if / else والتي يتم استخدامها ولكنها لا تتغير في أي مكان. سيكون التعبير في تعليمة if دائمًا خاطئًا ، لأن جميع القيم الممكنة لـ nGroups : 2 و 3 و 4 و 5 و 6 أقل من 8.



يدرك المحلل أن طريقة الذعر () لن يتم تنفيذها أبدًا ، وبالتالي يثير الإنذار. ولكن هنا ، على الأرجح ، تم استخدام "البرمجة الدفاعية" ، وليس هناك ما يدعو للقلق.

إضافة نفس العناصر

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);
    }
}

هذا الخطأ مرة أخرى بسبب النسخ واللصق. يتم إضافة عنصرين متطابقين إلى حاوية desAlgs . قام المطور بنسخ السطر الأخير من الكود ، لكنه نسي تصحيح الرقم 3 × 2 في اسم الحقل.

الفهرس خارج النطاق

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 . بعد ذلك ، في حلقة for الثانية ، يتم الوصول إلى عناصر المجموعة في الفهرس i . إنه يتحقق فقط من أن الفهرس i أقل من حجم المجموعة الأولى ، وأن حجم المجموعة الثانية غير محدد في الحلقة for . إذا كانت أحجام المجموعات مختلفة ، فمن المحتمل أنه يمكنك الحصول على IndexOutOfBoundsException... ومع ذلك ، تجدر الإشارة إلى أن هذا هو رمز طريقة الاختبار ، وهذا التحذير لا يشكل أي خطر معين ، منذ ذلك الحين تمتلئ المجموعات ببيانات الاختبار من ملف تم إنشاؤه مسبقًا ، وبالطبع ، بعد إضافة العناصر ، يكون للمجموعات نفس الحجم.

الاستخدام قبل فحص القيمة الفارغة

V6060 تم استخدام مرجع " المعلمات " قبل أن يتم التحقق منه مقابل قيمة خالية. 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 () . مباشرة بعد التعيين ، يتم التحقق من متغير المعلمات من أجل القيمة الخالية . إذا كان مسموحًا بأن تكون المعلمات فارغة في طريقة معينة ، فيجب عليك إجراء هذا التحقق قبل استخدام المتغير.

تسجيل وصول زائدة عن الحاجة إذا / آخر

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;
        } ....
    }
}

و إذا / بيان شيء آخر شيكات قيمة وسائط سلسلة مرتين من أجل المساواة مع السلسلة "- keyStoreType ". وبطبيعة الحال ، فإن الفحص الثاني زائد عن الحاجة ولا معنى له. ومع ذلك ، هذا لا يبدو خطأ ، منذ ذلك الحين لا توجد معلمات أخرى في نص تعليمات وسيطة سطر الأوامر التي لم تتم معالجتها في كتلة if / else . هذا على الأرجح رمز زائد يجب إزالته.

الطريقة ترجع نفس القيمة

V6014 من الغريب أن تقوم هذه الطريقة بإرجاع قيمة واحدة ونفس القيمة دائمًا. 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 أو التحليل الديناميكي ، فهذا لا يعني أنه يمكنك رفض التحليل الثابت. هذه منهجيات مختلفة تمامًا تكمل بعضها البعض تمامًا.



من خلال جعل التحليل الثابت إحدى مراحل التطوير ، تحصل على فرصة للعثور على الأخطاء على الفور تقريبًا ، في وقت كتابة الكود. نتيجة لذلك ، لن يحتاج المطورون إلى قضاء ساعات من تصحيح هذه الأخطاء. سيتعين على المختبرين إعادة إنتاج عدد أقل بكثير من الأخطاء المراوغة. سيحصل المستخدمون على برنامج يعمل بشكل أكثر موثوقية وأكثر استقرارًا.



بشكل عام ، تأكد من استخدام التحليل الثابت في مشاريعك! نحن نفعل ذلك بأنفسنا ، ونوصي به لك :)

إذا كنت ترغب في مشاركة هذه المقالة مع جمهور يتحدث الإنجليزية ، فيرجى استخدام رابط الترجمة: إيرينا بولينكينا. وحيد القرن على أهبة الاستعداد من أجل سلامتك: استكشاف رمز القلعة النطاطة .