Verificamos os códigos-fonte do Android usando o PVS-Studio, ou ninguém é perfeito

A.
O desenvolvimento de grandes projetos complexos é impossível sem o uso de metodologias e ferramentas de programação para ajudar a controlar a qualidade do código. Antes de tudo, é um padrão de codificação competente, revisões de código, testes de unidade, analisadores de código estático e dinâmico. Tudo isso ajuda a identificar defeitos no código nos estágios iniciais de desenvolvimento. Este artigo demonstra os recursos do analisador estático do PVS-Studio para detectar erros e possíveis vulnerabilidades no código do sistema operacional Android. Esperamos que o artigo atraia leitores para a metodologia de análise de código estática e que eles desejem implementá-la no processo de desenvolvimento de seus próprios projetos.

1. Introdução

Um ano se passou desde a redação de um grande artigo sobre erros no sistema operacional Tizen, e eu queria novamente conduzir um estudo igualmente interessante de algum tipo de sistema operacional. A escolha caiu no Android.

O código do sistema operacional Android é de qualidade, bem testado. Durante o desenvolvimento, pelo menos um analisador de código estático do Coverity é usado, conforme evidenciado pelos comentários do formulário:

/* Coverity: [FALSE-POSITIVE error] intended fall through */ /* Missing break statement between cases in switch statement */ /* fall through */ 

Em geral, este é um projeto interessante e de alta qualidade, e encontrar erros nele é um desafio para o nosso analisador estático PVS-Studio.

Penso que apenas pelo volume do artigo o leitor entende que o analisador PVS-Studio fez um excelente trabalho e encontrou muitos defeitos no código deste sistema operacional.

Enumeração de fraqueza comum

No artigo, você encontrará referências à Enumeração de Fraqueza Comum (CWE). Explicamos o motivo da referência a esta lista e por que é importante do ponto de vista da segurança.

Freqüentemente, a causa das vulnerabilidades nos programas não é um conjunto complicado de circunstâncias, mas um erro banal de software. Aqui será apropriado citar esta citação de prqa.com :

O Instituto Nacional de Padrões e Tecnologia (NIST) relata que 64% das vulnerabilidades de software decorrem de erros de programação e não da falta de recursos de segurança.

Você pode ler no artigo “ Como o PVS-Studio pode ajudar a encontrar vulnerabilidades? ” Com alguns exemplos de erros simples que levaram a vulnerabilidades em projetos como MySQL, iOS, NAS, illumos-gate.

Consequentemente, muitas vulnerabilidades podem ser eliminadas detectando erros comuns a tempo e corrigindo-os. E aqui a Enumeração Comum de Fraqueza entra em cena.

Os erros são diferentes e nem todos os erros são perigosos do ponto de vista da segurança. Os erros que podem causar vulnerabilidades são coletados na Enumeração de Fraqueza Comum. Esta lista está sendo atualizada e provavelmente há erros que podem levar a vulnerabilidades, mas elas ainda não foram incluídas nesta lista.

No entanto, se o erro for classificado de acordo com a CWE, significa que é teoricamente possível que possa ser explorado como uma vulnerabilidade ( CVE ). Sim, a probabilidade disso é pequena. Muito raramente, o CWE se transforma em CVE. No entanto, se você deseja proteger seu código contra vulnerabilidades, você deve, se possível, encontrar o maior número possível de erros descritos no CWE e corrigi-los.

Esquematicamente, o relacionamento entre PVS-Studio, erros, CWE e CVE é mostrado na figura:

Alguns erros são classificados como CWE. O PVS-Studio pode detectar muitos desses erros, impedindo que alguns desses defeitos se tornem vulnerabilidades (CVE).

Podemos dizer que o PVS-Studio identifica muitas vulnerabilidades em potencial antes que elas causem danos. Assim, o PVS-Studio é uma ferramenta estática de teste de segurança de aplicativos ( SAST ).

Agora, penso, está claro por que, ao descrever os erros, considerei importante observar como eles são classificados de acordo com a CWE. Dessa forma, é mais fácil mostrar a importância do uso de um analisador de código estático em projetos críticos, aos quais os sistemas operacionais se relacionam claramente.

Verificação Android

Para analisar o projeto, usei o analisador PVS-Studio versão 6.24. Atualmente, o analisador suporta os seguintes idiomas e compiladores:

• Windows Visual Studio 2010-2017 C, C ++, C ++ / CLI, C ++ / CX (WinRT), C #
• Windows IAR Embedded Workbench, Compilador C / C ++ para ARM C, C ++
• Windows / Linux Keil µVision, DS-MDK, Compilador ARM 5/6 C, C ++
• Windows / Linux Texas Instruments Code Composer Studio, Ferramentas de geração de código ARM C, C ++
• Windows / Linux / macOS. Clang C, C ++
• Linux / macOS. GCC C, C ++
• Windows MinGW C, C ++

Nota Talvez alguns de nossos leitores tenham perdido as notícias de que apoiamos o trabalho no ambiente macOS e estejam interessados ​​nesta publicação: " Lançamento do PVS-Studio para macOS: 64 pontos fracos no Apple XNU Kernel ".

O processo de verificação do código-fonte do Android não foi um problema, por isso não vou insistir nele. O problema, pelo contrário, era minha preocupação com outras tarefas, motivo pelo qual não encontrei tempo e energia para analisar o relatório com o máximo cuidado que gostaria. No entanto, mesmo uma rápida olhada foi mais que suficiente para reunir uma grande coleção de erros interessantes para um artigo sólido.

Mais importante: peço aos desenvolvedores do Android não apenas para corrigir os erros descritos no artigo, mas também para realizar uma análise independente mais cuidadosa. Olhei para o relatório do analisador superficialmente e poderia ter perdido muitos erros sérios.

No primeiro teste, o analisador produzirá muitos falsos positivos, mas isso não é um problema . Nossa equipe está pronta para ajudar com recomendações sobre a configuração do analisador para reduzir o número de falsos positivos. Também estamos prontos para fornecer uma chave de licença por um mês ou mais, se necessário. Em geral, escreva-nos , ajudaremos e informaremos.

Agora vamos ver quais erros e possíveis vulnerabilidades eu consegui encontrar. Espero que você goste do que o analisador de código estático do PVS-Studio pode encontrar. Boa leitura.

Comparações sem sentido

O analisador considera as expressões anormais se forem sempre verdadeiras ou falsas. Esses avisos, de acordo com a Enumeração de Fraqueza Comum, são classificados como:

• CWE-570 : A expressão é sempre falsa
• CWE-571 : Expressão é sempre verdadeira

O analisador gera muitos desses avisos e, infelizmente, a maioria deles é falsa para o código do Android. Nesse caso, o analisador não é o culpado. Apenas o código está escrito assim.
Vou demonstrar isso com um exemplo simples.

 #if GENERIC_TARGET const char alternative_config_path[] = "/data/nfc/"; #else const char alternative_config_path[] = ""; #endif CNxpNfcConfig& CNxpNfcConfig::GetInstance() { .... if (alternative_config_path[0] != '\0') { .... } 

Aqui, o analisador gera um aviso: A expressão V547 CWE-570 'alternative_config_path [0]! =' \ 0 '' é sempre falsa. phNxpConfig.cpp 401

