Quer ver um novo lote de erros encontrado pelo analisador estático PVS-Studio para Java? Então junte-se à leitura do artigo! Desta vez, o projeto do Castelo Bouncy passou a ser objeto de verificação. Os trechos de código mais interessantes, como sempre, estão esperando por você abaixo.
Um pouco sobre PVS-Studio
PVS-Studio é uma ferramenta para identificar erros e vulnerabilidades potenciais no código-fonte dos programas. No momento em que este artigo foi escrito, a análise estática foi implementada para programas escritos nas linguagens de programação C, C ++, C # e Java.
O analisador para Java é a linha mais jovem do PVS-Studio. Apesar disso, ele não é inferior a seus irmãos mais velhos na descoberta de defeitos no código. Isso se deve ao fato de que o analisador Java utiliza toda a potência dos mecanismos do analisador C ++. Você pode ler sobre essa união única de Java e C ++ aqui .
No momento, existem plug-ins para Gradle, Maven e IntelliJ IDEA para uso mais conveniente . Se você está familiarizado com a plataforma de garantia de qualidade contínua SonarQube, então você pode estar interessado na ideia de brincar comintegração do resultado da análise.
Um pouco sobre o Castelo Bouncy
Bouncy Castle é um pacote com a implementação de algoritmos criptográficos escritos na linguagem de programação Java (também existe uma implementação em C #, mas este artigo não é sobre isso). Esta biblioteca complementa a Standard Cryptographic Extension (JCE) e contém uma API adequada para uso em qualquer ambiente (incluindo J2ME).
Os programadores podem usar todos os recursos desta biblioteca. A implementação de um grande número de algoritmos e protocolos torna este projeto muito interessante para desenvolvedores de software que utilizam criptografia.
Vamos começar a verificar
O Bouncy Castle é um projeto bastante sério, pois qualquer erro em tal biblioteca pode reduzir a confiabilidade do sistema de criptografia. Portanto, no início até duvidamos se conseguiríamos encontrar pelo menos algo interessante nesta biblioteca, ou se todos os erros já haviam sido encontrados e corrigidos antes de nós. Digamos imediatamente que nosso analisador Java não nos decepcionou :)
Naturalmente, não podemos descrever todos os avisos do analisador em um artigo, mas temos uma licença gratuita para desenvolvedores que desenvolvem projetos de código aberto . Se desejar, você pode solicitar essa licença de nós e analisar de forma independente o projeto usando PVS-Studio.
E começaremos a examinar os trechos de código mais interessantes que foram descobertos.
Código inacessível
Código V6019 inacessível detectado. É possível que haja um erro. 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;
....
}
}
}O valor da variável height no método não muda, então o contador i no loop for não pode ser maior que 1024 (1 << 10). No entanto, na instrução switch, o segundo caso verifica i em relação ao valor 0x0822 (2082). Naturalmente, o byte de assinaturas [1] nunca será verificado .
Como este é um código de método de teste, não há nada com que se preocupar. É que os desenvolvedores precisam prestar atenção ao fato de que o teste de um dos bytes nunca é feito.
Subexpressões idênticas
V6001 Existem subexpressões idênticas 'tag == PacketTags.SECRET_KEY' à esquerda e à direita de '||' operador. 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;
}Neste trecho de código, a declaração em retorno , tag duplamente marcada == PacketTags.SECRET_KEY . Semelhante à verificação da chave pública, a última verificação deve ser de igualdade entre tag e PacketTags.SECRET_SUBKEY .
Código idêntico em if / else
V6004 A instrução 'then' é equivalente à instrução '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);
}
}Neste exemplo, o método retorna as mesmas instâncias da classe GenericKey , independentemente de a condição em if ser satisfeita ou não. É claro que o código nas ramificações if / else deve ser diferente, caso contrário, a verificação em if não faz sentido algum. Aqui, o programador foi claramente decepcionado por copiar e colar.
A expressão é sempre falsa
A expressão V6007 '! (NGroups <8)' é sempre falsa. 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();
}
}Aqui, a variável nGroups nos blocos de código if / else recebe um valor que é usado, mas não muda em lugar nenhum. A expressão na instrução if será sempre falsa, porque todos os valores possíveis para nGrupos : 2, 3, 4, 5 e 6 são menores que 8.
O analisador entende que o método panic () nunca será executado e, portanto, dispara o alarme. Mas aqui, provavelmente, foi usada "programação defensiva" e não há nada com que se preocupar.
Adicionando os mesmos elementos
V6033 Um item com a mesma chave 'PKCSObjectIdentifiers.pbeWithSHAAnd3_KeyTripleDES_CBC' já foi adicionado. 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);
}
}Este erro ocorre novamente devido ao copiar e colar. Dois elementos idênticos são adicionados ao contêiner desAlgs . O desenvolvedor copiou a última linha do código, mas esqueceu de corrigir o número 3 por 2 no nome do campo.
Índice fora do intervalo
V6025 Possivelmente o índice 'i' está fora dos limites. 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)));
}
}Adicionar elementos à lmsParameters e recolha lmOtsParameters é feito em primeiro de circuito , em diferentes ramos da instrução else if / . Então, no segundo loop for, os itens da coleção são acessados no índice i . Ele apenas verifica se o índice i é menor que o tamanho da primeira coleção, e o tamanho da segunda coleção não é verificado no loop for . Se os tamanhos das coleções forem diferentes, é provável que você possa obter uma IndexOutOfBoundsException... No entanto, deve-se notar que este é um código de método de teste, e este aviso não representa nenhum perigo particular, uma vez que coleções são preenchidas com dados de teste de um arquivo pré-criado e, naturalmente, após adicionar elementos, as coleções têm o mesmo tamanho.
Uso antes da verificação nula
V6060 A referência 'params' foi utilizada antes de ser verificada em relação a 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;
}A primeira linha do método define y como params.getY () . Imediatamente após a atribuição, a variável params é verificada quanto a nula . Se for permitido que os parâmetros possam ser nulos neste método , você deve ter feito essa verificação antes de usar a variável.
Check-in redundante se / senão
V6003 O uso do padrão 'if (A) {...} else if (A) {...}' foi detectado. Existe uma probabilidade de presença de erro lógico. 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;
} ....
}
}Na instrução if / else, o valor da string args é verificado duas vezes quanto à igualdade com a string "- keyStoreType ". Naturalmente, a segunda verificação é redundante e não faz sentido. No entanto, isso não parece um erro, uma vez que não há outros parâmetros no texto de ajuda do argumento da linha de comando que não sejam processados no bloco if / else . Este é provavelmente um código redundante que deve ser removido.
O método retorna o mesmo valor
V6014 É estranho que esse método sempre retorne um e o mesmo valor. 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;
}
}
}O analisador emite um aviso porque este método sempre retorna o mesmo. O comentário do método diz que dependendo se a assinatura é gerada ou não, a última chave gerada deve ser retornada ou a próxima. Aparentemente, esse método pareceu suspeito ao analisador por um motivo.
Vamos resumir
Como você pode ver, ainda conseguimos encontrar erros no projeto Bouncy Castle, e isso apenas confirma mais uma vez que nenhum de nós escreve código perfeito. Vários fatores podem funcionar: o desenvolvedor está cansado, desatento, alguém o distraiu ... Enquanto o código for escrito por uma pessoa, erros sempre ocorrerão.
Conforme os projetos crescem, encontrar problemas no código se torna cada vez mais difícil. Portanto, no mundo moderno, a análise estática de código não é apenas outra metodologia, mas uma necessidade real. Mesmo se você usar revisão de código, TDD ou análise dinâmica, isso não significa que você pode recusar a análise estática. São metodologias completamente diferentes que se complementam perfeitamente.
Ao tornar a análise estática um dos estágios de desenvolvimento, você tem a oportunidade de encontrar erros quase que imediatamente, no momento de escrever o código. Como resultado, os desenvolvedores não precisarão gastar horas depurando esses erros. Os testadores terão que reproduzir muito menos bugs elusivos. Os usuários receberão um programa mais confiável e estável.
Em geral, certifique-se de usar análise estática em seus projetos! Nós mesmos fazemos isso e recomendamos para você :)
Se você deseja compartilhar este artigo com um público que fala inglês, use o link de tradução: Irina Polynkina. Unicórnios em guarda para sua segurança: explorando o código do castelo inflável .