Clever Geek Handbook

独角兽为您提供安全：检查有弹性的城堡法规

image1.png


您是否要查看PVS-Studio for 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）的API。



程序员可以自由使用该库的所有功能。大量算法和协议的实现使该项目对于使用密码学的软件开发人员而言非常有趣。



让我们开始检查



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


方法中 的height变量的值不变，因此for循环中的i计数器不能大于1024（1 << 10）。然而，在开关语句中，第二壳体检查我靠在值0x0822（2082）。自然，签名[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。与检查公钥类似，最后检查应该是tag和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);
    }
}


在此示例中，该方法将返回GenericKey类的相同实例，而不管是否满足if中的条件。显然，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);
    }
}


再次由于复制粘贴而导致此错误。将两个相同的元素添加到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在对null进行验证之前，已使用“ params”参考。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，则应在使用变量之前进行此检查。



冗余检查是否



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


该if / else语句检查的价值ARGS串与串平等两次“ - 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或动态分析，也并不意味着您可以拒绝静态分析。这些是完全不同的方法，可以完美地互补。



通过使静态分析成为开发阶段之一，您有机会在编写代码时几乎立即发现错误。因此，开发人员将无需花费大量时间来调试这些错误。测试人员将不得不重现更少的难以捉摸的错误。用户将收到更可靠，更稳定的程序。



通常，请确保在项目中使用静态分析！我们自己做，我们推荐给您:)





如果您想与讲英语的读者分享这篇文章，请使用翻译链接：Irina Polynkina。保护您安全的独角兽：探索“弹性城堡”法则。


More articles:


All Articles