O fato é que a macro GENERIC_TARGET não está definida e, do ponto de vista do analisador, o código se parece com o seguinte:

 const char alternative_config_path[] = ""; .... if (alternative_config_path[0] != '\0') { 

O analisador é simplesmente obrigado a emitir um aviso, uma vez que a linha está vazia e um terminal zero está sempre localizado no deslocamento de zero. Assim, o analisador está formalmente certo ao emitir um aviso. No entanto, do ponto de vista prático, não há benefício com esse aviso.

Infelizmente, nada pode ser feito com tais situações. Teremos que revisar sistematicamente todos esses avisos e marcar muitos lugares como falsos positivos, para que o analisador não emita mais mensagens nessas linhas. Isso realmente precisa ser feito, porque, além de mensagens sem sentido, muitos defeitos reais serão encontrados.

Admito sinceramente que não estava interessado em examinar atentamente avisos desse tipo e os revisei superficialmente. No entanto, mesmo isso será suficiente para mostrar que esses diagnósticos são muito úteis e encontram erros interessantes.

Quero começar com a situação clássica quando a função de comparar dois objetos é implementada incorretamente. Por que clássico? Esse é um padrão de erro típico que encontramos constantemente em uma variedade de projetos. Provavelmente, existem três razões para sua ocorrência:

1. As funções de comparação são simples e são escritas “automaticamente” e usando a tecnologia Copy-Paste. Ao escrever esse código, uma pessoa é desatenta e, muitas vezes, faz erros de digitação.
2. Normalmente, a revisão de código não é executada para essas funções, pois é muito preguiçoso olhar para funções simples e chatas.
3. Testes de unidade geralmente não são realizados para essas funções. Por preguiça. Além disso, as funções são simples e os programadores não acreditam que sejam possíveis erros nelas.

Esses pensamentos e exemplos são descritos em mais detalhes no artigo "O mal vive nas funções de comparação ".

 static inline bool isAudioPlaybackRateEqual( const AudioPlaybackRate &pr1, const AudioPlaybackRate &pr2) { return fabs(pr1.mSpeed - pr2.mSpeed) < AUDIO_TIMESTRETCH_SPEED_MIN_DELTA && fabs(pr1.mPitch - pr2.mPitch) < AUDIO_TIMESTRETCH_PITCH_MIN_DELTA && pr2.mStretchMode == pr2.mStretchMode && pr2.mFallbackMode == pr2.mFallbackMode; } 

Portanto, diante de nós está a função clássica de comparar dois objetos do tipo AudioPlaybackRate . E, como penso, o leitor acha que está errado. O analisador PVS-Studio percebe dois erros de digitação imediatamente:

• V501 CWE-571 Existem subexpressões idênticas à esquerda e à direita do operador '==': pr2.mStretchMode == pr2.mStretchMode AudioResamplerPublic.h 107
• V501 CWE-571 Existem subexpressões idênticas à esquerda e à direita do operador '==': pr2.mFallbackMode == pr2.mFallbackMode AudioResamplerPublic.h 108

O campo pr2.mStretchMode e o campo pr2.mFallbackMode são comparados entre si. Acontece que a função não compara objetos com precisão suficiente.

A seguinte comparação inútil reside em uma função que armazena informações de impressão digital em um arquivo.

 static void saveFingerprint(worker_thread_t* listener, int idx) { .... int ns = fwrite(&listener->secureid[idx], sizeof(uint64_t), 1, fp); .... int nf = fwrite(&listener->fingerid[idx], sizeof(uint64_t), 1, fp); if (ns != 1 || ns !=1) // <= ALOGW("Corrupt emulator fingerprints storage; " "could not save fingerprints"); fclose(fp); return; } 

Uma anomalia é detectada neste código por dois diagnósticos ao mesmo tempo:

• V501 CWE-570 Existem subexpressões idênticas à esquerda e à direita do '||' operador: ns! = 1 || ns! = 1 impressão digital.c 126
• V560 CWE-570 Uma parte da expressão condicional é sempre falsa: ns! = 1. fingerprint.c 126

Não há como lidar com a situação quando a segunda chamada para a função de gravação não pode gravar dados em um arquivo. Em outras palavras, o valor da variável nf não é verificado. A verificação correta deve ficar assim:

 if (ns != 1 || nf != 1) 

Prosseguimos para o próximo erro associado ao uso do operador & .

 #define O_RDONLY 00000000 #define O_WRONLY 00000001 #define O_RDWR 00000002 static ssize_t verity_read(fec_handle *f, ....) { .... /* if we are in read-only mode and expect to read a zero block, skip reading and just return zeros */ if (f->mode & O_RDONLY && expect_zeros) { memset(data, 0, FEC_BLOCKSIZE); goto valid; } .... } 

Aviso do PVS-Studio: V560 CWE-570 Uma parte da expressão condicional é sempre falsa: f-> mode & 00000000. fec_read.cpp 322

Observe que a constante O_RDONLY é zero. Isso torna a expressão f-> mode & O_RDONLY inútil, como sempre é 0. Acontece que a condição da instrução if nunca é satisfeita, e a declaração true é código morto.

A verificação correta deve ser assim:

 if (f->mode == O_RDONLY && expect_zeros) { 

Agora, vamos olhar para um erro de digitação clássico quando esquecemos de escrever parte da condição.

 enum { .... CHANGE_DISPLAY_INFO = 1 << 2, .... }; void RotaryEncoderInputMapper::configure(nsecs_t when, const InputReaderConfiguration* config, uint32_t changes) { .... if (!changes || (InputReaderConfiguration::CHANGE_DISPLAY_INFO)) { .... } 

PVS-Studio Warning: V768 CWE-571 A constante de enumeração 'CHANGE_DISPLAY_INFO' é usada como uma variável do tipo booleano. InputReader.cpp 3016

A condição é sempre verdadeira, pois o operando InputReaderConfiguration :: CHANGE_DISPLAY_INFO é uma constante igual a 4.

Se você observar como o código é escrito na vizinhança, fica claro que, de fato, a condição deve ser assim:

 if (!changes || (changes & InputReaderConfiguration::CHANGE_DISPLAY_INFO)) { 

A comparação a seguir, que não faz sentido, eu conheci no operador de loop.

 void parse_printerAttributes(....) { .... ipp_t *collection = ippGetCollection(attrptr, i); for (j = 0, attrptr = ippFirstAttribute(collection); (j < 4) && (attrptr != NULL); attrptr = ippNextAttribute(collection)) { if (strcmp("....", ippGetName(attrptr)) == 0) { ....TopMargin = ippGetInteger(attrptr, 0); } else if (strcmp("....", ippGetName(attrptr)) == 0) { ....BottomMargin = ippGetInteger(attrptr, 0); } else if (strcmp("....", ippGetName(attrptr)) == 0) { ....LeftMargin = ippGetInteger(attrptr, 0); } else if (strcmp("....", ippGetName(attrptr)) == 0) { ....RightMargin = ippGetInteger(attrptr, 0); } } .... } 

Aviso do PVS-Studio: V560 CWE-571 Uma parte da expressão condicional é sempre verdadeira: (j <4). ipphelper.c 926

Observe que o valor da variável j não é incrementado em nenhum lugar. Isso significa que a subexpressão (j <4) é sempre verdadeira.

O maior número de operações úteis do analisador PVS-Studio, referentes a condições sempre verdadeiras / falsas, refere-se ao código em que o resultado da criação de objetos é verificado usando o novo operador. Em outras palavras, o analisador detecta o seguinte padrão de código:

 T *p = new T; if (p == nullptr) return ERROR; 

Tais verificações são inúteis. Se não foi possível alocar memória para o objeto, uma exceção do tipo std :: bad_alloc será lançada e ela não chegará à verificação do valor do ponteiro.

Nota O novo operador pode retornar nullptr escrevendo new (std :: nothrow) T. No entanto, isso não se aplica aos erros discutidos. O analisador PVS-Studio leva em consideração (std :: nothrow) e não emite um aviso se o objeto for criado dessa maneira.

Pode parecer que esses erros são inofensivos. Bem, pense bem, uma verificação extra que nunca funciona. De qualquer forma, será lançada uma exceção que será processada em algum lugar. Infelizmente, alguns desenvolvedores colocam na declaração true as ações if da declaração que liberam recursos etc. Como esse código não está em execução, pode levar a vazamentos de memória e outros erros.

Considere um desses casos que notei no código do Android.

 int parse_apk(const char *path, const char *target_package_name) { .... FileMap *dataMap = zip->createEntryFileMap(entry); if (dataMap == NULL) { ALOGW("%s: failed to create FileMap\n", __FUNCTION__); return -1; } char *buf = new char[uncompLen]; if (NULL == buf) { ALOGW("%s: failed to allocate %" PRIu32 " byte\n", __FUNCTION__, uncompLen); delete dataMap; return -1; } .... } 

Aviso do PVS-Studio: V668 CWE-570 Não há sentido em testar o ponteiro 'buf' contra nulo, pois a memória foi alocada usando o operador 'novo'. A exceção será gerada no caso de erro de alocação de memória. scan.cpp 213

Observe que, se não for possível alocar um segundo bloco de memória, o programador tenta liberar o primeiro bloco:

 delete dataMap; 

Esse código nunca terá controle. Este é um código morto. Se ocorrer uma exceção, ocorrerá um vazamento de memória.

Escrever esse código é fundamentalmente errado. Ponteiros inteligentes foram inventados para esses casos.

No total, o analisador PVS-Studio detectou 176 lugares no código do Android em que o ponteiro é verificado depois de criar objetos usando o novo . Não entendi o quão perigoso é cada um desses lugares e, é claro, não vou encher o artigo com todos esses avisos. Os interessados podem ver outros avisos no arquivo Android_V668.txt .

Desreferenciando um ponteiro nulo

O cancelamento da referência a um ponteiro nulo leva a um comportamento indefinido do programa, por isso é útil encontrar e corrigir esses locais. Dependendo da situação, o analisador PVS-Studio pode classificar esses erros de acordo com a Enumeração de fraqueza comum da seguinte maneira:

• CWE-119 : Restrição imprópria de operações dentro dos limites de um buffer de memória
• CWE-476 : Dereferência de ponteiro NULL
• CWE-628 : Chamada de função com argumentos especificados incorretamente
• CWE-690 : Valor de retorno não verificado para desreferência de ponteiro NULL

Costumo encontrar esses erros no código responsável por lidar com situações fora do padrão ou incorretas. Ninguém testa esse código, e o erro pode permanecer nele por um período muito longo. Apenas esse caso será considerado agora.

 bool parseEffect(....) { .... if (xmlProxyLib == nullptr) { ALOGE("effectProxy must contain a <%s>: %s", tag, dump(*xmlProxyLib)); return false; } .... } 

PVS-Studio Warning: V522 CWE-476 A desreferenciação do ponteiro nulo 'xmlProxyLib' pode ocorrer. EffectsConfig.cpp 205

Se o ponteiro xmlProxyLib for nullptr , o programador exibirá uma mensagem de depuração, para a qual é necessário desreferenciar esse ponteiro. Opa ...

Agora considere uma versão mais interessante do erro.

 static void soinfo_unload_impl(soinfo* root) { .... soinfo* needed = find_library(si->get_primary_namespace(), library_name, RTLD_NOLOAD, nullptr, nullptr); if (needed != nullptr) { // <= PRINT("warning: couldn't find %s needed by %s on unload.", library_name, si->get_realpath()); return; } else if (local_unload_list.contains(needed)) { return; } else if (needed->is_linked() && // <= needed->get_local_group_root() != root) { external_unload_list.push_back(needed); } else { unload_list.push_front(needed); } .... } 

PVS-Studio Warning: V522 CWE-476 A desreferenciação do ponteiro nulo 'necessário' pode ocorrer. linker.cpp 1847

Se o ponteiro necessário! = Nullptr , é emitido um aviso, que é um comportamento muito suspeito do programa. Finalmente, fica claro que o código contém um erro se você olhar abaixo e ver que, quando necessário == nullptr , o ponteiro nulo será desreferenciado na expressão needed-> is_linked () .

Muito provavelmente, os operadores! = E == estão simplesmente confusos aqui. Se você substituir, o código da função faz sentido e o erro desaparece.

A maior parte dos avisos sobre a desreferenciação potencial de um ponteiro nulo se refere a uma situação do formulário:

 T *p = (T *) malloc (N); *p = x; 

Funções como malloc , strdup etc. podem retornar NULL se a memória não puder ser alocada. Portanto, não é possível remover a referência de ponteiros que retornam essas funções sem primeiro verificar o ponteiro.

Existem muitos erros semelhantes, então darei apenas dois fragmentos de código simples: o primeiro com
malloc e o segundo com strdup .

 DownmixerBufferProvider::DownmixerBufferProvider(....) { .... effect_param_t * const param = (effect_param_t *) malloc(downmixParamSize); param->psize = sizeof(downmix_params_t); .... } 

Aviso do PVS-Studio: V522 CWE-690 Pode haver desreferenciação de um potencial ponteiro nulo 'param'. Verifique as linhas: 245, 244. BufferProviders.cpp 245

 static char* descriptorClassToDot(const char* str) { .... newStr = strdup(lastSlash); newStr[strlen(lastSlash)-1] = '\0'; .... } 

Aviso do PVS-Studio: V522 CWE-690 Pode haver desreferenciação de um ponteiro nulo potencial 'newStr'. Verifique as linhas: 203, 202. DexDump.cpp 203

Alguém pode dizer que esses são erros menores. Se não houver memória suficiente, o programa simplesmente trava ao remover a referência do ponteiro nulo, e isso é normal. Como não há memória, não há nada para tentar lidar de alguma forma com essa situação.

Essa pessoa está errada. Os ponteiros devem ser verificados! Examinei esse tópico em detalhes no artigo " Por que é importante verificar o que a função malloc retornou ". Eu recomendo que você o leia para todos que ainda não o leram.

Em suma, o perigo é que a gravação na memória não ocorra necessariamente perto do endereço zero. É possível gravar dados em algum lugar muito distante em uma página de memória que não esteja protegida contra gravação e, assim, causar um erro indescritível ou, em geral, esse erro pode ser usado como uma vulnerabilidade. Vamos ver o que quero dizer com o exemplo da função check_size .

 int check_size(radio_metadata_buffer_t **metadata_ptr, const uint32_t size_int) { .... metadata = realloc(metadata, new_size_int * sizeof(uint32_t)); memmove( (uint32_t *)metadata + new_size_int - (metadata->count + 1), (uint32_t *)metadata + metadata->size_int - (metadata->count + 1), (metadata->count + 1) * sizeof(uint32_t)); .... } 

Aviso do PVS-Studio: V769 CWE-119 O ponteiro '(uint32_t *) metadados' na expressão '(uint32_t *) metadados + expressão new_size_int' pode ser nullptr. Nesse caso, o valor resultante será insensato e não deve ser usado. Verifique as linhas: 91, 89. radio_metadata.c 91

Eu não entendi a lógica da função, mas isso não é necessário. O principal é que um novo buffer é criado e os dados são copiados para ele. Se a função realloc retornar NULL , os dados serão copiados para o endereço ((uint32_t *) NULL + metadados-> tamanho_int - (metadados-> contagem + 1)).

Se o valor de metadados-> size_int for grande, as consequências serão lamentáveis. Acontece que os dados são gravados em um pedaço aleatório de memória.

A propósito, existe outro tipo de cancelamento de referência de ponteiro nulo, que o analisador PVS-Studio classifica não como CWE-690, mas como CWE-628 (argumento inválido).

 static void parse_tcp_ports(const char *portstring, uint16_t *ports) { char *buffer; char *cp; buffer = strdup(portstring); if ((cp = strchr(buffer, ':')) == NULL) .... } 

Aviso do PVS-Studio: V575 CWE-628 O potencial ponteiro nulo é passado para a função 'strchr'. Inspecione o primeiro argumento. Verifique as linhas: 47, 46. libxt_tcp.c 47

O fato é que a desreferenciação do ponteiro ocorrerá quando a função strchr for chamada . Portanto, o analisador interpreta essa situação como passando um valor incorreto para a função.

Os 194 avisos restantes deste tipo I são listados no arquivo Android_V522_V575.txt .

A propósito, uma atenção especial a todos esses erros é dada pelos avisos discutidos anteriormente sobre a verificação do ponteiro após chamar o novo operador. Acontece que existem 195 chamadas para as funções malloc / realloc / strdup e assim por diante, quando o ponteiro não está marcado. Mas há 176 lugares em que o ponteiro é verificado depois de chamar de novo . Concordo, uma abordagem estranha!

No final, resta considerar os avisos V595 e V1004, que também estão associados ao uso de ponteiros nulos.

O V595 detecta situações em que o ponteiro é desreferenciado e depois verificado. Exemplo sintético:

 p->foo(); if (!p) Error(); 

O V1004 revela a situação inversa quando o ponteiro foi verificado primeiro e depois se esqueceu de fazê-lo. Exemplo sintético:

 if (p) p->foo(); p->doo(); 

Vejamos alguns fragmentos do código Android, onde houve erros desse tipo. Explique especialmente que seus recursos não são necessários.

 PV_STATUS RC_UpdateBuffer(VideoEncData *video, Int currLayer, Int num_skip) { rateControl *rc = video->rc[currLayer]; MultiPass *pMP = video->pMP[currLayer]; if (video == NULL || rc == NULL || pMP == NULL) return PV_FAIL; .... } 

Aviso do PVS-Studio: V595 CWE-476 O ponteiro 'video' foi utilizado antes de ser verificado no nullptr. Verifique as linhas: 385, 388. rate_control.cpp 385

 static void resampler_reset(struct resampler_itfe *resampler) { struct resampler *rsmp = (struct resampler *)resampler; rsmp->frames_in = 0; rsmp->frames_rq = 0; if (rsmp != NULL && rsmp->speex_resampler != NULL) { speex_resampler_reset_mem(rsmp->speex_resampler); } } 

Aviso do PVS-Studio: V595 CWE-476 O ponteiro 'rsmp' foi utilizado antes de ser verificado no nullptr. Verifique as linhas: 54, 57. resampler.c 54

 void bta_gattc_disc_cmpl(tBTA_GATTC_CLCB* p_clcb, UNUSED_ATTR tBTA_GATTC_DATA* p_data) { .... if (p_clcb->status != GATT_SUCCESS) { if (p_clcb->p_srcb) { std::vector<tBTA_GATTC_SERVICE>().swap( p_clcb->p_srcb->srvc_cache); } bta_gattc_cache_reset(p_clcb->p_srcb->server_bda); } .... } 

Aviso do PVS-Studio: V1004 CWE-476 O ponteiro 'p_clcb-> p_srcb' foi usado sem segurança depois de ter sido verificado com nullptr. Verifique as linhas: 695, 701. bta_gattc_act.cc 701

Não é interessante considerar outros avisos desse tipo. Entre eles, existem erros e avisos falsos que surgem devido a códigos mal ou dificilmente escritos.

Escrevi uma dúzia de avisos úteis:

• V1004 CWE-476 The 'ain' pointer was used unsafely after it was verified against nullptr. Check lines: 101, 105. rsCpuIntrinsicBLAS.cpp 105
• V595 CWE-476 The 'outError' pointer was utilized before it was verified against nullptr. Check lines: 437, 450. Command.cpp 437
• V595 CWE-476 The 'out_last_reference' pointer was utilized before it was verified against nullptr. Check lines: 432, 436. AssetManager2.cpp 432
• V595 CWE-476 The 'set' pointer was utilized before it was verified against nullptr. Check lines: 4524, 4529. ResourceTypes.cpp 4524
• V595 CWE-476 The 'reply' pointer was utilized before it was verified against nullptr. Check lines: 126, 133. Binder.cpp 126
• V595 CWE-476 The 'video' pointer was utilized before it was verified against nullptr. Check lines: 532, 540. rate_control.cpp 532
• V595 CWE-476 The 'video' pointer was utilized before it was verified against nullptr. Check lines: 702, 711. rate_control.cpp 702
• V595 CWE-476 The 'pInfo' pointer was utilized before it was verified against nullptr. Check lines: 251, 254. ResolveInfo.cpp 251
• V595 CWE-476 The 'address' pointer was utilized before it was verified against nullptr. Check lines: 53, 55. DeviceHalHidl.cpp 53
• V595 CWE-476 The 'halAddress' pointer was utilized before it was verified against nullptr. Check lines: 55, 82. DeviceHalHidl.cpp 55

E então fiquei entediado e filtrei avisos desse tipo. Portanto, eu nem sei quantos erros reais o analisador detectou. Esses avisos aguardam seu herói, que os estudará cuidadosamente e fará alterações no código.

Eu só quero chamar a atenção dos meus novos leitores para erros desse tipo:

 NJ_EXTERN NJ_INT16 njx_search_word(NJ_CLASS *iwnn, ....) { .... NJ_PREVIOUS_SELECTION_INFO *prev_info = &(iwnn->previous_selection); if (iwnn == NULL) { return NJ_SET_ERR_VAL(NJ_FUNC_NJ_SEARCH_WORD, NJ_ERR_PARAM_ENV_NULL); } .... } 

Aviso do PVS-Studio: V595 CWE-476 O ponteiro 'iwnn' foi utilizado antes de ser verificado no nullptr. Verifique as linhas: 686, 689. ndapi.c 686

Algumas pessoas pensam que não há erro aqui, porque "não há desreferência real do ponteiro". O endereço de uma variável inexistente é simplesmente calculado. Além disso, se o ponteiro iwnn for zero, a função será encerrada. Portanto, nada de ruim aconteceu que anteriormente calculamos incorretamente o endereço de um membro da classe.

Não, você não pode falar assim. Esse código leva a um comportamento indefinido e, portanto, não pode ser escrito assim. O comportamento indefinido pode se manifestar, por exemplo, da seguinte maneira:

1. O compilador vê o ponteiro desreferenciado: iwnn-> previous_selection
2. Você não pode desreferenciar um ponteiro nulo, porque esse é um comportamento indefinido
3. O compilador conclui que o ponteiro iwnn é sempre diferente de zero
4. O compilador remove a verificação extra: if (iwnn == NULL)
5. Agora, quando o programa é executado, a verificação do ponteiro nulo não é executada e o trabalho começa com o ponteiro incorreto para o membro da classe.

Este tópico é descrito em mais detalhes no meu artigo "O cancelamento da referência a um ponteiro nulo leva a um comportamento indefinido ".

Dados privados não são apagados na memória

Considere um tipo sério de vulnerabilidade potencial que é classificada de acordo com a Enumeração de fraqueza comum como CWE-14 : Remoção de código do compilador para limpar buffers.

Em resumo, a conclusão é a seguinte: o compilador tem o direito de remover a chamada de função memset se depois disso o buffer não for mais usado.

Quando escrevo sobre esse tipo de vulnerabilidade, os comentários parecem necessariamente que essa é apenas uma falha no compilador que precisa ser corrigida. Não, isso não é uma falha. E antes de se opor, leia os seguintes materiais:

• Roman Fomichev. Dados privados limpos com segurança .
• Enumeração de fraqueza comum. CWE-14 .
• Descrição do diagnóstico V597 .

Em geral, tudo é sério. Existem tais erros no Android? Claro que existe.Geralmente são muitos onde existem: provas :).

Vamos voltar ao código do Android e ver o início e o fim da função FwdLockGlue_InitializeRoundKeys , pois o meio não é interessante para nós.

 static void FwdLockGlue_InitializeRoundKeys() { unsigned char keyEncryptionKey[KEY_SIZE]; .... memset(keyEncryptionKey, 0, KEY_SIZE); // Zero out key data. } 

PVS-Studio Warning: V597 CWE-14 O compilador pode excluir a chamada de função 'memset', usada para liberar o buffer 'keyEncryptionKey'. A função memset_s () deve ser usada para apagar os dados privados. FwdLockGlue.c 102

A matriz keyEncryptionKey é criada na pilha e armazena informações particulares. No final da função, eles desejam preencher essa matriz com zeros para que não chegue acidentalmente onde não deveria. Como as informações podem chegar onde não deveriam, o artigo " Substituir memória - por quê? ", Dirá .

Para preencher uma matriz com informações privadas com zeros, a função memset é usada . O comentário "Zerar dados importantes" confirma que entendemos tudo corretamente.

O problema é que, com uma probabilidade muito alta, o compilador excluirá a chamada de função memset ao criar a versão de lançamento . Como o buffer após a chamada memset não é usado, a chamada da função memset do ponto de vista do compilador é supérflua.

Mais 10 avisos eu escrevi um arquivo Android_V597.txt .

Houve outro erro em que a memória não foi substituída, embora neste caso a função memset não tenha nada a ver com isso.

 void SHA1Transform(uint32_t state[5], const uint8_t buffer[64]) { uint32_t a, b, c, d, e; .... /* Wipe variables */ a = b = c = d = e = 0; } 

PVS-Studio Warning: V1001 CWE-563 A variável 'a' é atribuída, mas não é usada até o final da função. sha1.c 213 O

PVS-Studio detectou uma anomalia relacionada ao fato de que, após atribuir valores às variáveis, elas não são mais usadas. O analisador classificou esse defeito como CWE-563 : Atribuição a variável sem uso. E, formalmente, ele está certo, embora, de fato, aqui estamos novamente lidando com o CWE-14. O compilador descartará essas atribuições, pois, do ponto de vista de C e C ++, elas são supérfluas. Como resultado, a pilha irá permanecer os mesmos valores das variáveis de um , b , c , d e e , o armazenamento de dados particular.

Comportamento não especificado / definido pela implementação

Enquanto você não estiver cansado, vejamos um caso complexo que exigirá uma descrição completa da minha parte.

 typedef int32_t GGLfixed; GGLfixed gglFastDivx(GGLfixed n, GGLfixed d) { if ((d>>24) && ((d>>24)+1)) { n >>= 8; d >>= 8; } return gglMulx(n, gglRecip(d)); } 

Aviso do PVS-Studio: V793 É estranho que o resultado da instrução '(d >> 24) + 1' faça parte da condição. Talvez essa afirmação deva ter sido comparada com outra coisa. fixed.cpp 75

programador queria verificar que os 8 bits superiores da variável d contém a unidade, mas não todos os bits de uma vez. Em outras palavras, o programador queria verificar se qualquer valor diferente de 0x00 e 0xFF está no byte alto.

Ele abordou a solução desse problema desnecessariamente de forma criativa. Para começar, ele verificou que os bits mais significativos são diferentes de zero escrevendo uma expressão (d >> 24). Existem reivindicações para essa expressão, mas é mais interessante analisar o lado direito da expressão: ((d >> 24) +1). O programador muda os oito bits altos para o byte baixo. Ao mesmo tempo, calcula que o bit de sinal mais significativo é duplicado em todos os outros bits. I.e.se a variável d for 0b11111111'00000000'00000000'00000000, depois do turno obteremos o valor 0b11111111'11111111'11111111'11111111. Adicionando 1 ao valor 0xFFFFFFFF do tipo int , o programador planeja obter 0. Ou seja: -1 + 1 = 0. Assim, usando a expressão ((d >> 24) +1), ele verifica se nem todos os oito bits altos são iguais a 1. Entendo que isso é bastante difícil, portanto, não se apresse e tente descobrir como e o que funciona :).

Agora vamos ver o que há de errado com este código.

Ao mudar, o bit de sinal mais significativo não é necessariamente "manchado". Aqui está o que o padrão diz sobre isso: “O valor de E1 >> E2 é E1 com posições de bits E2 deslocadas à direita. Se E1 tiver um tipo não assinado ou E1 tiver um tipo assinado e um valor não negativo, o valor do resultado será parte integrante do quociente E1 / 2 ^ E2. Se E1 tiver um tipo assinado e um valor negativo, o valor resultante será definido pela implementação. "

A última oferta é importante para nós. Então, conhecemos o comportamento definido pela implementação. O funcionamento desse código depende da arquitetura do microprocessador e da implementação do compilador. Após a mudança, os zeros podem aparecer nos bits mais significativos e, em seguida, a expressão ((d >> 24) +1) será sempre diferente de 0, ou seja, sempre será um valor verdadeiro.

Daí a conclusão: não é preciso ser sábio. O código se tornará mais confiável e compreensível se você escrever, por exemplo, assim:

 GGLfixed gglFastDivx(GGLfixed n, GGLfixed d) { uint32_t hibits = static_cast<uint32_t>(d) >> 24; if (hibits != 0x00 && hibits != 0xFF) { n >>= 8; d >>= 8; } return gglMulx(n, gglRecip(d)); } 

Provavelmente, sugeri não a opção ideal, mas neste código não há comportamento definido pela implementação, e será mais fácil para o leitor entender o que está sendo verificado.

Você merece uma xícara de café ou chá. Distraia e continue: estamos aguardando um caso interessante de comportamento não especificado.

Na entrevista, como uma das primeiras perguntas ao solicitante, pergunto o seguinte: "O que a função printf imprimirá e por quê?"

 int i = 5; printf("%d,%d", i++, i++) 

A resposta correta: esse é um comportamento não especificado. O procedimento para calcular os argumentos reais ao chamar a função não está definido. Ocasionalmente, até demonstro que esse código, compilado com a ajuda do Visual C ++, exibe "6,5" na tela, o que confunde os recém-chegados que são fracos em conhecimento e espírito a uma paralisação total :)

Isso pode parecer um problema absurdo. Mas não, esse código pode ser encontrado em aplicativos sérios, por exemplo, no código do Android.

 bool ComposerClient::CommandReader::parseSetLayerCursorPosition( uint16_t length) { if (length != CommandWriterBase::kSetLayerCursorPositionLength) { return false; } auto err = mHal.setLayerCursorPosition(mDisplay, mLayer, readSigned(), readSigned()); if (err != Error::NONE) { mWriter.setError(getCommandLoc(), err); } return true; } 

PVS-Studio Warning: V681 CWE-758 O padrão da linguagem não define uma ordem na qual as funções 'readSigned' serão chamadas durante a avaliação dos argumentos. ComposerClient.cpp 836

Estamos interessados ​​nesta linha de código:

 mHal.setLayerCursorPosition(...., readSigned(), readSigned()); 

Chamando a função readSigned , dois valores são lidos. Mas é impossível prever em que ordem os valores serão lidos. Este é um caso clássico de comportamento não especificado.

Benefícios do uso de um analisador de código estático

Este artigo inteiro populariza a análise de código estático em geral e nossa ferramenta PVS-Studio em particular. No entanto, alguns erros são perfeitos para demonstrar as possibilidades de análise estática. Eles são difíceis de identificar por revisões de código e apenas um programa não cansado os nota facilmente. Considere alguns desses casos.

 const std::map<std::string, int32_t> kBootReasonMap = { .... {"watchdog_sdi_apps_reset", 106}, {"smpl", 107}, {"oem_modem_failed_to_powerup", 108}, {"reboot_normal", 109}, {"oem_lpass_cfg", 110}, // <= {"oem_xpu_ns_error", 111}, // <= {"power_key_press", 112}, {"hardware_reset", 113}, {"reboot_by_powerkey", 114}, {"reboot_verity", 115}, {"oem_rpm_undef_error", 116}, {"oem_crash_on_the_lk", 117}, {"oem_rpm_reset", 118}, {"oem_lpass_cfg", 119}, // <= {"oem_xpu_ns_error", 120}, // <= {"factory_cable", 121}, {"oem_ar6320_failed_to_powerup", 122}, {"watchdog_rpm_bite", 123}, {"power_on_cable", 124}, {"reboot_unknown", 125}, .... }; 

Avisos do PVS-Studio:

• V766 CWE-462 Um item com a mesma chave '"oem_lpass_cfg"' já foi adicionado. bootstat.cpp 264
• V766 CWE-462 Um item com a mesma chave '"oem_xpu_ns_error"' já foi adicionado. bootstat.cpp 265

Valores diferentes com as mesmas chaves são inseridos no contêiner associativo classificado ( std :: map ). Em termos de enumeração de fraqueza comum, esta é a CWE-462 : chave duplicada na lista associativa.

O texto do programa é reduzido e os erros são marcados com comentários; portanto, o erro parece óbvio, mas quando você lê esse código apenas com os olhos, é muito difícil encontrar esses erros.

Considere outro trecho de código que é muito difícil de ler, pois é do mesmo tipo e desinteressante.

 MtpResponseCode MyMtpDatabase::getDevicePropertyValue(....) { .... switch (type) { case MTP_TYPE_INT8: packet.putInt8(longValue); break; case MTP_TYPE_UINT8: packet.putUInt8(longValue); break; case MTP_TYPE_INT16: packet.putInt16(longValue); break; case MTP_TYPE_UINT16: packet.putUInt16(longValue); break; case MTP_TYPE_INT32: packet.putInt32(longValue); break; case MTP_TYPE_UINT32: packet.putUInt32(longValue); break; case MTP_TYPE_INT64: packet.putInt64(longValue); break; case MTP_TYPE_UINT64: packet.putUInt64(longValue); break; case MTP_TYPE_INT128: packet.putInt128(longValue); break; case MTP_TYPE_UINT128: packet.putInt128(longValue); // <= break; .... } 

Aviso do PVS-Studio: V525 CWE-682 O código contém a coleção de blocos semelhantes. Verifique os itens 'putInt8', 'putUInt8', 'putInt16', 'putUInt16', 'putInt32', 'putUInt32', 'putInt64', 'putUInt64', 'putInt128', 'putInt128' nas linhas 620, 623, 626, 629 , 632, 635, 638, 641, 644, 647. android_mtp_MtpDatabase.cpp 620

Se MTP_TYPE_UINT128 teve que ser causada por função putUInt128 , não putInt128 .

E o último desta seção é um ótimo exemplo de Copy-Paste malsucedido.

 static void btif_rc_upstreams_evt(....) { .... case AVRC_PDU_REQUEST_CONTINUATION_RSP: { BTIF_TRACE_EVENT( "%s() REQUEST CONTINUATION: target_pdu: 0x%02d", __func__, pavrc_cmd->continu.target_pdu); tAVRC_RESPONSE avrc_rsp; if (p_dev->rc_connected == TRUE) { memset(&(avrc_rsp.continu), 0, sizeof(tAVRC_NEXT_RSP)); avrc_rsp.continu.opcode = opcode_from_pdu(AVRC_PDU_REQUEST_CONTINUATION_RSP); avrc_rsp.continu.pdu = AVRC_PDU_REQUEST_CONTINUATION_RSP; avrc_rsp.continu.status = AVRC_STS_NO_ERROR; avrc_rsp.continu.target_pdu = pavrc_cmd->continu.target_pdu; send_metamsg_rsp(p_dev, -1, label, ctype, &avrc_rsp); } } break; case AVRC_PDU_ABORT_CONTINUATION_RSP: { BTIF_TRACE_EVENT( "%s() ABORT CONTINUATION: target_pdu: 0x%02d", __func__, pavrc_cmd->abort.target_pdu); tAVRC_RESPONSE avrc_rsp; if (p_dev->rc_connected == TRUE) { memset(&(avrc_rsp.abort), 0, sizeof(tAVRC_NEXT_RSP)); avrc_rsp.abort.opcode = opcode_from_pdu(AVRC_PDU_ABORT_CONTINUATION_RSP); avrc_rsp.abort.pdu = AVRC_PDU_ABORT_CONTINUATION_RSP; avrc_rsp.abort.status = AVRC_STS_NO_ERROR; avrc_rsp.abort.target_pdu = pavrc_cmd->continu.target_pdu; send_metamsg_rsp(p_dev, -1, label, ctype, &avrc_rsp); } } break; .... } 

Antes de ler o aviso do analisador e mais texto, sugiro procurar o erro.

Para que você não leia a resposta acidentalmente imediatamente, aqui está uma foto para você desviar a atenção. Se você está interessado no significado de um ovo com a inscrição Java, aqui está você .

Espero que você tenha gostado da busca por erros de digitação. Agora é a hora de avisar o analisador: V778 CWE-682 Dois fragmentos de código semelhantes foram encontrados. Talvez seja um erro de digitação e a variável 'abort' deva ser usada em vez de 'continu'. btif_rc.cc 1554

Aparentemente, o código foi escrito usando o método Copy-Paste, e uma pessoa, como sempre, não pôde estar atenta no processo de edição do fragmento de código copiado. Como resultado, no final, ele não substituiu " continu " por " abort ".

I.e. no segundo bloco deve ser escrito:

 avrc_rsp.abort.target_pdu = pavrc_cmd->abort.target_pdu; 

Essa situação se enquadra completamente na definição de " efeito de última linha ", pois um erro na substituição de nomes foi cometido no final.

Facepalm

Um bug muito engraçado está relacionado à conversão entre os formatos de dados little-endian e big-endian (consulte Ordem dos bytes ).

 inline uint32_t bswap32(uint32_t pData) { return (((pData & 0xFF000000) >> 24) | ((pData & 0x00FF0000) >> 8) | ((pData & 0x0000FF00) << 8) | ((pData & 0x000000FF) << 24)); } bool ELFAttribute::merge(....) { .... uint32_t subsection_length = *reinterpret_cast<const uint32_t*>(subsection_data); if (llvm::sys::IsLittleEndianHost != m_Config.targets().isLittleEndian()) bswap32(subsection_length); .... } 

Aviso do PVS-Studio: V530 CWE-252 O valor de retorno da função 'bswap32' deve ser utilizado. ELFAttribute.cpp 84

Não há reclamações sobre a função bswap32 . Mas é usado incorretamente:

 bswap32(subsection_length); 

O autor sugeriu que a variável é passada para a função por referência e é alterada lá. No entanto, você deve usar o valor retornado pela função. Como resultado, nenhuma conversão de dados ocorre.

O analisador identificou esse erro como CWE-252 : Valor de retorno não verificado. Mas, de fato, é mais apropriado chamar CWE-198 : Uso de pedidos de bytes incorretos aqui. Infelizmente, o analisador não consegue entender qual é o erro do ponto de vista de alto nível. No entanto, isso não o impede de identificar esse defeito grave no código.

O código correto é:

 subsection_length = bswap32(subsection_length); 

Existem mais três lugares no código do Android com o mesmo erro:

• V530 CWE-252 O valor de retorno da função 'bswap32' deve ser utilizado. ELFAttribute.cpp 218
• V530 CWE-252 O valor de retorno da função 'bswap32' deve ser utilizado. ELFAttribute.cpp 346
• V530 CWE-252 O valor de retorno da função 'bswap32' deve ser utilizado. ELFAttribute.cpp 352

Para evitar esses erros, é recomendável usar o atributo [[nodiscard]] . Este atributo é usado para indicar que o valor de retorno de uma função deve ser usado quando invocado. Portanto, se você escrever assim:

 [[nodiscard]] inline uint32_t bswap32(uint32_t pData) { ... } 

o erro seria detectado mesmo na fase de compilação do arquivo. Você pode aprender mais sobre alguns novos atributos úteis do artigo do meu colega " C ++ 17 ".

Código inacessível

Na teoria da programação e do compilador, o código inacessível é a parte do código do programa que não pode ser executada sob nenhuma circunstância, pois é inacessível no gráfico de fluxo de controle.

Do ponto de vista da enumeração Common Weakness, é CWE-561 : Dead Code.

 virtual sp<IEffect> createEffect(....) { .... if (pDesc == NULL) { return effect; if (status != NULL) { *status = BAD_VALUE; } } .... } 

Aviso do PVS-Studio: V779 CWE-561 Código inacessível detectado. É possível que haja um erro. IAudioFlinger.cpp 733

A declaração de retorno deve estar explicitamente localizada abaixo.

Outros erros deste tipo:

• V779 CWE-561 Código inacessível detectado. É possível que haja um erro. bta_hf_client_main.cc 612
• V779 CWE-561 Código inacessível detectado. É possível que haja um erro. android_media_ImageReader.cpp 468
• V779 CWE-561 Código inacessível detectado. É possível que haja um erro. AMPEG4AudioAssembler.cpp 187

quebrar

A interrupção esquecida dentro do switch é um erro clássico dos programadores de C e C ++. Para combatê-lo, uma anotação útil apareceu no C ++ 17 como [[avanço]] . Você pode ler mais sobre esse erro e [[avanço]] no meu artigo " avanço e avanço ".

Mas enquanto o mundo está cheio de códigos antigos em que [[avanço]] não é usado, o PVS-Studio é útil para você. Considere alguns erros encontrados no Android. De acordo com a Common Weakness Enumeration, esses erros são classificados como CWE-484 : Omitted Break Statement no Switch.

 bool A2dpCodecConfigLdac::setCodecConfig(....) { .... case BTAV_A2DP_CODEC_SAMPLE_RATE_192000: if (sampleRate & A2DP_LDAC_SAMPLING_FREQ_192000) { result_config_cie.sampleRate = A2DP_LDAC_SAMPLING_FREQ_192000; codec_capability_.sample_rate = codec_user_config_.sample_rate; codec_config_.sample_rate = codec_user_config_.sample_rate; } case BTAV_A2DP_CODEC_SAMPLE_RATE_16000: case BTAV_A2DP_CODEC_SAMPLE_RATE_24000: case BTAV_A2DP_CODEC_SAMPLE_RATE_NONE: codec_capability_.sample_rate = BTAV_A2DP_CODEC_SAMPLE_RATE_NONE; codec_config_.sample_rate = BTAV_A2DP_CODEC_SAMPLE_RATE_NONE; break; .... } 

Aviso do PVS-Studio: V796 CWE-484 É possível que a instrução 'break' esteja ausente na instrução switch. a2dp_vendor_ldac.cc 912

Acho que o erro não precisa de explicação. Observo apenas que muitas vezes uma anomalia no código é detectada em mais de uma maneira. Por exemplo, esse erro também é detectado pelos avisos da V519:

• V519 CWE-563 A variável 'codec_capability_.sample_rate' recebe valores duas vezes sucessivos. Talvez isso seja um erro. Verifique as linhas: 910, 916. a2dp_vendor_ldac.cc 916
• V519 CWE-563 A variável 'codec_config_.sample_rate' recebe valores duas vezes sucessivamente. Talvez isso seja um erro. Verifique as linhas: 911, 917. a2dp_vendor_ldac.cc 917

E mais alguns erros:

 Return<void> EffectsFactory::getAllDescriptors(....) { .... switch (status) { case -ENOSYS: { // Effect list has changed. goto restart; } case -ENOENT: { // No more effects available. result.resize(i); } default: { result.resize(0); retval = Result::NOT_INITIALIZED; } } .... } 

Aviso do PVS-Studio: V796 CWE-484 É possível que a instrução 'break' esteja ausente na instrução switch. EffectsFactory.cpp 118

 int Reverb_getParameter(....) { .... case REVERB_PARAM_REFLECTIONS_LEVEL: *(uint16_t *)pValue = 0; case REVERB_PARAM_REFLECTIONS_DELAY: *(uint32_t *)pValue = 0; case REVERB_PARAM_REVERB_DELAY: *(uint32_t *)pValue = 0; break; .... } 

Aviso do PVS-Studio: V796 CWE-484 É possível que a instrução 'break' esteja ausente na instrução switch. EffectReverb.cpp 1847

 static SLresult IAndroidConfiguration_GetConfiguration(....) { .... switch (IObjectToObjectID((thiz)->mThis)) { case SL_OBJECTID_AUDIORECORDER: result = android_audioRecorder_getConfig( (CAudioRecorder *) thiz->mThis, configKey, pValueSize, pConfigValue); break; case SL_OBJECTID_AUDIOPLAYER: result = android_audioPlayer_getConfig( (CAudioPlayer *) thiz->mThis, configKey, pValueSize, pConfigValue); default: result = SL_RESULT_FEATURE_UNSUPPORTED; break; } .... } 

Aviso do PVS-Studio: V796 CWE-484 É possível que a instrução 'break' esteja ausente na instrução switch. IAndroidConfiguration.cpp 90

Gerenciamento de memória incorreto

Aqui eu compilei erros relacionados ao gerenciamento incorreto de memória. Esses avisos, de acordo com a Enumeração de Fraqueza Comum, são classificados como:

• CWE-401 : Liberação incorreta da memória antes de remover a última referência ('Vazamento de memória')
• CWE-562 : Retorno do endereço variável da pilha
• CWE-762 : Rotinas de gerenciamento de memória incompatíveis

Vamos começar com funções que retornam uma referência a uma variável já destruída.

 TransformIterator& operator++(int) { TransformIterator tmp(*this); ++*this; return tmp; } TransformIterator& operator--(int) { TransformIterator tmp(*this); --*this; return tmp; } 

Avisos do PVS-Studio:

• V558 A função CWE-562 retorna a referência ao objeto local temporário: tmp. transform_iterator.h 77
• V558 A função CWE-562 retorna a referência ao objeto local temporário: tmp. transform_iterator.h 92

Quando as funções terminam sua execução, a variável tmp é destruída, pois foi criada na pilha. Conseqüentemente, as funções retornam uma referência a um objeto já destruído (inexistente).

A solução correta seria retornar um valor de:

 TransformIterator operator++(int) { TransformIterator tmp(*this); ++*this; return tmp; } TransformIterator operator--(int) { TransformIterator tmp(*this); --*this; return tmp; } 

Considere um código ainda mais triste que merece muita atenção.

 int register_socket_transport( int s, const char* serial, int port, int local) { atransport* t = new atransport(); if (!serial) { char buf[32]; snprintf(buf, sizeof(buf), "T-%p", t); serial = buf; } .... } 

PVS-Studio Warning: V507 CWE-562 O ponteiro para a matriz local 'buf' é armazenado fora do escopo desta matriz. Esse ponteiro se tornará inválido. transport.cpp 1030

Este é um código perigoso. Se o valor real do argumento serial for NULL, um buffer temporário na pilha deve ser usado. Quando o corpo da instrução if termina, o array buf deixa de existir. O local em que o buffer foi criado pode ser usado para armazenar outras variáveis ​​criadas na pilha. Uma bagunça infernal começará nos dados, e as consequências de tal erro são mal previstas.

Os seguintes erros estão relacionados a métodos incompatíveis para criar e destruir objetos.

 void SensorService::SensorEventConnection::reAllocateCacheLocked(....) { sensors_event_t *eventCache_new; const int new_cache_size = computeMaxCacheSizeLocked(); eventCache_new = new sensors_event_t[new_cache_size]; .... delete mEventCache; mEventCache = eventCache_new; mCacheSize += count; mMaxCacheSize = new_cache_size; } 

Aviso do PVS-Studio: V611 CWE-762 A memória foi alocada usando o operador 'new T []', mas foi liberada usando o operador 'delete'. Considere inspecionar este código. Provavelmente é melhor usar 'delete [] mEventCache;'. Verifique as linhas: 391, 384. SensorEventConnection.cpp 391

Tudo é simples aqui. O buffer apontado por um membro da classe mEventCache é alocado usando o novo operador [] . E libere essa memória usando o operador de exclusão . Isso está incorreto e leva ao comportamento indefinido do programa.

Erro semelhante:

 aaudio_result_t AAudioServiceEndpointCapture::open(....) { .... delete mDistributionBuffer; int distributionBufferSizeBytes = getStreamInternal()->getFramesPerBurst() * getStreamInternal()->getBytesPerFrame(); mDistributionBuffer = new uint8_t[distributionBufferSizeBytes]; .... } 

Aviso do PVS-Studio: V611 CWE-762 A memória foi alocada usando o operador 'new T []', mas foi liberada usando o operador 'delete'. Considere inspecionar este código. Provavelmente é melhor usar 'delete [] mDistributionBuffer;'. AAudioServiceEndpointCapture.cpp 50

Acho que o erro não requer explicações.

O caso a seguir é um pouco mais interessante, mas a essência do erro é exatamente a mesma.

 struct HeifFrameInfo { .... void set(....) { .... mIccData.reset(new uint8_t[iccSize]); .... } .... std::unique_ptr<uint8_t> mIccData; }; 

V554 CWE-762 Uso incorreto de unique_ptr. A memória alocada com 'new []' será limpa usando 'delete'.HeifDecoderAPI.h 62

Por padrão, a classe de ponteiro inteligente std :: unique_ptr chama o operador de exclusão para destruir um objeto . No entanto, na função definida , a memória é alocada usando o novo operador [] .

A opção correta:

 std::unique_ptr<uint8_t[]> mIccData; 

Outros erros:

• V554 CWE-762 Uso incorreto de unique_ptr. A memória alocada com 'new []' será limpa usando 'delete'. atrace.cpp 949
• V554 CWE-762 Uso incorreto de unique_ptr. A memória alocada com 'new []' será limpa usando 'delete'. atrace.cpp 950
• V554 CWE-762 Uso incorreto de unique_ptr. A memória alocada com 'new []' será limpa usando 'delete'. HeifDecoderImpl.cpp 102
• V554 CWE-762 Uso incorreto de unique_ptr. A memória alocada com 'new []' será limpa usando 'delete'. HeifDecoderImpl.cpp 166
• V554 CWE-762 Uso incorreto de unique_ptr. A memória alocada com 'new []' será limpa usando 'delete'. ColorSpace.cpp 360

Os erros de vazamento de memória concluirão esta seção. Uma surpresa desagradável é que houve mais de 20 desses erros.Parece que esses defeitos podem ser muito dolorosos, levando a uma diminuição gradual da memória livre durante a operação contínua prolongada do sistema operacional.

 Asset* Asset::createFromUncompressedMap(FileMap* dataMap, AccessMode mode) { _FileAsset* pAsset; status_t result; pAsset = new _FileAsset; result = pAsset->openChunk(dataMap); if (result != NO_ERROR) return NULL; pAsset->mAccessMode = mode; return pAsset; } 

Aviso do PVS-Studio: V773 CWE-401 A função foi encerrada sem liberar o ponteiro 'pAsset'. É possível um vazamento de memória.Asset.cpp 296

Se você não pode abrir um pedaço era, então a função de saídas sem destruir o objeto, um ponteiro para a qual é armazenada na variável Passet . Como resultado, ocorrerá um vazamento de memória.

Outros erros são semelhantes, portanto não vejo razão para considerá-los no artigo. Os interessados podem ver outros avisos no arquivo: Android_V773.txt .

Indo para o exterior uma matriz

Há um grande número de padrões incorretos que levam ao estouro da matriz. No caso do Android, detectamos apenas um padrão de erro do seguinte tipo:

 if (i < 0 || i > MAX) return; A[i] = x; 

Em C e C ++, as células da matriz são numeradas de 0, portanto, o índice máximo de um elemento na matriz deve ser um menor que o tamanho da própria matriz. A verificação correta deve ser assim:

 if (i < 0 || i >= MAX) return; A[i] = x; 

O overflow de uma matriz, de acordo com a Common Weakness Enumeration, é classificado como CWE-119 : Restrição imprópria de operações dentro dos limites de um buffer de memória.

Veja como esses erros aparecem no código do Android.

 static btif_hf_cb_t btif_hf_cb[BTA_AG_MAX_NUM_CLIENTS]; static bool IsSlcConnected(RawAddress* bd_addr) { if (!bd_addr) { LOG(WARNING) << __func__ << ": bd_addr is null"; return false; } int idx = btif_hf_idx_by_bdaddr(bd_addr); if (idx < 0 || idx > BTA_AG_MAX_NUM_CLIENTS) { LOG(WARNING) << __func__ << ": invalid index " << idx << " for " << *bd_addr; return false; } return btif_hf_cb[idx].state == BTHF_CONNECTION_STATE_SLC_CONNECTED; } 

Aviso do PVS-Studio: É possível saturação da matriz V557 CWE-119. O valor do índice 'idx' pode chegar a 6. btif_hf.cc 277

Opção de verificação correta:

 if (idx < 0 || idx >= BTA_AG_MAX_NUM_CLIENTS) { 

Exatamente os mesmos erros foram encontrados mais duas coisas:

• É possível saturação da matriz V557 CWE-119. O valor do índice 'idx' pode chegar a 6. btif_hf.cc 869
• É possível saturação da matriz V557 CWE-119. O valor do índice 'index' pode chegar a 6. btif_rc.cc 374

Ciclos interrompidos

Existem muitas maneiras de escrever um ciclo com defeito. No código do Android, encontrei erros que, de acordo com a Enumeração de fraqueza comum, podem ser classificados como:

• CWE-20 : Validação de entrada incorreta
• CWE-670 : Implementação do fluxo de controle sempre incorreta
• CWE-691 : Gerenciamento insuficiente de fluxo de controle
• CWE-834 : Iteração Excessiva

Ao mesmo tempo, é claro, existem outras maneiras de "dar um tiro na própria perna" ao escrever ciclos, mas desta vez eles não me conheceram.

 int main(int argc, char **argv) { .... char c; printf("%s is already in *.base_fs format, just ..... ", ....); rewind(blk_alloc_file); while ((c = fgetc(blk_alloc_file)) != EOF) { fputc(c, base_fs_file); } .... } 

Aviso do PVS-Studio: O V739 CWE-20 EOF não deve ser comparado com um valor do tipo 'char'. O '(c = fgetc (blk_alloc_file))' deve ser do tipo 'int'. blk_alloc_to_base_fs.c 61

O analisador detectou que a constante EOF é comparada com uma variável do tipo char . Vamos ver por que esse código está incorreto.

A função fgetc retorna um valor int , a saber: pode retornar um número de 0 a 255 ou EOF (-1). O valor de leitura é colocado em uma variável do tipo char . Por esse motivo, um caractere com o valor 0xFF (255) se transforma em -1 e é interpretado exatamente da mesma maneira que o final de um arquivo (EOF).

Devido a esses erros, os usuários que usam códigos ASCII estendidos às vezes encontram uma situação em que um dos caracteres do alfabeto é processado incorretamente pelos programas. Por exemplo, a última letra do alfabeto russo na codificação Windows-1251 possui apenas o código 0xFF e é percebida por alguns programas como o final do arquivo.

Resumindo, podemos dizer que a condição para interromper o ciclo está gravada incorretamente. Para corrigir a situação, a variável c deve ser do tipo int.

Continuamos e consideramos erros mais familiares ao usar a instrução for .

 status_t AudioPolicyManager::registerPolicyMixes(....) { .... for (size_t i = 0; i < mixes.size(); i++) { .... for (size_t j = 0; i < mHwModules.size(); j++) { // <= if (strcmp(AUDIO_HARDWARE_MODULE_ID_REMOTE_SUBMIX, mHwModules[j]->mName) == 0 && mHwModules[j]->mHandle != 0) { rSubmixModule = mHwModules[j]; break; } .... } .... } 

Aviso do PVS-Studio: V534 CWE-691 É provável que uma variável incorreta esteja sendo comparada dentro do operador 'for'. Considere revisar 'i'. AudioPolicyManager.cpp 2489

Por causa de um erro de digitação em um loop aninhado, a condição usa a variável i , embora você deva usar a variável j . Como resultado, a variável j é incrementada incontrolavelmente, o que, com o tempo, levará a matriz mHwModules a ficar fora dos limites . O que acontecerá a seguir é impossível prever, pois haverá um comportamento indefinido do programa.

A propósito, esse fragmento com erro foi completamente copiado para outra função. Portanto, o analisador encontrou exatamente o mesmo erro aqui: AudioPolicyManager.cpp 2586.

Existem mais três trechos de código que são muito suspeitos para mim. No entanto, não pretendo dizer que esse código seja definitivamente errado, pois existe uma lógica complexa. De qualquer forma, devo prestar atenção a este código para que o autor o verifique.

O primeiro fragmento:

 void ce_t3t_handle_check_cmd(....) { .... for (i = 0; i < p_cb->cur_cmd.num_blocks; i++) { .... for (i = 0; i < T3T_MSG_NDEF_ATTR_INFO_SIZE; i++) { checksum += p_temp[i]; } .... } .... } 

Aviso do PVS-Studio: V535 CWE-691 A variável 'i' está sendo usada para esse loop e para o loop externo. Verifique as linhas: 398, 452. ce_t3t.cc 452

Observe que a variável i é usada para os ciclos externo e interno.

Mais dois gatilhos do analisador semelhantes:

• V535 CWE-691 A variável 'xx' está sendo usada para esse loop e para o loop externo. Verifique as linhas: 801, 807. sdp_db.cc 807
• V535 CWE-691 A variável 'xx' está sendo usada para esse loop e para o loop externo. Verifique as linhas: 424, 438. nfa_hci_act.cc 438

Você não está cansado ainda? Sugiro pausar e baixar o PVS-Studio para tentar verificar seu projeto.

Agora vamos continuar.

 #define NFA_HCI_LAST_PROP_GATE 0xFF tNFA_HCI_DYN_GATE* nfa_hciu_alloc_gate(uint8_t gate_id, tNFA_HANDLE app_handle) { .... for (gate_id = NFA_HCI_FIRST_HOST_SPECIFIC_GENERIC_GATE; gate_id <= NFA_HCI_LAST_PROP_GATE; gate_id++) { if (gate_id == NFA_HCI_CONNECTIVITY_GATE) gate_id++; if (nfa_hciu_find_gate_by_gid(gate_id) == NULL) break; } if (gate_id > NFA_HCI_LAST_PROP_GATE) { LOG(ERROR) << StringPrintf( "nfa_hci_alloc_gate - no free Gate ID: %u " "App Handle: 0x%04x", gate_id, app_handle); return (NULL); } .... } 

Aviso do PVS-Studio: V654 CWE-834 A condição 'gate_id <= 0xFF' do loop é sempre verdadeira. nfa_hci_utils.cc 248

Observe o seguinte:

• A constante NFA_HCI_LAST_PROP_GATE é igual a 0xFF.
• Uma variável do tipo uint8_t é usada como um contador de loop . Portanto, o intervalo de valores dessa variável é [0..0xFF].

Acontece que a condição gate_id <= NFA_HCI_LAST_PROP_GATE é sempre verdadeira e não pode parar a execução do loop.

O analisador classificou esse erro como CWE-834, mas também pode ser interpretado como CWE-571: Expressão é sempre verdadeira.

O próximo erro no loop está relacionado ao comportamento indefinido.

 status_t SimpleDecodingSource::doRead(....) { .... for (int retries = 0; ++retries; ) { .... } 

Aviso do PVS-Studio: V654 CWE-834 A condição '++ tentativas' do loop é sempre verdadeira.

Aparentemente, o programador queria que a variável tentativas recebesse todos os valores possíveis para a variável int e somente então o loop terminou.

O loop deve parar quando a expressão ++ tenta novamente 0. E isso só é possível se a variável estourar. Como a variável é do tipo assinado, seu estouro leva a um comportamento indefinido. Portanto, esse código está incorreto e pode levar a consequências imprevisíveis. Por exemplo, o compilador tem o direito total de remover a verificação e deixar apenas instruções para incrementar o contador.

E o último erro nesta seção.

 status_t Check(const std::string& source) { .... int pass = 1; .... do { .... switch(rc) { case 0: SLOGI("Filesystem check completed OK"); return 0; case 2: SLOGE("Filesystem check failed (not a FAT filesystem)"); errno = ENODATA; return -1; case 4: if (pass++ <= 3) { SLOGW("Filesystem modified - rechecking (pass %d)", pass); continue; // <= } SLOGE("Failing check after too many rechecks"); errno = EIO; return -1; case 8: SLOGE("Filesystem check failed (no filesystem)"); errno = ENODATA; return -1; default: SLOGE("Filesystem check failed (unknown exit code %d)", rc); errno = EIO; return -1; } } while (0); // <= return 0; } 

Aviso do PVS-Studio: V696 CWE-670 O operador 'continue' encerrará o loop 'do {...} while (FALSE)' porque a condição é sempre falsa. Verifique as linhas: 105, 121. Vfat.cpp 105

Antes de nós, existe um loop do formulário:

 do { .... if (x) continue; .... } while (0) 

Para executar iterações repetidas, o programador usa a instrução continue . Isto está errado.A instrução continue não retoma o loop imediatamente, mas prossegue para verificar a condição. Como a condição é sempre falsa, em qualquer caso, o ciclo será executado apenas uma vez.

Para corrigir o erro, o código pode ser reescrito, por exemplo, assim:

 for (;;) { .... if (x) continue; .... break; } 

Reatribuição Variável

Um erro muito comum é reescrever uma variável antes que o valor anterior fosse usado. Na maioria das vezes, esses erros ocorrem devido a um erro de digitação ou copiar e colar malsucedido. De acordo com a Enumeração de fraqueza comum, esses erros são classificados como CWE-563 : Atribuição a variável sem uso. Não sem esses erros no Android.

 status_t XMLNode::flatten_node(....) const { .... memset(&namespaceExt, 0, sizeof(namespaceExt)); if (mNamespacePrefix.size() > 0) { namespaceExt.prefix.index = htodl(strings.offsetForString(mNamespacePrefix)); } else { namespaceExt.prefix.index = htodl((uint32_t)-1); } namespaceExt.prefix.index = htodl(strings.offsetForString(mNamespacePrefix)); namespaceExt.uri.index = htodl(strings.offsetForString(mNamespaceUri)); .... } 

PVS-Studio Warning: V519 CWE-563 A variável 'namespaceExt.prefix.index' recebe valores duas vezes sucessivos. Talvez isso seja um erro. Verifique as linhas: 1535, 1539. XMLNode.cpp 1539

Para destacar a essência do erro, escreverei um pseudocódigo:

 if (a > 0) X = 1; else X = 2; X = 1; 

Independentemente da condição, sempre será atribuído um valor à variável X (no código atual é namespaceExt.prefix.index ).

 bool AudioFlinger::RecordThread::threadLoop() { .... size_t framesToRead = mBufferSize / mFrameSize; framesToRead = min(mRsmpInFramesOA - rear, mRsmpInFramesP2 / 2); .... } 

PVS-Studio Warning: V519 CWE-563 A variável 'framesToRead' recebe valores duas vezes sucessivos. Talvez isso seja um erro. Verifique as linhas: 6341, 6342. Threads.cpp 6342

Não está claro por que foi necessário inicializar a variável ao declarar, se outro valor é imediatamente gravado nela. Algo está errado aqui.

 void SchedulingLatencyVisitorARM::VisitArrayGet(....) { .... if (index->IsConstant()) { last_visited_latency_ = kArmMemoryLoadLatency; } else { if (has_intermediate_address) { } else { last_visited_internal_latency_ += kArmIntegerOpLatency; } last_visited_internal_latency_ = kArmMemoryLoadLatency; } .... } 

Aviso do PVS-Studio: V519 CWE-563 A variável 'last_visited_internal_latency_' recebe valores duas vezes sucessivos. Talvez isso seja um erro. Verifique as linhas: 680, 682. scheduler_arm.cc 682

Código muito estranho e sem sentido. Atrevo-me a sugerir que deveria ter sido escrito:

 last_visited_internal_latency_ += kArmMemoryLoadLatency; 

E o último erro, demonstrando como o analisador encontra incansavelmente erros com maior probabilidade de serem ignorados, mesmo com uma revisão cuidadosa do código.

 void multiprecision_fast_mod(uint32_t* c, uint32_t* a) { uint32_t U; uint32_t V; .... c[0] += U; V = c[0] < U; c[1] += V; V = c[1] < V; c[2] += V; // V = c[2] < V; // <= c[2] += U; // V = c[2] < U; // <= c[3] += V; V = c[3] < V; c[4] += V; V = c[4] < V; c[5] += V; V = c[5] < V; .... } 

Aviso PVS-Studio: V519 CWE-563 A variável 'V' recebe valores duas vezes sucessivamente. Talvez isso seja um erro. Verifique as linhas: 307, 309. p_256_multprecision.cc 309

O código é tão "arranca os olhos" que eu não quero entender. Isso é claramente visível: há um erro de digitação no código, que destaquei com comentários.

Outros bugs

Existem erros dispersos para os quais não faz sentido criar capítulos separados. No entanto, eles são tão interessantes e traiçoeiros quanto os considerados anteriormente.

Operações Prioritárias

 void TagMonitor::parseTagsToMonitor(String8 tagNames) { std::lock_guard<std::mutex> lock(mMonitorMutex); // Expand shorthands if (ssize_t idx = tagNames.find("3a") != -1) { ssize_t end = tagNames.find(",", idx); char* start = tagNames.lockBuffer(tagNames.size()); start[idx] = '\0'; .... } .... } 

Aviso do PVS-Studio: V593 CWE-783 Considere revisar a expressão do tipo 'A = B! = C'. A expressão é calculada da seguinte forma: 'A = (B! = C)'. TagMonitor.cpp 50

Enumeração de fraqueza comum: CWE-783 : Erro de lógica de precedência do operador.

O programador concebeu o seguinte. A subcadeia “3a” é pesquisada e a posição dessa subcadeia é gravada na variável idx . Se a substring for encontrada (idx! = -1), o código começará a ser executado, que usa o valor da variável idx .

Infelizmente, o programador está confuso sobre as prioridades das operações. De fato, a verificação funciona assim:

 if (ssize_t idx = (tagNames.find("3a") != -1)) 

Primeiro, ele verifica se há uma subcadeia "3a" na string e o resultado false / true é colocado na variável idx . Como resultado, a variável idx tem o valor 0 ou 1.

Se a condição for verdadeira (a variável idx é 1), a lógica que usa a variável idx começa a ser executada . Uma variável sempre igual a 1 levará ao comportamento incorreto do programa.

Você pode corrigir o erro se fizer a inicialização da variável a partir da condição:

 ssize_t idx = tagNames.find("3a"); if (idx != -1) 

A nova versão do C ++ 17 também permite que você escreva:

 if (ssize_t idx = tagNames.find("3a"); idx != -1) 

Construtor inválido

 struct HearingDevice { .... HearingDevice() { HearingDevice(RawAddress::kEmpty, false); } .... }; 

Aviso do PVS-Studio: V603 CWE-665 O objeto foi criado, mas não está sendo usado. Se você deseja chamar o construtor, 'this-> HearingDevice :: HearingDevice (....)' deve ser usado. listening_aid.cc 176

Enumeração de fraqueza comum: CWE-665 : inicialização incorreta.

Os programadores geralmente se enganam ao tentar chamar explicitamente um construtor para inicializar um objeto. Existem dois construtores na classe. Para reduzir o tamanho do código fonte, o programador decidiu chamar um construtor de outro. Mas esse código não faz nada do que o desenvolvedor espera.

O seguinte acontece. Um novo objeto sem nome do tipo HearingDevice é criado e destruído. Como resultado, os campos da classe permanecem não inicializados.

Para corrigir o erro, você pode usar o construtor de delegação (esse recurso apareceu no C ++ 11). O código correto é:

 HearingDevice() : HearingDevice(RawAddress::kEmpty, false) { } 

A função não retorna valor

 int NET_RecvFrom(int s, void *buf, int len, unsigned int flags, struct sockaddr *from, int *fromlen) { socklen_t socklen = *fromlen; BLOCKING_IO_RETURN_INT( s, recvfrom(s, buf, len, flags, from, &socklen) ); *fromlen = socklen; } 

PVS-Studio Warning: V591 CWE-393 A função não nula deve retornar um valor. linux_close.cpp 139

Enumeração de fraqueza comum: CWE-393 : Retorno do código de status incorreto.

A função retornará um valor aleatório. Outro erro: V591 CWE-393 A função não nula deve retornar um valor. linux_close.cpp 158 Cálculo

incorreto do tamanho da estrutura

 int MtpFfsHandle::handleControlRequest(....) { .... struct mtp_device_status *st = reinterpret_cast<struct mtp_device_status*>(buf.data()); st->wLength = htole16(sizeof(st)); .... } 

PVS-Studio Warning: V568 É estranho que o operador 'sizeof ()' avalie o tamanho de um ponteiro para uma classe, mas não o tamanho do objeto de classe 'st'. MtpFfsHandle.cpp 251

Estou certo de que na variável membro wLength queria colocar o tamanho da estrutura, não o tamanho do ponteiro. Então o código correto deve ser assim:

 st->wLength = htole16(sizeof(*st)); 

Respostas semelhantes do analisador:

• V568 É estranho que o operador 'sizeof ()' avalie o tamanho de um ponteiro para uma classe, mas não o tamanho do objeto de classe 'cacheinfo'. NetlinkEvent.cpp 220
• V568 É estranho que o operador 'sizeof ()' avalie o tamanho de um ponteiro para uma classe, mas não o tamanho do objeto de classe 'page-> next'. linker_block_allocator.cpp 146
• V568 É estranho que o argumento do operador sizeof () seja a expressão '& session_id'. reference-ril.c 1775

Operações de bits sem sentido

 #define EGL_CONTEXT_OPENGL_DEBUG_BIT_KHR 0x00000001 #define EGL_CONTEXT_OPENGL_FORWARD_COMPATIBLE_BIT_KHR 0x00000002 #define EGL_CONTEXT_OPENGL_ROBUST_ACCESS_BIT_KHR 0x00000004 EGLContext eglCreateContext(....) { .... case EGL_CONTEXT_FLAGS_KHR: if ((attrib_val | EGL_CONTEXT_OPENGL_DEBUG_BIT_KHR) || (attrib_val | EGL_CONTEXT_OPENGL_FORWARD_C....) || (attrib_val | EGL_CONTEXT_OPENGL_ROBUST_ACCESS_BIT_KHR)) { context_flags = attrib_val; } else { RETURN_ERROR(EGL_NO_CONTEXT,EGL_BAD_ATTRIBUTE); } .... } 

Aviso do PVS-Studio: V617 CWE-480 Considere inspecionar a condição. O argumento '0x00000001' do '|' operação bit a bit contém um valor diferente de zero. egl.cpp 1329

Enumeração de fraqueza comum: CWE-480 : Uso de operador incorreto.

Uma expressão do formulário (A | 1) || (A | 2) || (A | 4) não faz sentido, pois o resultado sempre será verdadeiro. De fato, você precisa usar o operador & , e o código faz sentido:

 if ((attrib_val & EGL_CONTEXT_OPENGL_DEBUG_BIT_KHR) || (attrib_val & EGL_CONTEXT_OPENGL_FORWARD_COMPATIBLE_BIT_KHR) || (attrib_val & EGL_CONTEXT_OPENGL_ROBUST_ACCESS_BIT_KHR)) 

Erro semelhante: V617 CWE-480 Considere inspecionar a condição. O argumento '0x00000001' do '|' operação bit a bit contém um valor diferente de zero. egl.cpp 1338 Mudança

incorreta de bits

 template <typename AddressType> struct RegsInfo { .... uint64_t saved_reg_map = 0; AddressType saved_regs[64]; .... inline AddressType* Save(uint32_t reg) { if (reg > sizeof(saved_regs) / sizeof(AddressType)) { abort(); } saved_reg_map |= 1 << reg; saved_regs[reg] = (*regs)[reg]; return &(*regs)[reg]; } .... } 

Aviso do PVS-Studio: V629 CWE-190 Considere inspecionar a expressão '1 << reg'. Mudança de bit do valor de 32 bits com uma expansão subsequente para o tipo de 64 bits. RegsInfo.h 47

Enumeração de fraqueza comum: CWE-190 : Estouro de número inteiro ou envolvente.

Com um deslocamento de 1 << reg, o valor da variável reg fica no intervalo [0..63]. A expressão serve para obter diferentes graus de dois, começando com 2 ^ 0 e terminando com 2 ^ 63.

O código não funciona. O fato é que o literal numérico 1 tem um tipo int de 32 bits . Portanto, não funcionará para obter um valor maior que 1 ^ 31. Uma mudança para um valor maior leva ao estouro da variável e ao surgimento de comportamento indefinido.

O código correto é:

 saved_reg_map |= static_cast<uint64_t>(1) << reg; 

ou:

 saved_reg_map |= 1ULL << reg; 

As strings são copiadas para elas mesmas.

 void PCLmGenerator::writeJobTicket() { // Write JobTicket char inputBin[256]; char outputBin[256]; if (!m_pPCLmSSettings) { return; } getInputBinString(m_pPCLmSSettings->userInputBin, &inputBin[0]); getOutputBin(m_pPCLmSSettings->userOutputBin, &outputBin[0]); strcpy(inputBin, inputBin); strcpy(outputBin, outputBin); .... } 

Avisos do PVS-Studio:

• V549 CWE-688 O primeiro argumento da função 'strcpy' é igual ao segundo argumento. genPCLm.cpp 1181
• V549 CWE-688 O primeiro argumento da função 'strcpy' é igual ao segundo argumento. genPCLm.cpp 1182

De acordo com a classificação da Enumeração de fraqueza comum: CWE-688 : Chamada de função com variável incorreta ou referência como argumento.

Por algum motivo, as strings são copiadas para elas mesmas. Provavelmente, existem alguns erros de digitação aqui.

Usando uma variável não inicializada

 void mca_set_cfg_by_tbl(....) { tMCA_DCB* p_dcb; const tL2CAP_FCR_OPTS* p_opt; tMCA_FCS_OPT fcs = MCA_FCS_NONE; if (p_tbl->tcid == MCA_CTRL_TCID) { p_opt = &mca_l2c_fcr_opts_def; } else { p_dcb = mca_dcb_by_hdl(p_tbl->cb_idx); if (p_dcb) { p_opt = &p_dcb->p_chnl_cfg->fcr_opt; fcs = p_dcb->p_chnl_cfg->fcs; } } memset(p_cfg, 0, sizeof(tL2CAP_CFG_INFO)); p_cfg->mtu_present = true; p_cfg->mtu = p_tbl->my_mtu; p_cfg->fcr_present = true; memcpy(&p_cfg->fcr, p_opt, sizeof(tL2CAP_FCR_OPTS)); // <= .... } 

Aviso do PVS-Studio: V614 CWE-824 Ponteiro potencialmente não inicializado 'p_opt' usado. Considere verificar o segundo argumento real da função 'memcpy'. mca_main.cc 252

De acordo com a classificação do Weakness Enumeration do comum: o CWE-824 : Acesso de ponteiro não inicializado.

Se p_tbl-> tcid! = MCA_CTRL_TCID e p_dcb == nullptr , o ponteiro p_opt permanecerá não inicializado.

Uso mais estranho de variável não inicializada

 struct timespec { __time_t tv_sec; /* Seconds. */ long int tv_nsec; /* Nanoseconds. */ }; static inline timespec NsToTimespec(int64_t ns) { timespec t; int32_t remainder; t.tv_sec = ns / kNanosPerSecond; remainder = ns % kNanosPerSecond; if (remainder < 0) { t.tv_nsec--; remainder += kNanosPerSecond; } t.tv_nsec = remainder; return t; } 

Aviso do PVS-Studio: V614 CWE-457 Variável não inicializada 't.tv_nsec' usada. clock_ns.h 55

Enumeração de fraqueza comum: CWE-457 : Uso de variável não inicializada.

No momento de decrementar a variável t.tv_nsec, ela não é inicializada. A variável é inicializada posteriormente: t.tv_nsec = restante; .Algo aqui está claramente confuso.

Super expressão

 void bta_dm_co_ble_io_req(....) { .... *p_auth_req = bte_appl_cfg.ble_auth_req | (bte_appl_cfg.ble_auth_req & 0x04) | ((*p_auth_req) & 0x04); .... } 

PVS-Studio Warning: V578 Detectada uma operação bit a bit. Considere verificar isso. bta_dm_co.cc 259

Esta expressão é redundante. Se você remover a subexpressão (bte_appl_cfg.ble_auth_req & 0x04) , o resultado da expressão não será alterado. Talvez haja algum erro de digitação aqui.

Lidar com vazamento

 bool RSReflectionCpp::genEncodedBitCode() { FILE *pfin = fopen(mBitCodeFilePath.c_str(), "rb"); if (pfin == nullptr) { fprintf(stderr, "Error: could not read file %s\n", mBitCodeFilePath.c_str()); return false; } unsigned char buf[16]; int read_length; mOut.indent() << "static const unsigned char __txt[] ="; mOut.startBlock(); while ((read_length = fread(buf, 1, sizeof(buf), pfin)) > 0) { mOut.indent(); for (int i = 0; i < read_length; i++) { char buf2[16]; snprintf(buf2, sizeof(buf2), "0x%02x,", buf[i]); mOut << buf2; } mOut << "\n"; } mOut.endBlock(true); mOut << "\n"; return true; } 

Aviso do PVS-Studio: V773 CWE-401 A função foi encerrada sem liberar o identificador 'pfin'. Um vazamento de recurso é possível.slang_rs_reflection_cpp.cpp 448

O analisador classificou esse erro de acordo com a Enumeração de fraqueza comum como: CWE-401 : liberação inadequada de memória antes de remover a última referência ('vazamento de memória'). No entanto, seria mais correto aqui emitir o CWE-775 : versão ausente do descritor ou identificador de arquivo após a vida útil efetiva. Instruirei meus colegas a corrigir esse defeito no PVS-Studio.

O identificador pfin não é liberado em nenhum lugar. Esqueci de chamar a função fclose no final . Um erro desagradável que pode esgotar rapidamente todo o suprimento de descritores disponíveis, após o qual será impossível abrir novos arquivos.

Conclusão

Como você pode ver, mesmo em um projeto tão conhecido e testado como o Android, o analisador PVS-Studio encontra facilmente muitos erros e possíveis vulnerabilidades. Para resumir quais fraquezas (possíveis vulnerabilidades) foram encontradas:

• CWE-14: Remoção de código pelo compilador para limpar buffers
• CWE-20: Validação de entrada incorreta
• CWE-119: Restrição imprópria de operações dentro dos limites de um buffer de memória
• CWE-190: Estouro de número inteiro ou envolvente
• CWE-198: Uso de pedidos de bytes incorretos
• CWE-393: Retorno do código de status incorreto
• CWE-401: Liberação incorreta da memória antes de remover a última referência ('Vazamento de memória')
• CWE-457: Uso de Variável Não Inicializada
• CWE-462: Chave duplicada na lista associativa
• CWE-480: Uso do operador incorreto
• CWE-484: Declaração de quebra omitida no comutador
• CWE-561: Código Morto
• CWE-562: Retorno do endereço variável da pilha
• CWE-563: Assignment to Variable without Use
• CWE-570: Expression is Always False
• CWE-571: Expression is Always True
• CWE-476: NULL Pointer Dereference
• CWE-628: Function Call with Incorrectly Specified Arguments
• CWE-665: Improper Initialization
• CWE-670: Always-Incorrect Control Flow Implementation
• CWE-682: Incorrect Calculation
• CWE-688: Function Call With Incorrect Variable or Reference as Argument
• CWE-690: Unchecked Return Value to NULL Pointer Dereference
• CWE-691: Insufficient Control Flow Management
• CWE-758: Reliance on Undefined, Unspecified, or Implementation-Defined Behavior
• CWE-762: Mismatched Memory Management Routines
• CWE-775: Missing Release of File Descriptor or Handle after Effective Lifetime
• CWE-783: Operator Precedence Logic Error
• CWE-824: Access of Uninitialized Pointer
• CWE-834: Excessive Iteration

No total, descrevi 490 possíveis vulnerabilidades no artigo. De fato, o analisador é capaz de identificar mais deles, mas, como escrevi anteriormente, não encontrei forças para estudar o relatório com mais cuidado.

O tamanho da base de código testada é de cerca de 2.168.000 linhas de código em C e C ++. Destes, 14,4% são comentários. No total, temos cerca de 1.855.000 linhas de código limpo.

Portanto, temos 490 CWEs para 1.855.000 linhas de código.

Acontece que o analisador PVS-Studio é capaz de detectar mais de uma fraqueza (vulnerabilidade potencial) para cada 4000 linhas de código em um projeto Android. Bom resultado para um analisador de código, estou feliz.

Obrigado pela atenção! Desejo a todos um código sem código e proponho o seguinte:

1. Faça o download do PVS-Studio e verifique o rascunho de trabalho.
2. : : .
3. , : twitter , RSS , vk.com .

Se você deseja compartilhar este artigo com um público que fala inglês, use o link para a tradução: Andrey Karpov. We Checked the Android Source Codes by PVS-Studio or Nothing is Perfect