A.
Le développement de grands projets complexes est impossible sans l'utilisation de méthodologies et d'outils de programmation pour aider à contrôler la qualité du code. Tout d'abord, c'est une norme de codage compétente, des revues de code, des tests unitaires, des analyseurs de code statiques et dynamiques. Tout cela permet d'identifier les défauts du code dès les premiers stades de développement. Cet article montre les capacités de l'analyseur statique PVS-Studio pour détecter les erreurs et les vulnérabilités potentielles dans le code du système d'exploitation Android. Nous espérons que l'article attirera les lecteurs vers la méthodologie de l'analyse de code statique et ils voudront la mettre en œuvre dans le processus de développement de leurs propres projets.
Présentation
Un an s'est écoulé depuis la rédaction d'un grand
article sur les erreurs dans le système d'exploitation Tizen, et je voulais à nouveau mener une étude tout aussi intéressante d'un système d'exploitation. Le choix s'est porté sur Android.
Le code du système d'exploitation Android est de qualité, bien testé. Pendant le développement, au moins un analyseur de code statique Coverity est utilisé, comme en témoignent les commentaires du formulaire:
En général, c'est un projet intéressant et de haute qualité, et y trouver des erreurs est un défi pour notre analyseur statique PVS-Studio.
Je pense que seulement par le volume de l'article, le lecteur comprend que l'analyseur PVS-Studio a fait un excellent travail et a trouvé beaucoup de défauts dans le code de ce système d'exploitation.
Énumération des faiblesses courantes
Dans l'article, vous trouverez des références à l'
énumération des faiblesses communes (CWE). Expliquons la raison pour laquelle nous nous référons à cette liste et pourquoi elle est importante du point de vue de la sécurité.
Souvent, la cause des vulnérabilités des programmes n'est pas un ensemble de circonstances délicates, mais une erreur logicielle banale. Ici, il sera approprié de citer cette citation de
prqa.com :
L'Institut national des normes et de la technologie (NIST) rapporte que 64% des vulnérabilités logicielles proviennent d'erreurs de programmation et non d'un manque de fonctionnalités de sécurité.
Vous pouvez lire dans l'article «
Comment PVS-Studio peut aider à trouver des vulnérabilités? » Avec quelques exemples d'erreurs simples qui ont conduit à des vulnérabilités dans des projets tels que MySQL, iOS, NAS, illumos-gate.
En conséquence, de nombreuses vulnérabilités peuvent être éliminées en détectant les erreurs courantes dans le temps et en les corrigeant. Et ici, l'énumération des faiblesses communes entre en scène.
Les erreurs sont différentes et toutes les erreurs ne sont pas dangereuses du point de vue de la sécurité. Les erreurs susceptibles de provoquer des vulnérabilités sont collectées dans l'énumération des faiblesses communes. Cette liste est en cours de mise à jour et il y a probablement des erreurs pouvant entraîner des vulnérabilités, mais elles n'ont pas encore été incluses dans cette liste.
Cependant, si l'erreur est classée selon le CWE, cela signifie qu'il est théoriquement possible qu'elle puisse être exploitée en tant que vulnérabilité (
CVE ). Oui, la probabilité de cela est faible. Très rarement, CWE se transforme en CVE. Cependant, si vous souhaitez protéger votre code contre les vulnérabilités, vous devriez, si possible, trouver autant de bogues décrits dans CWE que possible et les corriger.
Schématiquement, la relation entre PVS-Studio, les erreurs, CWE et CVE est illustrée dans la figure:
Certaines erreurs sont classées CWE. PVS-Studio peut détecter bon nombre de ces erreurs, empêchant ainsi certains de ces défauts de devenir des vulnérabilités (CVE).
Nous pouvons dire que PVS-Studio identifie de nombreuses vulnérabilités potentielles avant qu'elles ne causent des dommages. Ainsi, PVS-Studio est un outil de test de sécurité d'application statique (
SAST ).
Maintenant, je pense, il est clair pourquoi, lors de la description des erreurs, j'ai considéré qu'il était important de noter comment elles sont classées selon le CWE. De cette façon, il est plus facile de montrer l'importance d'utiliser un analyseur de code statique dans des projets critiques, auxquels les systèmes d'exploitation sont clairement liés.
Vérification Android
Pour analyser le projet, j'ai utilisé l'analyseur PVS-Studio version 6.24. L'analyseur prend actuellement en charge les langages et compilateurs suivants:
- Windows Visual Studio 2010-2017 C, C ++, C ++ / CLI, C ++ / CX (WinRT), C #
- Windows IAR Embedded Workbench, compilateur C / C ++ pour ARM C, C ++
- Windows / Linux Keil µVision, DS-MDK, ARM Compiler 5/6 C, C ++
- Windows / Linux Texas Instruments Code Composer Studio, Outils de génération de code ARM C, C ++
- Windows / Linux / macOS. Clang C, C ++
- Linux / macOS. GCC C, C ++
- Windows MinGW C, C ++
Remarque Certains de nos lecteurs ont peut-être manqué la nouvelle que nous soutenions le travail dans l'environnement macOS et ils seront intéressés par cette publication: "
Version PVS-Studio pour macOS: 64 faiblesses dans le noyau Apple XNU ".
Le processus de vérification du code source Android n'était pas un problème, donc je ne m'y attarderai pas. Le problème était plutôt ma préoccupation pour d’autres tâches, c’est pourquoi je n’ai pas trouvé le temps et l’énergie nécessaires pour examiner le rapport aussi attentivement que je le souhaiterais. Cependant, même un coup d'œil rapide était plus que suffisant pour rassembler une grande collection d'erreurs intéressantes pour un article solide.
Plus important encore: je demande aux développeurs Android non seulement de corriger les erreurs décrites dans l'article, mais également de mener une analyse indépendante plus approfondie. J'ai regardé le rapport de l'analyseur superficiellement et j'aurais pu manquer beaucoup d'erreurs graves.
Lors du premier test, l'analyseur produira beaucoup de faux positifs, mais
ce n'est pas un problème . Notre équipe est prête à vous aider avec des recommandations sur la configuration de l'analyseur pour réduire le nombre de faux positifs. Nous sommes également prêts à fournir une clé de licence pour un mois ou plus, si nécessaire. En général,
écrivez-nous , nous vous aiderons et vous le dirons.
Voyons maintenant quelles erreurs et vulnérabilités potentielles j'ai réussi à trouver. J'espère que vous apprécierez ce que l'analyseur de code statique PVS-Studio peut trouver. Bonne lecture.
Des comparaisons dénuées de sens
L'analyseur considère les expressions comme anormales si elles sont toujours vraies ou fausses. Ces avertissements, selon l'énumération commune des faiblesses, sont classés comme suit:
- CWE-570 : L'expression est toujours fausse
- CWE-571 : L'expression est toujours vraie
L'analyseur génère de nombreux avertissements de ce type et, malheureusement, la plupart d'entre eux sont faux pour le code Android. Dans ce cas, l'analyseur n'est pas à blâmer. Seul le code est écrit comme ça.
Je vais le démontrer avec un exemple simple.
#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') { .... }
Ici, l'analyseur génère un avertissement: V547 CWE-570 L'expression 'alternative_config_path [0]! =' \ 0 '' est toujours fausse. phNxpConfig.cpp 401
Le fait est que la macro
GENERIC_TARGET n'est pas définie, et du point de vue de l'analyseur, le code ressemble à ceci:
const char alternative_config_path[] = ""; .... if (alternative_config_path[0] != '\0') {
L'analyseur est simplement obligé d'émettre un avertissement, car la ligne est vide et un zéro terminal est toujours situé au décalage zéro. Ainsi, l'analyseur a formellement raison d'émettre un avertissement. Cependant, d'un point de vue pratique, cet avertissement ne présente aucun avantage.
Malheureusement, rien ne peut être fait dans de telles situations. Nous devrons examiner systématiquement tous ces avertissements et marquer de nombreux endroits comme des faux positifs afin que l'analyseur n'émette plus de messages sur ces lignes. Cela doit vraiment être fait, car, en plus des messages dénués de sens, de nombreux défauts réels seront détectés.
J'avoue honnêtement que je n'étais pas intéressé à regarder attentivement les avertissements de ce type, et je les ai passés en revue superficiellement. Cependant, même cela suffira pour montrer que de tels diagnostics sont très utiles et trouvent des erreurs intéressantes.
Je veux commencer par la situation classique lorsque la fonction de comparaison de deux objets est incorrectement implémentée. Pourquoi classique? Il s'agit d'un modèle d'erreur typique que nous rencontrons constamment dans une variété de projets. Très probablement, il y a trois raisons à son apparition:
- Les fonctions de comparaison sont simples et écrites «automatiquement» et utilisant la technologie Copy-Paste. Lors de l'écriture d'un tel code, une personne est inattentive et fait souvent des fautes de frappe.
- En règle générale, la révision de code n'est pas effectuée pour ces fonctions, car elle est trop paresseuse pour examiner des fonctions simples et ennuyeuses.
- Les tests unitaires ne sont généralement pas effectués pour de telles fonctions. Parce que la paresse. De plus, les fonctions sont simples et les programmeurs ne pensent pas que des erreurs y soient possibles.
Ces réflexions et exemples sont décrits plus en détail dans l'article «Le
mal vit dans les fonctions de comparaison ».
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; }
Donc, devant nous est la fonction classique de comparer deux objets de type
AudioPlaybackRate . Et, comme je pense, le lecteur devine que c'est faux. L'analyseur PVS-Studio remarque immédiatement deux fautes de frappe:
- V501 CWE-571 Il existe des sous-expressions identiques à gauche et à droite de l'opérateur '==': pr2.mStretchMode == pr2.mStretchMode AudioResamplerPublic.h 107
- V501 CWE-571 Il existe des sous-expressions identiques à gauche et à droite de l'opérateur '==': pr2.mFallbackMode == pr2.mFallbackMode AudioResamplerPublic.h 108
Le champ
pr2.mStretchMode et le champ
pr2.mFallbackMode sont comparés à eux-mêmes. Il s'avère que la fonction ne compare pas suffisamment les objets avec précision.
La comparaison inutile suivante réside dans une fonction qui stocke les informations d'empreintes digitales dans un fichier.
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)
Une anomalie est détectée dans ce code par deux diagnostics à la fois:
- V501 CWE-570 Il existe des sous-expressions identiques à gauche et à droite de '||' opérateur: ns! = 1 || ns! = 1 empreinte digitale.c 126
- V560 CWE-570 Une partie de l'expression conditionnelle est toujours fausse: ns! = 1. fingerprint.c 126
Il n'y a aucune gestion de la situation lorsque le deuxième appel à la fonction
fwrite ne peut pas écrire de données dans un fichier. En d'autres termes, la valeur de la variable
nf n'est pas vérifiée. La vérification correcte devrait ressembler à ceci:
if (ns != 1 || nf != 1)
Nous passons à l'erreur suivante associée à l'utilisation de l'opérateur
& .
#define O_RDONLY 00000000 #define O_WRONLY 00000001 #define O_RDWR 00000002 static ssize_t verity_read(fec_handle *f, ....) { .... if (f->mode & O_RDONLY && expect_zeros) { memset(data, 0, FEC_BLOCKSIZE); goto valid; } .... }
Avertissement PVS-Studio: V560 CWE-570 Une partie de l'expression conditionnelle est toujours fausse: f-> mode & 00000000. fec_read.cpp 322
Notez que la constante
O_RDONLY est nulle. Cela rend l'expression
f-> mode & O_RDONLY inutile, car elle est toujours 0. Il s'avère que la condition de l'
instruction if n'est jamais satisfaite et que l'instruction-true est du code mort.
La vérification correcte devrait être comme ceci:
if (f->mode == O_RDONLY && expect_zeros) {
Examinons maintenant une faute de frappe classique lorsque nous avons oublié d'écrire une partie de la condition.
enum { .... CHANGE_DISPLAY_INFO = 1 << 2, .... }; void RotaryEncoderInputMapper::configure(nsecs_t when, const InputReaderConfiguration* config, uint32_t changes) { .... if (!changes || (InputReaderConfiguration::CHANGE_DISPLAY_INFO)) { .... }
Avertissement PVS-Studio: V768 CWE-571 La constante d'énumération 'CHANGE_DISPLAY_INFO' est utilisée comme variable de type booléen. InputReader.cpp 3016
La condition est toujours vraie, puisque l'opérande
InputReaderConfiguration :: CHANGE_DISPLAY_INFO est une constante égale à 4.
Si vous regardez comment le code est écrit dans le quartier, il devient clair qu'en fait, la condition devrait être la suivante:
if (!changes || (changes & InputReaderConfiguration::CHANGE_DISPLAY_INFO)) {
La comparaison suivante, qui n'a pas de sens, je l'ai rencontré dans l'opérateur de boucle.
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); } } .... }
Avertissement PVS-Studio: V560 CWE-571 Une partie de l'expression conditionnelle est toujours vraie: (j <4). ipphelper.c 926
Notez que la valeur de la variable
j n'est incrémentée nulle part. Cela signifie que la sous-expression
(j <4) est toujours vraie.
Le plus grand nombre d'opérations utiles de l'analyseur PVS-Studio, concernant toujours des conditions vraies / fausses, se réfère au code où le résultat de la création d'objets est vérifié à l'aide du
nouvel opérateur. En d'autres termes, l'analyseur détecte le modèle de code suivant:
T *p = new T; if (p == nullptr) return ERROR;
De tels contrôles sont inutiles. S'il n'était pas possible d'allouer de la mémoire à l'objet, une exception du type
std :: bad_alloc est
levée, et il ne sera tout simplement pas possible de vérifier la valeur du pointeur.
Remarque Le
nouvel opérateur peut retourner
nullptr en écrivant
new (std :: nothrow) T. Cependant, cela ne s'applique pas aux erreurs discutées. L'analyseur PVS-Studio prend en compte
(std :: nothrow) et ne donne pas d'avertissement si l'objet est créé de cette manière.
Il peut sembler que de telles erreurs sont inoffensives. Eh bien, pensez-y, un chèque supplémentaire qui ne fonctionne jamais. Quoi qu'il en soit, une exception sera levée qui sera traitée quelque part. Malheureusement, certains développeurs placent dans l'instruction-true les actions de l'
instruction if qui libèrent des ressources, etc. Étant donné que ce code ne s'exécute pas, il peut entraîner des fuites de mémoire et d'autres erreurs.
Considérez l'un de ces cas que j'ai remarqué dans le code 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; } .... }
Avertissement PVS-Studio: V668 CWE-570 Il est inutile de tester le pointeur 'buf' contre null, car la mémoire a été allouée à l'aide de l'opérateur 'new'. L'exception sera générée en cas d'erreur d'allocation de mémoire. scan.cpp 213
Veuillez noter que s'il n'est pas possible d'allouer un deuxième bloc mémoire, le programmeur essaie de libérer le premier bloc:
delete dataMap;
Ce code ne sera jamais contrôlé. C'est du code mort. Si une exception se produit, une fuite de mémoire se produit.
L'écriture d'un tel code est fondamentalement erronée.
Des pointeurs intelligents ont été inventés pour de tels cas.
Au total, l'analyseur PVS-Studio a détecté
176 emplacements dans le code Android où le pointeur est vérifié après avoir créé des objets en utilisant
new . Je n'ai pas compris à quel point chacun de ces endroits est dangereux et, bien sûr, je n'encombrerai pas l'article avec tous ces avertissements. Les personnes
intéressées peuvent voir d'autres avertissements dans le fichier
Android_V668.txt .
Déréférencer un pointeur nul
Déréférencer un pointeur nul conduit à un comportement de programme indéfini, il est donc utile de trouver et de corriger de tels endroits. Selon la situation, l'analyseur PVS-Studio peut classer ces erreurs selon l'énumération de faiblesse commune comme suit:
- CWE-119 : Restriction incorrecte des opérations dans les limites d'un tampon mémoire
- CWE-476 : Déréférencement de pointeur NULL
- CWE-628 : Appel de fonction avec des arguments spécifiés de manière incorrecte
- CWE-690 : valeur de retour non vérifiée vers la référence nulle au pointeur
Je trouve souvent de telles erreurs dans le code responsable de la gestion des situations non standard ou incorrectes. Personne ne teste un tel code et l'erreur peut y vivre très longtemps. Un tel cas sera examiné maintenant.
bool parseEffect(....) { .... if (xmlProxyLib == nullptr) { ALOGE("effectProxy must contain a <%s>: %s", tag, dump(*xmlProxyLib)); return false; } .... }
Avertissement PVS-Studio: V522 CWE-476 Le déréférencement du pointeur nul 'xmlProxyLib' peut avoir lieu. EffectsConfig.cpp 205
Si le pointeur
xmlProxyLib est
nullptr , le programmeur affiche un message de débogage, pour lequel il est nécessaire de déréférencer ce pointeur. Oups ...
Considérons maintenant une version plus intéressante de l'erreur.
static void soinfo_unload_impl(soinfo* root) { .... soinfo* needed = find_library(si->get_primary_namespace(), library_name, RTLD_NOLOAD, nullptr, nullptr); if (needed != nullptr) {
Avertissement PVS-Studio: V522 CWE-476 Le déréférencement du pointeur nul "nécessaire" peut avoir lieu. linker.cpp 1847
Si le pointeur
avait besoin! = Nullptr , un avertissement est émis, ce qui est un comportement très suspect du programme. Il devient finalement clair que le code contient une erreur si vous regardez ci-dessous et voyez que si
nécessaire == nullptr , le pointeur null sera déréférencé dans l'expression
nécessaire-> is_linked () .
Très probablement, les opérateurs! = Et == sont simplement confondus ici. Si vous remplacez, le code de fonction est logique et l'erreur disparaît.
La majeure partie des avertissements concernant le déréférencement potentiel d'un pointeur nul fait référence à une situation de la forme:
T *p = (T *) malloc (N); *p = x;
Des fonctions telles que
malloc ,
strdup, etc. peuvent retourner
NULL si la mémoire ne peut pas être allouée. Par conséquent, vous ne pouvez pas déréférencer des pointeurs qui renvoient ces fonctions sans d'abord vérifier le pointeur.
Il existe de nombreuses erreurs similaires, donc je ne donnerai que deux fragments de code simples: le premier avec
malloc et le second avec
strdup .
DownmixerBufferProvider::DownmixerBufferProvider(....) { .... effect_param_t * const param = (effect_param_t *) malloc(downmixParamSize); param->psize = sizeof(downmix_params_t); .... }
Avertissement PVS-Studio: V522 CWE-690 Il peut y avoir un déréférencement d'un pointeur nul potentiel 'param'. Vérifiez les lignes: 245, 244. BufferProviders.cpp 245
static char* descriptorClassToDot(const char* str) { .... newStr = strdup(lastSlash); newStr[strlen(lastSlash)-1] = '\0'; .... }
Avertissement PVS-Studio: V522 CWE-690 Il peut y avoir un déréférencement d'un pointeur nul potentiel 'newStr'. Vérifiez les lignes: 203, 202. DexDump.cpp 203
Quelqu'un peut dire que ce sont des erreurs mineures. S'il n'y a pas assez de mémoire, le programme se bloquera simplement lors du déréférencement du pointeur nul, ce qui est normal. Puisqu'il n'y a pas de mémoire, il n'y a rien pour essayer de gérer d'une manière ou d'une autre cette situation.
Une telle personne a tort. Les pointeurs doivent être vérifiés! J'ai examiné ce sujet en détail dans l'article «
Pourquoi est-il important de vérifier ce que la fonction malloc a renvoyé ». Je vous recommande fortement de le lire à tous ceux qui ne l'ont pas lu.
En bref, le danger est que l'écriture dans la mémoire ne se produise pas nécessairement près de l'adresse zéro. Il est possible d'écrire des données quelque part très loin dans une page mémoire qui n'est pas protégée en écriture, et ainsi de provoquer une erreur difficile à atteindre, ou en général cette erreur peut être utilisée comme une vulnérabilité. Voyons ce que je veux dire par l'exemple de la fonction
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)); .... }
Avertissement PVS-Studio: V769 CWE-119 Le pointeur '(uint32_t *) metadata' dans l'expression '(uint32_t *) metadata + new_size_int' pourrait être nullptr. Dans ce cas, la valeur résultante sera insensée et ne doit pas être utilisée. Vérifier les lignes: 91, 89. radio_metadata.c 91
Je n'ai pas compris la logique de la fonction, mais ce n'est pas nécessaire. L'essentiel est qu'un nouveau tampon soit créé et que les données y soient copiées. Si la fonction
realloc renvoie
NULL , les données seront copiées à l'adresse ((uint32_t *) NULL + métadonnées-> size_int - (métadonnées-> nombre + 1)).
Si la
valeur metadata-> size_int est grande, les conséquences seront regrettables. Il s'avère que les données sont écrites dans une mémoire aléatoire.
Soit dit en passant, il existe un autre type de déréférencement de pointeur nul, que l'analyseur PVS-Studio classe non pas comme CWE-690, mais comme CWE-628 (argument non valide).
static void parse_tcp_ports(const char *portstring, uint16_t *ports) { char *buffer; char *cp; buffer = strdup(portstring); if ((cp = strchr(buffer, ':')) == NULL) .... }
Avertissement PVS-Studio: V575 CWE-628 Le pointeur null potentiel est transmis à la fonction 'strchr'. Inspectez le premier argument. Vérifiez les lignes: 47, 46. libxt_tcp.c 47
Le fait est que le déréférencement du pointeur se produit lorsque la fonction
strchr est
appelée . Par conséquent, l'analyseur interprète cette situation comme transmettant une valeur incorrecte à la fonction.
Les
194 autres avertissements de ce type sont répertoriés dans le fichier
Android_V522_V575.txt .
Soit dit en passant, un piquant particulier à toutes ces erreurs est donné par les avertissements discutés précédemment sur la vérification du pointeur après avoir appelé le
nouvel opérateur. Il s'avère qu'il y a 195 appels aux fonctions
malloc /
realloc /
strdup, et ainsi de suite, lorsque le pointeur n'est pas vérifié. Mais il y a 176 endroits où le pointeur est vérifié après avoir appelé
new . D'accord, une approche bizarre!
Au final, il nous reste à considérer les avertissements V595 et V1004, qui sont également associés à l'utilisation de pointeurs nuls.
V595 détecte les situations où le pointeur est déréférencé puis vérifié. Exemple synthétique:
p->foo(); if (!p) Error();
V1004 révèle la situation inverse lorsque le pointeur a été vérifié en premier, puis a oublié de le faire. Exemple synthétique:
if (p) p->foo(); p->doo();
Regardons quelques fragments de code Android, où il y avait des erreurs de ce type. .
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; .... }
PVS-Studio: V595 CWE-476 The 'video' pointer was utilized before it was verified against nullptr. Check lines: 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); } }
PVS-Studio: V595 CWE-476 The 'rsmp' pointer was utilized before it was verified against nullptr. Check lines: 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); } .... }
PVS-Studio: V1004 CWE-476 The 'p_clcb->p_srcb' pointer was used unsafely after it was verified against nullptr. Check lines: 695, 701. bta_gattc_act.cc 701
. , , - .
:
- 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
, . , . , .
:
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); } .... }
PVS-Studio: V595 CWE-476 The 'iwnn' pointer was utilized before it was verified against nullptr. Check lines: 686, 689. ndapi.c 686
, , « ». . ,
iwnn , . , , .
, . , . , , :
- , : iwnn->previous_selection
- , undefined behavior
- , iwnn
- : if (iwnn == NULL)
- , ,
"
".
, Common Weakness Enumeration
CWE-14 : Compiler Removal of Code to Clear Buffers.
, :
memset , .
, , , . , . , :
, . Android? Bien sûr que oui. :
proof :).
Android
FwdLockGlue_InitializeRoundKeys , .
static void FwdLockGlue_InitializeRoundKeys() { unsigned char keyEncryptionKey[KEY_SIZE]; .... memset(keyEncryptionKey, 0, KEY_SIZE);
PVS-Studio: V597 CWE-14 The compiler could delete the 'memset' function call, which is used to flush 'keyEncryptionKey' buffer. The memset_s() function should be used to erase the private data. FwdLockGlue.c 102
keyEncryptionKey . , . , , "
— ? ".
memset . «Zero out key data» , .
, release-
memset .
memset ,
memset .
10 Android_V597.txt .
, ,
memset .
void SHA1Transform(uint32_t state[5], const uint8_t buffer[64]) { uint32_t a, b, c, d, e; .... a = b = c = d = e = 0; }
PVS-Studio: V1001 CWE-563 The 'a' variable is assigned but is not used until the end of the function. sha1.c 213
PVS-Studio , , , .
CWE-563 : Assignment to Variable without Use. , , , , , CWE-14. , C C++ .
a ,
b ,
c ,
d e , .
Unspecified/implementation-defined behavior
, , .
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)); }
PVS-Studio: V793 It is odd that the result of the '(d >> 24) + 1' statement is a part of the condition. Perhaps, this statement should have been compared with something else. fixed.cpp 75
, 8
d , . , , , 0x00 0xFF.
. , , (d>>24). , : ((d>>24)+1). . , . C'est-à-dire d 0b11111111'00000000'00000000'00000000, 0b11111111'11111111'11111111'11111111. 1 0xFFFFFFFF
int , 0. : -1+1=0. , ((d>>24)+1) , 1. , , , :).
, .
«». : «The value of E1 >> E2 is E1 right-shifted E2 bit positions. If E1 has an unsigned type or if E1 has a signed type and a non-negative value, the value of the result is the integral part of the quotient of E1/2^E2. If E1 has a signed type and a negative value, the resulting value is implementation-defined».
. , , (implementation-defined behavior). , . , ((d>>24)+1) 0, .. .
: . , , , :
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)); }
, , , , , .
. , : .
: «
printf ?»
int i = 5; printf("%d,%d", i++, i++)
: . . , , Visual C++, «6,5», , :).
, . , , , 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: V681
CWE-758 The language standard does not define an order in which the 'readSigned' functions will be called during evaluation of arguments. ComposerClient.cpp 836
:
mHal.setLayerCursorPosition(...., readSigned(), readSigned());
readSigned . , . Unspecified Behavior.
PVS-Studio . . , . .
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},
PVS-Studio:
- V766 CWE-462 An item with the same key '«oem_lpass_cfg»' has already been added. bootstat.cpp 264
- V766 CWE-462 An item with the same key '«oem_xpu_ns_error»' has already been added. bootstat.cpp 265
(
std::map ) . Common Weakness Enumeration —
CWE-462 : Duplicate Key in Associative List.
, , , , .
, , .
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);
PVS-Studio: V525
CWE-682 The code contains the collection of similar blocks. Check items 'putInt8', 'putUInt8', 'putInt16', 'putUInt16', 'putInt32', 'putUInt32', 'putInt64', 'putUInt64', 'putInt128', 'putInt128' in lines 620, 623, 626, 629, 632, 635, 638, 641, 644, 647. android_mtp_MtpDatabase.cpp 620
MTP_TYPE_UINT128 putUInt128 ,
putInt128 .
Copy-Paste.
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; .... }
, .
, . , Java,
.
, , . : V778 CWE-682 Two similar code fragments were found. Perhaps, this is a typo and 'abort' variable should be used instead of 'continu'. btif_rc.cc 1554
, Copy-Paste, , , . "
continu " "
abort ".
C'est-à-dire :
avrc_rsp.abort.target_pdu = pavrc_cmd->abort.target_pdu;
"
", .
Facepalm
little-endian big-endian (.
).
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); .... }
PVS-Studio: V530 CWE-252 The return value of function 'bswap32' is required to be utilized. ELFAttribute.cpp 84
bswap32 . :
bswap32(subsection_length);
, . . .
CWE-252 : Unchecked Return Value. , ,
CWE-198 : Use of Incorrect Byte Ordering. , . , .
:
subsection_length = bswap32(subsection_length);
Android :
- V530 CWE-252 The return value of function 'bswap32' is required to be utilized. ELFAttribute.cpp 218
- V530 CWE-252 The return value of function 'bswap32' is required to be utilized. ELFAttribute.cpp 346
- V530 CWE-252 The return value of function 'bswap32' is required to be utilized. ELFAttribute.cpp 352
,
[[nodiscard]] . , , . , :
[[nodiscard]] inline uint32_t bswap32(uint32_t pData) { ... }
. "
++17 ".
, , .
Common Weakness Enumeration —
CWE-561 : Dead Code.
virtual sp<IEffect> createEffect(....) { .... if (pDesc == NULL) { return effect; if (status != NULL) { *status = BAD_VALUE; } } .... }
PVS-Studio: V779 CWE-561 Unreachable code detected. It is possible that an error is present. IAudioFlinger.cpp 733
return .
:
- V779 CWE-561 Unreachable code detected. It is possible that an error is present. bta_hf_client_main.cc 612
- V779 CWE-561 Unreachable code detected. It is possible that an error is present. android_media_ImageReader.cpp 468
- V779 CWE-561 Unreachable code detected. It is possible that an error is present. AMPEG4AudioAssembler.cpp 187
break
break switch — C C++ . , C++17 ,
[[fallthrough]] .
[[fallthrough]] "
break fallthrough ".
,
[[fallthrough]] , PVS-Studio. , Android. Common Weakness Enumeration,
CWE-484 : Omitted Break Statement in 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; .... }
PVS-Studio: V796 CWE-484 It is possible that 'break' statement is missing in switch statement. a2dp_vendor_ldac.cc 912
, . , . , V519:
- V519 CWE-563 The 'codec_capability_.sample_rate' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 910, 916. a2dp_vendor_ldac.cc 916
- V519 CWE-563 The 'codec_config_.sample_rate' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 911, 917. a2dp_vendor_ldac.cc 917
:
Return<void> EffectsFactory::getAllDescriptors(....) { .... switch (status) { case -ENOSYS: {
PVS-Studio: V796 CWE-484 It is possible that 'break' statement is missing in switch statement. 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; .... }
PVS-Studio: V796 CWE-484 It is possible that 'break' statement is missing in switch statement. 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; } .... }
PVS-Studio: V796 CWE-484 It is possible that 'break' statement is missing in switch statement. IAndroidConfiguration.cpp 90
, . , Common Weakness Enumeration, :
- CWE-401 : Improper Release of Memory Before Removing Last Reference ('Memory Leak')
- CWE-562 : Return of Stack Variable Address
- CWE-762 : Mismatched Memory Management Routines
, .
TransformIterator& operator++(int) { TransformIterator tmp(*this); ++*this; return tmp; } TransformIterator& operator--(int) { TransformIterator tmp(*this); --*this; return tmp; }
PVS-Studio:
- V558 CWE-562 Function returns the reference to temporary local object: tmp. transform_iterator.h 77
- V558 CWE-562 Function returns the reference to temporary local object: tmp. transform_iterator.h 92
,
tmp , . , () .
:
TransformIterator operator++(int) { TransformIterator tmp(*this); ++*this; return tmp; } TransformIterator operator--(int) { TransformIterator tmp(*this); --*this; return tmp; }
, .
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: V507 CWE-562 Pointer to local array 'buf' is stored outside the scope of this array. Such a pointer will become invalid. transport.cpp 1030
.
serial NULL, .
if ,
buf . , , , . , .
.
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; }
PVS-Studio: V611 CWE-762 The memory was allocated using 'new T[]' operator but was released using the 'delete' operator. Consider inspecting this code. It's probably better to use 'delete [] mEventCache;'. Check lines: 391, 384. SensorEventConnection.cpp 391
. ,
mEventCache ,
new [] . ,
delete . .
:
aaudio_result_t AAudioServiceEndpointCapture::open(....) { .... delete mDistributionBuffer; int distributionBufferSizeBytes = getStreamInternal()->getFramesPerBurst() * getStreamInternal()->getBytesPerFrame(); mDistributionBuffer = new uint8_t[distributionBufferSizeBytes]; .... }
PVS-Studio: V611 CWE-762 The memory was allocated using 'new T[]' operator but was released using the 'delete' operator. Consider inspecting this code. It's probably better to use 'delete [] mDistributionBuffer;'. AAudioServiceEndpointCapture.cpp 50
, .
, .
struct HeifFrameInfo { .... void set(....) { .... mIccData.reset(new uint8_t[iccSize]); .... } .... std::unique_ptr<uint8_t> mIccData; };
V554 CWE-762 Incorrect use of unique_ptr. La mémoire allouée avec «nouveau []» sera nettoyée à l'aide de «supprimer». HeifDecoderAPI.h 62
std::unique_ptr delete .
set new [] .
L'option correcte:
std::unique_ptr<uint8_t[]> mIccData;
Autres erreurs:
- V554 CWE-762 Incorrect use of unique_ptr. La mémoire allouée avec «nouveau []» sera nettoyée à l'aide de «supprimer». atrace.cpp 949
- V554 CWE-762 Incorrect use of unique_ptr. La mémoire allouée avec «nouveau []» sera nettoyée à l'aide de «supprimer». atrace.cpp 950
- V554 CWE-762 Incorrect use of unique_ptr. La mémoire allouée avec «nouveau []» sera nettoyée à l'aide de «supprimer». HeifDecoderImpl.cpp 102
- V554 CWE-762 Incorrect use of unique_ptr. La mémoire allouée avec «nouveau []» sera nettoyée à l'aide de «supprimer». HeifDecoderImpl.cpp 166
- V554 CWE-762 Incorrect use of unique_ptr. La mémoire allouée avec «nouveau []» sera nettoyée à l'aide de «supprimer». ColorSpace.cpp 360
, . , 20. , , .
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; }
PVS-Studio: V773 CWE-401 The function was exited without releasing the 'pAsset' pointer. Une fuite de mémoire est possible. Asset.cpp 296
chunk, ,
pAsset . .
, . :
Android_V773.txt .
, . Android :
if (i < 0 || i > MAX) return; A[i] = x;
C C++ 0, , . :
if (i < 0 || i >= MAX) return; A[i] = x;
, Common Weakness Enumeration,
CWE-119 : Improper Restriction of Operations within the Bounds of a Memory Buffer.
, 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; }
PVS-Studio: V557 CWE-119 Array overrun is possible. The value of 'idx' index could reach 6. btif_hf.cc 277
:
if (idx < 0 || idx >= BTA_AG_MAX_NUM_CLIENTS) {
:
- V557 CWE-119 Array overrun is possible. The value of 'idx' index could reach 6. btif_hf.cc 869
- V557 CWE-119 Array overrun is possible. The value of 'index' index could reach 6. btif_rc.cc 374
. Android , , Common Weakness Enumeration, :
- CWE-20 : Improper Input Validation
- CWE-670 : Always-Incorrect Control Flow Implementation
- CWE-691 : Insufficient Control Flow Management
- CWE-834 : Excessive Iteration
, , « » , .
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); } .... }
PVS-Studio: V739 CWE-20 EOF should not be compared with a value of the 'char' type. The '(c = fgetc(blk_alloc_file))' should be of the 'int' type. blk_alloc_to_base_fs.c 61
,
EOF char . , .
fgetc int , : 0 255 EOF (-1).
char . - 0xFF (255) -1 , (EOF).
- , Extended ASCII Codes, , . , Windows-1251 0xFF .
, , . ,
c int.
for .
status_t AudioPolicyManager::registerPolicyMixes(....) { .... for (size_t i = 0; i < mixes.size(); i++) { .... for (size_t j = 0; i < mHwModules.size(); j++) {
PVS-Studio: V534 CWE-691 It is likely that a wrong variable is being compared inside the 'for' operator. Consider reviewing 'i'. AudioPolicyManager.cpp 2489
-
i ,
j .
j ,
mHwModules . , , .
, . : AudioPolicyManager.cpp 2586.
3 , . , , . , .
:
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]; } .... } .... }
PVS-Studio: V535 CWE-691 The variable 'i' is being used for this loop and for the outer loop. Check lines: 398, 452. ce_t3t.cc 452
,
i , .
:
- V535 CWE-691 The variable 'xx' is being used for this loop and for the outer loop. Check lines: 801, 807. sdp_db.cc 807
- V535 CWE-691 The variable 'xx' is being used for this loop and for the outer loop. Check lines: 424, 438. nfa_hci_act.cc 438
?
PVS-Studio , .
.
#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); } .... }
PVS-Studio: V654 CWE-834 The condition 'gate_id <= 0xFF' of loop is always true. nfa_hci_utils.cc 248
:
- NFA_HCI_LAST_PROP_GATE 0xFF.
- uint8_t . , [0..0xFF].
,
gate_id <= NFA_HCI_LAST_PROP_GATE .
CWE-834, CWE-571: Expression is Always True.
.
status_t SimpleDecodingSource::doRead(....) { .... for (int retries = 0; ++retries; ) { .... }
PVS-Studio: V654 CWE-834 The condition '++ retries' of loop is always true. SimpleDecodingSource.cpp 226
, ,
retries int .
,
++retries 0. , . , . , . , .
.
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;
PVS-Studio: V696 CWE-670 The 'continue' operator will terminate 'do {… } while (FALSE)' loop because the condition is always false. Check lines: 105, 121. Vfat.cpp 105
:
do { .... if (x) continue; .... } while (0)
,
continue . .
continue , . , .
, , , :
for (;;) { .... if (x) continue; .... break; }
, . - Copy-Paste. Common Weakness Enumeration,
CWE-563 : Assignment to Variable without Use. 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: V519 CWE-563 The 'namespaceExt.prefix.index' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 1535, 1539. XMLNode.cpp 1539
, :
if (a > 0) X = 1; else X = 2; X = 1;
,
X (
namespaceExt.prefix.index ) .
bool AudioFlinger::RecordThread::threadLoop() { .... size_t framesToRead = mBufferSize / mFrameSize; framesToRead = min(mRsmpInFramesOA - rear, mRsmpInFramesP2 / 2); .... }
PVS-Studio: V519 CWE-563 The 'framesToRead' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 6341, 6342. Threads.cpp 6342
, , . - .
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; } .... }
PVS-Studio: V519 CWE-563 The 'last_visited_internal_latency_' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 680, 682. scheduler_arm.cc 682
, . , :
last_visited_internal_latency_ += kArmMemoryLoadLatency;
, , , code review.
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;
PVS-Studio: V519 CWE-563 The 'V' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 307, 309. p_256_multprecision.cc 309
« », . : , .
, . , .
void TagMonitor::parseTagsToMonitor(String8 tagNames) { std::lock_guard<std::mutex> lock(mMonitorMutex);
PVS-Studio: V593 CWE-783 Consider reviewing the expression of the 'A = B != C' kind. The expression is calculated as following: 'A = (B != C)'. TagMonitor.cpp 50
Common Weakness Enumeration:
CWE-783 : Operator Precedence Logic Error.
. «3a»
idx . (idx != -1), ,
idx .
, . :
if (ssize_t idx = (tagNames.find("3a") != -1))
, «3a», false/true
idx .
idx 0 1.
(
idx 1), ,
idx . 1 .
, :
ssize_t idx = tagNames.find("3a"); if (idx != -1)
C++17 :
if (ssize_t idx = tagNames.find("3a"); idx != -1)
struct HearingDevice { .... HearingDevice() { HearingDevice(RawAddress::kEmpty, false); } .... };
PVS-Studio: V603 CWE-665 The object was created but it is not being used. If you wish to call constructor, 'this->HearingDevice::HearingDevice(....)' should be used. hearing_aid.cc 176
Common Weakness Enumeration:
CWE-665 : Improper Initialization.
, . . . , .
.
HearingDevice . .
, ( C++11). :
HearingDevice() : HearingDevice(RawAddress::kEmpty, false) { }
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: V591 CWE-393 Non-void function should return a value. linux_close.cpp 139
Common Weakness Enumeration:
CWE-393 : Return of Wrong Status Code.
. : V591 CWE-393 Non-void function should return a value. linux_close.cpp 158
int MtpFfsHandle::handleControlRequest(....) { .... struct mtp_device_status *st = reinterpret_cast<struct mtp_device_status*>(buf.data()); st->wLength = htole16(sizeof(st)); .... }
PVS-Studio: V568 It's odd that 'sizeof()' operator evaluates the size of a pointer to a class, but not the size of the 'st' class object. MtpFfsHandle.cpp 251
, -
wLength , . :
st->wLength = htole16(sizeof(*st));
:
- V568 It's odd that 'sizeof()' operator evaluates the size of a pointer to a class, but not the size of the 'cacheinfo' class object. NetlinkEvent.cpp 220
- V568 It's odd that 'sizeof()' operator evaluates the size of a pointer to a class, but not the size of the 'page->next' class object. linker_block_allocator.cpp 146
- V568 It's odd that the argument of sizeof() operator is the '& session_id' expression. reference-ril.c 1775
#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); } .... }
PVS-Studio: V617 CWE-480 Consider inspecting the condition. The '0x00000001' argument of the '|' bitwise operation contains a non-zero value. egl.cpp 1329
Common Weakness Enumeration:
CWE-480 : Use of Incorrect Operator.
(A | 1) || (A | 2) || (A | 4) , true. ,
& , :
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))
: V617 CWE-480 Consider inspecting the condition. The '0x00000001' argument of the '|' bitwise operation contains a non-zero value. egl.cpp 1338
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]; } .... }
PVS-Studio: V629 CWE-190 Consider inspecting the '1 << reg' expression. Bit shifting of the 32-bit value with a subsequent expansion to the 64-bit type. RegsInfo.h 47
Common Weakness Enumeration:
CWE-190 : Integer Overflow or Wraparound.
1 << reg reg [0..63]. , , 2^0 2^63.
. , 1 32-
int . 1^31. .
:
saved_reg_map |= static_cast<uint64_t>(1) << reg;
:
saved_reg_map |= 1ULL << reg;
void PCLmGenerator::writeJobTicket() {
PVS-Studio:
- V549 CWE-688 The first argument of 'strcpy' function is equal to the second argument. genPCLm.cpp 1181
- V549 CWE-688 The first argument of 'strcpy' function is equal to the second argument. genPCLm.cpp 1182
Common Weakness Enumeration:
CWE-688 : Function Call With Incorrect Variable or Reference as Argument.
- . , - .
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));
PVS-Studio: V614 CWE-824 Potentially uninitialized pointer 'p_opt' used. Consider checking the second actual argument of the 'memcpy' function. mca_main.cc 252
Common Weakness Enumeration:
CWE-824 : Access of Uninitialized Pointer.
p_tbl->tcid != MCA_CTRL_TCID p_dcb == nullptr ,
p_opt .
struct timespec { __time_t tv_sec; long int tv_nsec; }; 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; }
PVS-Studio: V614 CWE-457 Uninitialized variable 't.tv_nsec' used. clock_ns.h 55
Common Weakness Enumeration:
CWE-457 : Use of Uninitialized Variable.
t.tv_nsec . :
t.tv_nsec = remainder; . - .
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: V578 An odd bitwise operation detected. Consider verifying it. bta_dm_co.cc 259
.
(bte_appl_cfg.ble_auth_req & 0x04) , . , - .
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; }
PVS-Studio: V773 CWE-401 The function was exited without releasing the 'pfin' handle. Une fuite de ressource est possible. slang_rs_reflection_cpp.cpp 448
, Common Weakness Enumeration, :
CWE-401 : Improper Release of Memory Before Removing Last Reference ('Memory Leak').
CWE-775 : Missing Release of File Descriptor or Handle after Effective Lifetime. PVS-Studio.
pfin .
fclose . , , .
Conclusion
, , Android, PVS-Studio . , weakness ( ) :
- CWE-14: Compiler Removal of Code to Clear Buffers
- CWE-20: Improper Input Validation
- CWE-119: Improper Restriction of Operations within the Bounds of a Memory Buffer
- CWE-190: Integer Overflow or Wraparound
- CWE-198: Use of Incorrect Byte Ordering
- CWE-393: Return of Wrong Status Code
- CWE-401: Improper Release of Memory Before Removing Last Reference ('Memory Leak')
- CWE-457: Use of Uninitialized Variable
- CWE-462: Duplicate Key in Associative List
- CWE-480: Use of Incorrect Operator
- CWE-484: Omitted Break Statement in Switch
- CWE-561: Dead Code
- CWE-562: Return of Stack Variable Address
- 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
490 . , , , , .
2168000 C C++. 14,4% — . , 1855000 .
, 490 CWE 1855000 .
, PVS-Studio 1 weakness ( ) 4000 Android. , .
Merci de votre attention! :
- PVS-Studio .
- : : .
- , : twitter , RSS , vk.com .
Si vous souhaitez partager cet article avec un public anglophone, veuillez utiliser le lien vers la traduction: Andrey Karpov.
We Checked the Android Source Codes by PVS-Studio or Nothing is Perfect