A. A.
Die Entwicklung großer komplexer Projekte ist ohne den Einsatz von Programmiermethoden und -werkzeugen zur Kontrolle der Codequalität nicht möglich. Erstens ist es ein kompetenter Codierungsstandard, Codeüberprüfungen, Komponententests, statische und dynamische Codeanalysatoren. All dies hilft, Fehler im Code in den frühesten Entwicklungsstadien zu identifizieren. Dieser Artikel beschreibt die Funktionen des statischen Analysators PVS-Studio zum Erkennen von Fehlern und potenziellen Schwachstellen im Code des Android-Betriebssystems. Wir hoffen, dass der Artikel die Leser für die Methodik der statischen Code-Analyse attraktiv macht und sie bei der Entwicklung ihrer eigenen Projekte implementieren möchte.
Einführung
Ein Jahr ist vergangen, seit ein großer
Artikel über Fehler im Tizen-Betriebssystem geschrieben wurde, und ich wollte noch einmal eine ebenso interessante Studie über ein Betriebssystem durchführen. Die Wahl fiel auf Android.
Der Code des Android-Betriebssystems ist qualitativ hochwertig und gut getestet. Während der Entwicklung wird mindestens ein statischer Coverity-Code-Analysator verwendet, wie aus den Kommentaren des Formulars hervorgeht:
Im Allgemeinen ist dies ein interessantes und qualitativ hochwertiges Projekt, und das Auffinden von Fehlern ist eine Herausforderung für unseren statischen Analysator PVS-Studio.
Ich denke, dass der Leser nur durch den Umfang des Artikels versteht, dass der PVS-Studio-Analysator hervorragende Arbeit geleistet hat und viele Fehler im Code dieses Betriebssystems gefunden hat.
Aufzählung der allgemeinen Schwächen
In dem Artikel finden Sie Verweise auf die
Common Weakness Enumeration (CWE). Lassen Sie uns den Grund für die Bezugnahme auf diese Liste erläutern und erklären, warum dies aus Sicherheitsgründen wichtig ist.
Oft ist die Ursache für Schwachstellen in Programmen nicht eine schwierige Situation, sondern ein banaler Softwarefehler. Hier ist es angebracht, dieses Zitat von
prqa.com zu zitieren:
Das Nationale Institut für Standards und Technologie (NIST) berichtet, dass 64% der Software-Schwachstellen auf Programmierfehler und nicht auf fehlende Sicherheitsfunktionen zurückzuführen sind.
Sie können im Artikel „
Wie PVS-Studio helfen kann, Schwachstellen zu finden? “ Lesen. Einige Beispiele für einfache Fehler, die zu Schwachstellen in Projekten wie MySQL, iOS, NAS und illumos-gate führten.
Dementsprechend können viele Schwachstellen beseitigt werden, indem häufige Fehler rechtzeitig erkannt und behoben werden. Und hier kommt die Common Weakness Enumeration ins Spiel.
Fehler sind unterschiedlich und nicht alle Fehler sind aus Sicherheitsgründen gefährlich. Fehler, die möglicherweise zu Sicherheitslücken führen können, werden in der Common Weakness Enumeration erfasst. Diese Liste wird aktualisiert, und es gibt wahrscheinlich Fehler, die zu Sicherheitslücken führen können, die jedoch noch nicht in diese Liste aufgenommen wurden.
Wenn der Fehler jedoch gemäß CWE klassifiziert wird, bedeutet dies, dass es theoretisch möglich ist, ihn als Sicherheitsanfälligkeit (
CVE )
auszunutzen . Ja, die Wahrscheinlichkeit dafür ist gering. Sehr selten verwandelt sich CWE in CVE. Wenn Sie Ihren Code jedoch vor Sicherheitslücken schützen möchten, sollten Sie nach Möglichkeit so viele in CWE beschriebene Fehler wie möglich finden und beheben.
Die Beziehung zwischen PVS-Studio, Fehlern, CWE und CVE ist schematisch in der Abbildung dargestellt:
Einige Fehler werden als CWE klassifiziert. PVS-Studio kann viele dieser Fehler erkennen und so verhindern, dass einige dieser Fehler zu Sicherheitslücken (CVE) werden.
Wir können sagen, dass PVS-Studio viele potenzielle Schwachstellen identifiziert, bevor sie Schaden anrichten. Somit ist PVS-Studio ein statisches Tool zum Testen der Anwendungssicherheit (
SAST ).
Nun, ich denke, es ist klar, warum ich es bei der Beschreibung der Fehler für wichtig hielt, zu beachten, wie sie gemäß CWE klassifiziert werden. Auf diese Weise ist es einfacher zu zeigen, wie wichtig es ist, einen statischen Code-Analysator in kritischen Projekten zu verwenden, auf die sich Betriebssysteme eindeutig beziehen.
Android-Check
Zur Analyse des Projekts habe ich den PVS-Studio Analyzer Version 6.24 verwendet. Der Analysator unterstützt derzeit die folgenden Sprachen und Compiler:
- Windows Visual Studio 2010-2017 C, C ++, C ++ / CLI, C ++ / CX (WinRT), C #
- Windows IAR Embedded Workbench, C / C ++ - Compiler für ARM C, C ++
- Windows / Linux Keil µVision, DS-MDK, ARM-Compiler 5/6 C, C ++
- Windows / Linux Code Composer Studio von Texas Instruments, Tools zur ARM-Codegenerierung C, C ++
- Windows / Linux / macOS. Clang C, C ++
- Linux / MacOS. GCC C, C ++
- Windows MinGW C, C ++
Hinweis Vielleicht haben einige unserer Leser die Nachricht verpasst, dass wir die Arbeit in der macOS-Umgebung unterstützt haben, und sie werden an dieser Veröffentlichung interessiert sein: "
PVS-Studio-Version für macOS: 64 Schwachstellen im Apple XNU-Kernel ".
Das Überprüfen des Android-Quellcodes war kein Problem, daher werde ich nicht weiter darauf eingehen. Das Problem war vielmehr meine Beschäftigung mit anderen Aufgaben, weshalb ich nicht die Zeit und Energie fand, den Bericht so sorgfältig zu betrachten, wie ich es gerne hätte. Ein kurzer Blick war jedoch mehr als genug, um eine große Sammlung interessanter Fehler für einen soliden Artikel zu sammeln.
Am wichtigsten: Ich fordere Android-Entwickler auf, nicht nur die im Artikel beschriebenen Fehler zu korrigieren, sondern auch eine sorgfältigere unabhängige Analyse durchzuführen. Ich habe den Analysebericht oberflächlich betrachtet und viele schwerwiegende Fehler übersehen können.
Beim ersten Test erzeugt der Analysator viele falsch positive Ergebnisse,
dies ist jedoch kein Problem . Unser Team hilft Ihnen gerne mit Empfehlungen zur Konfiguration des Analysators, um die Anzahl der Fehlalarme zu verringern. Bei Bedarf stellen wir Ihnen auch einen Lizenzschlüssel für einen Monat oder länger zur Verfügung.
Schreiben Sie uns im Allgemeinen, wir helfen Ihnen und sagen es Ihnen.
Nun wollen wir sehen, welche Fehler und potenziellen Schwachstellen ich gefunden habe. Ich hoffe, Ihnen gefällt, was der statische Code-Analysator PVS-Studio finden kann. Viel Spaß beim Lesen.
Sinnlose Vergleiche
Der Analysator betrachtet die Ausdrücke als abnormal, wenn sie immer wahr oder falsch sind. Solche Warnungen werden gemäß der Common Weakness Enumeration wie folgt klassifiziert:
Der Analysator generiert viele solcher Warnungen, und leider sind die meisten für Android-Code falsch. In diesem Fall ist der Analysator nicht schuld. Nur der Code ist so geschrieben.
Ich werde dies anhand eines einfachen Beispiels demonstrieren.
#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') { .... }
Hier generiert der Analysator eine Warnung: V547 CWE-570 Der Ausdruck 'alternative_config_path [0]! =' \ 0 '' ist immer falsch. phNxpConfig.cpp 401
Tatsache ist, dass das Makro
GENERIC_TARGET nicht definiert ist und aus Sicht des Analysators der Code folgendermaßen aussieht:
const char alternative_config_path[] = ""; .... if (alternative_config_path[0] != '\0') {
Der Analysator muss lediglich eine Warnung ausgeben, da die Leitung leer ist und sich am Nullpunktversatz immer eine Klemmennull befindet. Somit hat der Analysator formal Recht, eine Warnung auszugeben. Aus praktischer Sicht bietet diese Warnung jedoch keinen Nutzen.
Leider kann mit solchen Situationen nichts gemacht werden. Wir müssen alle diese Warnungen systematisch überprüfen und viele Stellen als falsch positiv markieren, damit der Analysator keine Nachrichten mehr in diesen Zeilen ausgibt. Dies muss wirklich getan werden, da neben bedeutungslosen Nachrichten viele echte Mängel festgestellt werden.
Ich gebe ehrlich zu, dass ich nicht daran interessiert war, Warnungen dieser Art sorgfältig zu prüfen, und habe sie oberflächlich durchgesehen. Selbst dies wird jedoch ausreichen, um zu zeigen, dass solche Diagnosen sehr nützlich sind und interessante Fehler finden.
Ich möchte mit der klassischen Situation beginnen, in der die Funktion zum Vergleichen zweier Objekte falsch implementiert ist. Warum klassisch? Dies ist ein typisches Fehlermuster, auf das wir in einer Vielzahl von Projekten ständig stoßen. Höchstwahrscheinlich gibt es drei Gründe für sein Auftreten:
- Die Vergleichsfunktionen sind einfach und werden „automatisch“ und mithilfe der Copy-Paste-Technologie geschrieben. Beim Schreiben eines solchen Codes ist eine Person unaufmerksam und macht häufig Tippfehler.
- In der Regel wird für solche Funktionen keine Codeüberprüfung durchgeführt, da es zu faul ist, einfache und langweilige Funktionen zu betrachten.
- Unit-Tests werden für solche Funktionen normalerweise nicht durchgeführt. Weil Faulheit. Darüber hinaus sind die Funktionen einfach und Programmierer glauben nicht, dass Fehler in ihnen möglich sind.
Diese Gedanken und Beispiele werden im Artikel "Das
Böse lebt in den Funktionen des Vergleichs " ausführlicher beschrieben.
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; }
Vor uns liegt also die klassische Funktion, zwei Objekte vom Typ
AudioPlaybackRate zu vergleichen . Und wie ich denke, vermutet der Leser, dass es falsch ist. Der PVS-Studio-Analysator bemerkt sofort zwei Tippfehler:
- V501 CWE-571 Links und rechts vom Operator '==' befinden sich identische Unterausdrücke: pr2.mStretchMode == pr2.mStretchMode AudioResamplerPublic.h 107
- V501 CWE-571 Links und rechts vom Operator '==' befinden sich identische Unterausdrücke: pr2.mFallbackMode == pr2.mFallbackMode AudioResamplerPublic.h 108
Das Feld
pr2.mStretchMode und das Feld
pr2.mFallbackMode werden mit sich selbst verglichen. Es stellt sich heraus, dass die Funktion Objekte nicht genau genug vergleicht.
Der folgende sinnlose Vergleich befindet sich in einer Funktion, die Fingerabdruckinformationen in einer Datei speichert.
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)
In diesem Code wird eine Anomalie durch zwei Diagnosen gleichzeitig erkannt:
- V501 CWE-570 Links und rechts vom '||' befinden sich identische Unterausdrücke. Operator: ns! = 1 || ns! = 1 Fingerabdruck.c 126
- V560 CWE-570 Ein Teil des bedingten Ausdrucks ist immer falsch: ns! = 1. Fingerabdruck.c 126
Es gibt keine Behandlung der Situation, in der der zweite Aufruf der Funktion
fwrite keine Daten in eine Datei schreiben kann. Mit anderen Worten wird der Wert der Variablen
nf nicht überprüft. Die richtige Prüfung sollte folgendermaßen aussehen:
if (ns != 1 || nf != 1)
Wir fahren mit dem nächsten Fehler fort, der mit der Verwendung des Operators
& verbunden ist .
#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; } .... }
PVS-Studio-Warnung: V560 CWE-570 Ein Teil des bedingten Ausdrucks ist immer falsch: f-> mode & 00000000. fec_read.cpp 322
Beachten Sie, dass die Konstante
O_RDONLY Null ist. Dies macht den Ausdruck
f-> mode & O_RDONLY sinnlos, da er immer 0 ist. Es stellt sich heraus, dass die Bedingung der
if-Anweisung niemals erfüllt ist und Anweisung-true toter Code ist.
Die richtige Prüfung sollte folgendermaßen aussehen:
if (f->mode == O_RDONLY && expect_zeros) {
Schauen wir uns nun einen klassischen Tippfehler an, als wir vergessen haben, einen Teil der Bedingung zu schreiben.
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 Warnung: V768 CWE-571 Die Aufzählungskonstante 'CHANGE_DISPLAY_INFO' wird als Variable eines Booleschen Typs verwendet. InputReader.cpp 3016
Die Bedingung ist immer wahr, da der Operand
InputReaderConfiguration :: CHANGE_DISPLAY_INFO eine Konstante gleich 4 ist.
Wenn Sie sich ansehen, wie der Code in der Nachbarschaft geschrieben ist, wird klar, dass die Bedingung tatsächlich so sein sollte:
if (!changes || (changes & InputReaderConfiguration::CHANGE_DISPLAY_INFO)) {
Den folgenden Vergleich, der keinen Sinn ergibt, habe ich im Schleifenoperator getroffen.
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); } } .... }
PVS-Studio Warnung: V560 CWE-571 Ein Teil des bedingten Ausdrucks ist immer wahr: (j <4). ipphelper.c 926
Beachten Sie, dass der Wert der Variablen
j nirgendwo erhöht wird. Dies bedeutet, dass der Unterausdruck
(j <4) immer wahr ist.
Die größte Anzahl nützlicher Operationen des PVS-Studio-Analysators, die immer wahre / falsche Bedingungen betreffen, bezieht sich auf den Code, in dem das Ergebnis der Erstellung von Objekten mit dem
neuen Operator überprüft wird. Mit anderen Worten, der Analysator erkennt das folgende Codemuster:
T *p = new T; if (p == nullptr) return ERROR;
Solche Überprüfungen sind sinnlos. Wenn es nicht möglich war, Speicher für das Objekt zuzuweisen, wird eine Ausnahme vom Typ
std :: bad_alloc ausgelöst, und der Zeigerwert wird nur nicht überprüft.
Hinweis Der
neue Operator kann
nullptr zurückgeben, indem er
new (std :: nothrow) T schreibt. Dies gilt jedoch nicht für die besprochenen Fehler. Der PVS-Studio-Analysator berücksichtigt
(std :: nothrow) und gibt keine Warnung aus, wenn das Objekt auf diese Weise erstellt wird.
Es scheint, dass solche Fehler harmlos sind. Denken Sie daran, ein zusätzlicher Scheck, der niemals funktioniert. Auf jeden Fall wird eine Ausnahme ausgelöst, die irgendwo verarbeitet wird. Leider setzen einige Entwickler in die Anweisung-true der
if-Anweisungsaktionen , die Ressourcen usw. freisetzen. Da dieser Code nicht ausgeführt wird, kann dies zu Speicherlecks und anderen Fehlern führen.
Betrachten Sie einen dieser Fälle, die mir im Android-Code aufgefallen sind.
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; } .... }
PVS-Studio Warnung: V668 CWE-570 Es macht keinen Sinn, den 'buf'-Zeiger gegen null zu testen, da der Speicher mit dem' neuen 'Operator zugewiesen wurde. Die Ausnahme wird bei einem Speicherzuordnungsfehler generiert. scan.cpp 213
Bitte beachten Sie, dass der Programmierer versucht, den ersten Block freizugeben, wenn es nicht möglich ist, einen zweiten Speicherblock zuzuweisen:
delete dataMap;
Dieser Code wird niemals die Kontrolle bekommen. Das ist toter Code. Wenn eine Ausnahme auftritt, tritt ein Speicherverlust auf.
Das Schreiben eines solchen Codes ist grundsätzlich falsch. Für solche Fälle
wurden intelligente Zeiger erfunden.
Insgesamt hat der PVS-Studio-Analysator
176 Stellen im Android-Code erkannt, an denen der Zeiger überprüft wird, nachdem Objekte mit
new erstellt wurden . Ich habe nicht verstanden, wie gefährlich jeder dieser Orte ist, und natürlich werde ich den Artikel nicht mit all diesen Warnungen überladen.
Interessenten können andere Warnungen in der Datei
Android_V668.txt sehen .
Dereferenzieren eines Nullzeigers
Das Dereferenzieren eines Nullzeigers führt zu einem undefinierten Programmverhalten. Daher ist es hilfreich, solche Stellen zu finden und zu beheben. Abhängig von der Situation kann der PVS-Studio-Analysator diese Fehler gemäß der Common Weakness Enumeration wie folgt klassifizieren:
- CWE-119 : Unsachgemäße Einschränkung von Operationen innerhalb der Grenzen eines Speicherpuffers
- CWE-476 : NULL-Zeiger-Dereferenzierung
- CWE-628 : Funktionsaufruf mit falsch angegebenen Argumenten
- CWE-690 : Deaktivierter Rückgabewert für NULL-Zeiger-Dereferenzierung
Ich finde solche Fehler oft im Code, der für den Umgang mit nicht standardmäßigen oder falschen Situationen verantwortlich ist. Niemand testet solchen Code, und der Fehler kann sehr lange darin leben. Ein solcher Fall wird jetzt betrachtet.
bool parseEffect(....) { .... if (xmlProxyLib == nullptr) { ALOGE("effectProxy must contain a <%s>: %s", tag, dump(*xmlProxyLib)); return false; } .... }
PVS-Studio Warnung: V522 CWE-476 Es kann zu einer Dereferenzierung des Nullzeigers 'xmlProxyLib' kommen. EffectsConfig.cpp 205
Wenn der
xmlProxyLib- Zeiger
nullptr ist , zeigt der Programmierer eine Debug-Meldung an, für die dieser Zeiger dereferenziert werden muss. Ups ...
Betrachten Sie nun eine interessantere Version des Fehlers.
static void soinfo_unload_impl(soinfo* root) { .... soinfo* needed = find_library(si->get_primary_namespace(), library_name, RTLD_NOLOAD, nullptr, nullptr); if (needed != nullptr) {
PVS-Studio Warnung: V522 CWE-476 Es kann zu einer Dereferenzierung des Nullzeigers 'benötigt' kommen. linker.cpp 1847
Wenn der Zeiger
benötigt wird! = Nullptr , wird eine Warnung ausgegeben, was ein sehr verdächtiges Verhalten des Programms ist. Es wird schließlich klar, dass der Code einen Fehler enthält, wenn Sie unten
nachsehen , dass bei
Bedarf == nullptr der Nullzeiger in dem Ausdruck
erforderlich-> is_linked () dereferenziert wird.
Höchstwahrscheinlich sind die Operatoren! = Und == hier einfach verwirrt. Wenn Sie ersetzen, ist der Funktionscode sinnvoll und der Fehler verschwindet.
Der Großteil der Warnungen bezüglich der möglichen Dereferenzierung eines Nullzeigers bezieht sich auf eine Situation der Form:
T *p = (T *) malloc (N); *p = x;
Funktionen wie
malloc ,
strdup usw. können
NULL zurückgeben, wenn kein Speicher zugewiesen werden kann. Daher können Sie Zeiger, die diese Funktionen zurückgeben, nicht dereferenzieren, ohne zuvor den Zeiger zu überprüfen.
Es gibt viele ähnliche Fehler, daher werde ich nur zwei einfache Codefragmente angeben: das erste mit
malloc und der zweite mit
strdup .
DownmixerBufferProvider::DownmixerBufferProvider(....) { .... effect_param_t * const param = (effect_param_t *) malloc(downmixParamSize); param->psize = sizeof(downmix_params_t); .... }
PVS-Studio Warnung: V522 CWE-690 Möglicherweise wird ein potenzieller Nullzeiger 'param' dereferenziert. Überprüfen Sie die Zeilen: 245, 244. BufferProviders.cpp 245
static char* descriptorClassToDot(const char* str) { .... newStr = strdup(lastSlash); newStr[strlen(lastSlash)-1] = '\0'; .... }
PVS-Studio Warnung: V522 CWE-690 Möglicherweise wird ein potenzieller Nullzeiger 'newStr' dereferenziert. Überprüfen Sie die Zeilen: 203, 202. DexDump.cpp 203
Jemand könnte sagen, dass dies kleinere Fehler sind. Wenn nicht genügend Speicher vorhanden ist, stürzt das Programm beim Dereferenzieren des Nullzeigers einfach ab. Dies ist normal. Da es keinen Speicher gibt, gibt es nichts zu versuchen, mit dieser Situation irgendwie umzugehen.
Eine solche Person ist falsch. Zeiger müssen überprüft werden! Ich habe dieses Thema im Artikel „
Warum ist es wichtig zu überprüfen, was die Malloc-Funktion zurückgegeben hat “ ausführlich untersucht. Ich empfehle Ihnen dringend, es allen vorzulesen, die es nicht gelesen haben.
Kurz gesagt besteht die Gefahr, dass das Schreiben in den Speicher nicht unbedingt in der Nähe der Nulladresse erfolgt. Es ist möglich, Daten irgendwo sehr weit in eine Speicherseite zu schreiben, die nicht schreibgeschützt ist, und dadurch einen schwer fassbaren Fehler zu verursachen, oder im Allgemeinen kann dieser Fehler als Sicherheitsanfälligkeit verwendet werden. Mal sehen, was ich am Beispiel der Funktion
check_size meine .
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)); .... }
PVS-Studio-Warnung: V769 CWE-119 Der Zeiger '(uint32_t *) Metadaten' im Ausdruck '(uint32_t *) Metadaten + new_size_int' könnte nullptr sein. In diesem Fall ist der resultierende Wert sinnlos und sollte nicht verwendet werden. Überprüfen Sie die Zeilen: 91, 89. radio_metadata.c 91
Ich habe die Logik der Funktion nicht verstanden, aber das ist nicht notwendig. Hauptsache, es wird ein neuer Puffer erstellt und die Daten dort kopiert. Wenn die
Realloc- Funktion
NULL zurückgibt, werden die Daten an die Adresse ((uint32_t *) NULL + Metadaten-> size_int - (Metadaten-> Anzahl + 1)) kopiert.
Wenn der
Wert für metadata-> size_int groß ist, sind die Konsequenzen bedauerlich. Es stellt sich heraus, dass die Daten in einen zufälligen Speicher geschrieben werden.
Übrigens gibt es eine andere Art der Nullzeiger-Dereferenzierung, die der PVS-Studio-Analysator nicht als CWE-690, sondern als CWE-628 (ungültiges Argument) klassifiziert.
static void parse_tcp_ports(const char *portstring, uint16_t *ports) { char *buffer; char *cp; buffer = strdup(portstring); if ((cp = strchr(buffer, ':')) == NULL) .... }
PVS-Studio Warnung: V575 CWE-628 Der potenzielle Nullzeiger wird an die Funktion 'strchr' übergeben. Überprüfen Sie das erste Argument. Überprüfen Sie die Zeilen: 47, 46. libxt_tcp.c 47
Tatsache ist, dass eine Zeiger-Dereferenzierung auftritt, wenn die
strchr- Funktion
aufgerufen wird . Daher interpretiert der Analysator diese Situation so, dass er einen falschen Wert an die Funktion übergibt.
Die restlichen
194 Warnungen dieses Typs liste ich in der Datei
Android_V522_V575.txt auf .
Eine besondere Pikantheit für all diese Fehler bieten übrigens die zuvor erläuterten Warnungen zum Überprüfen des Zeigers nach dem Aufrufen des
neuen Operators. Es stellt sich heraus, dass 195 Aufrufe der Funktionen
malloc /
realloc /
strdup usw.
vorliegen , wenn der Zeiger nicht
aktiviert ist. Es gibt jedoch 176 Stellen, an denen der Zeiger nach dem Aufruf von
new überprüft wird. Stimmen Sie zu, ein seltsamer Ansatz!
Am Ende müssen wir nur die Warnungen V595 und V1004 berücksichtigen, die auch mit der Verwendung von Nullzeigern verbunden sind.
V595 erkennt Situationen, in denen der Zeiger dereferenziert und dann überprüft wird. Synthetisches Beispiel:
p->foo(); if (!p) Error();
V1004 zeigt die umgekehrte Situation an, als der Zeiger zuerst überprüft und dann vergessen wurde. Synthetisches Beispiel:
if (p) p->foo(); p->doo();
Schauen wir uns einige Fragmente des Android-Codes an, bei denen Fehler dieses Typs aufgetreten sind.
Speziell erklären, dass ihre Funktionen nicht erforderlich sind. 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 Warnung: V595 CWE-476 Der 'Video'-Zeiger wurde verwendet, bevor er gegen nullptr verifiziert wurde. Überprüfen Sie die Zeilen: 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 Warnung: V595 CWE-476 Der Zeiger 'rsmp' wurde verwendet, bevor er gegen nullptr verifiziert wurde. Überprüfen Sie die Zeilen: 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-Warnung: V1004 CWE-476 Der Zeiger 'p_clcb-> p_srcb' wurde unsicher verwendet, nachdem er gegen nullptr überprüft wurde. Überprüfen Sie die Zeilen: 695, 701. bta_gattc_act.cc 701Es ist nicht interessant, andere Warnungen dieses Typs zu berücksichtigen. Darunter befinden sich sowohl Fehler als auch falsche Warnungen, die aufgrund von schlecht oder schwer geschriebenem Code auftreten.Ich habe ein Dutzend nützliche Warnungen geschrieben:- 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
Und dann langweilte ich mich und filterte Warnungen dieser Art heraus. Daher weiß ich nicht einmal, wie viele echte Fehler der Analysator erkannt hat. Diese Warnungen warten auf ihren Helden, der sie sorgfältig studiert und Änderungen am Code vornimmt.Ich möchte meine neuen Leser nur auf Fehler dieser Art aufmerksam machen: 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 Warnung: V595 CWE-476 Der Zeiger 'iwnn' wurde verwendet, bevor er gegen nullptr verifiziert wurde. Überprüfen Sie die Zeilen: 686, 689. ndapi.c 686Einige Leute denken, dass hier kein Fehler vorliegt, weil "es keine echte Dereferenzierung des Zeigers gibt". Die Adresse einer nicht vorhandenen Variablen wird einfach berechnet. Wenn der Zeiger iwnn Null ist, wird die Funktion beendet. Daher ist nichts Schlimmes passiert, dass wir zuvor die Adresse eines Klassenmitglieds falsch berechnet haben.Nein, so kannst du nicht reden. Dieser Code führt zu undefiniertem Verhalten und kann daher nicht so geschrieben werden. Undefiniertes Verhalten kann sich beispielsweise wie folgt manifestieren:- Der Compiler sieht den Zeiger dereferenziert: iwnn-> previous_selection
- Sie können einen Nullzeiger nicht dereferenzieren, da dies ein undefiniertes Verhalten ist
- Der Compiler kommt zu dem Schluss, dass der iwnn- Zeiger immer ungleich Null ist
- Der Compiler entfernt die zusätzliche Prüfung: if (iwnn == NULL)
- Jetzt, wenn das Programm ausgeführt wird, wird die Prüfung auf den Nullzeiger nicht durchgeführt, und die Arbeit beginnt mit dem falschen Zeiger auf das Klassenmitglied
Dieses Thema wird in meinem Artikel " Dereferenzieren eines Nullzeigers führt zu undefiniertem Verhalten " ausführlicher beschrieben .Private Daten werden nicht im Speicher gelöscht
Stellen Sie sich eine schwerwiegende potenzielle Sicherheitsanfälligkeit vor, die gemäß der Common Weakness Enumeration als CWE-14 : Compiler-Entfernung von Code zum Löschen von Puffern klassifiziert wird .Kurz gesagt, das Fazit lautet: Der Compiler hat das Recht, den Memset- Funktionsaufruf zu entfernen, wenn der Puffer danach nicht mehr verwendet wird.Wenn ich über diese Art von Sicherheitsanfälligkeit schreibe, erscheinen Kommentare notwendigerweise, dass dies nur ein Fehler im Compiler ist, der behoben werden muss. Nein, das ist keine Panne. Bevor Sie Einwände erheben, lesen Sie bitte die folgenden Materialien:Im Allgemeinen ist alles ernst. Gibt es solche Fehler in Android? Natürlich gibt es.
Es gibt im Allgemeinen viele, wo es gibt: Beweise :). Kehrenwir zum Android-Code zurück und schauen uns den Anfang und das Ende der Funktion FwdLockGlue_InitializeRoundKeys an , da die Mitte für uns nicht interessant ist. static void FwdLockGlue_InitializeRoundKeys() { unsigned char keyEncryptionKey[KEY_SIZE]; .... memset(keyEncryptionKey, 0, KEY_SIZE);
PVS-Studio Warnung: V597 CWE-14 Der Compiler könnte den Funktionsaufruf 'memset' löschen, mit dem der Puffer 'keyEncryptionKey' geleert wird. Die Funktion memset_s () sollte verwendet werden, um die privaten Daten zu löschen. FwdLockGlue.c 102Das Array keyEncryptionKey wird auf dem Stapel erstellt und speichert private Informationen. Am Ende der Funktion möchten sie dieses Array mit Nullen füllen, damit es nicht versehentlich dort ankommt, wo es nicht sein sollte. Wie Informationen dahin gelangen können, wo sie nicht sein sollten, wird der Artikel " Speicher überschreiben - warum? "Um ein Array mit privaten Informationen mit Nullen zu füllen, wird die Memset- Funktion verwendet . Der Kommentar „Schlüsseldaten auf Null setzen“ bestätigt, dass wir alles richtig verstehen.Das Problem ist, dass der Compiler beim Erstellen der Release-Version mit sehr hoher Wahrscheinlichkeit den Memset- Funktionsaufruf löscht . Da der Puffer nach dem Memset- Aufruf nicht verwendet wird, ist der Aufruf der Memset- Funktion selbst aus Sicht des Compilers überflüssig.Weitere 10 Warnungen schrieb ich eine Datei Android_V597.txt .Es gab einen weiteren Fehler, bei dem der Speicher nicht überschrieben wurde, obwohl in diesem Fall die Memset- Funktion nichts damit zu tun hat. 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 Warnung: V1001 CWE-563 Die Variable 'a' wird zugewiesen, aber erst am Ende der Funktion verwendet. sha1.c 213PVS-Studio hat eine Anomalie festgestellt, die darauf zurückzuführen ist, dass Variablen nach dem Zuweisen von Werten nicht mehr verwendet werden. Der Analysator klassifizierte diesen Defekt als CWE-563 : Zuordnung zu Variable ohne Verwendung. Und formal hat er Recht, obwohl es sich hier tatsächlich wieder um CWE-14 handelt. Der Compiler wird diese Zuweisungen wegwerfen, da sie aus Sicht von C und C ++ überflüssig sind. Infolgedessen behält der Stapel die alten Werte der Variablen a , b , c , d und e bei , in denen private Daten gespeichert sind.Nicht spezifiziertes / implementierungsdefiniertes Verhalten
Während Sie nicht müde sind, schauen wir uns einen komplexen Fall an, der eine gründliche Beschreibung meinerseits erfordert. 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-Warnung: V793 Es ist seltsam, dass das Ergebnis der Anweisung '(d >> 24) + 1' Teil der Bedingung ist. Vielleicht hätte diese Aussage mit etwas anderem verglichen werden sollen. fixed.cpp 75Der Programmierer wollte überprüfen, ob die 8 höherwertigen Bits der Variablen d Einheiten enthalten, aber nicht alle Bits gleichzeitig. Mit anderen Worten, der Programmierer wollte überprüfen, ob sich ein anderer Wert als 0x00 und 0xFF im High-Byte befindet.Er ging die Lösung dieses Problems unnötig kreativ an. Zunächst verifizierte er durch Schreiben eines Ausdrucks (d >> 24), dass die höchstwertigen Bits ungleich Null sind. Es gibt Ansprüche auf diesen Ausdruck, aber es ist interessanter, die rechte Seite des Ausdrucks zu analysieren: ((d >> 24) +1). Der Programmierer verschiebt die hohen acht Bits in das niedrige Byte. Gleichzeitig wird berechnet, dass das höchstwertige Vorzeichenbit in allen anderen Bits dupliziert wird. Das heißt,
Wenn die Variable d 0b11111111'00000000'00000000'00000000 ist, erhalten wir nach der Verschiebung den Wert 0b11111111'11111111'111111'11111111. Durch Hinzufügen von 1 zum Wert 0xFFFFFFFF vom Typ int plant der Programmierer, 0 zu erhalten. Das heißt: -1 + 1 = 0. Mit dem Ausdruck ((d >> 24) +1) prüft er also, ob nicht alle hohen acht Bits gleich 1 sind. Ich verstehe, dass dies ziemlich schwierig ist. Nehmen Sie sich also bitte Zeit und versuchen Sie herauszufinden, wie und was funktioniert :).Nun wollen wir sehen, was mit diesem Code nicht stimmt.Beim Verschieben wird das höchstwertige Vorzeichenbit nicht unbedingt "verschmiert". Der Standard sagt dazu Folgendes: „Der Wert von E1 >> E2 ist E1 nach rechts verschobene E2-Bitpositionen. Wenn E1 einen vorzeichenlosen Typ hat oder wenn E1 einen vorzeichenbehafteten Typ und einen nicht negativen Wert hat, ist der Wert des Ergebnisses der integrale Bestandteil des Quotienten von E1 / 2 ^ E2. Wenn E1 einen vorzeichenbehafteten Typ und einen negativen Wert hat, ist der resultierende Wert implementierungsdefiniert. "Das neueste Angebot ist uns wichtig. Wir haben also das implementierungsdefinierte Verhalten erreicht. Wie dieser Code funktioniert, hängt von der Mikroprozessorarchitektur und der Compiler-Implementierung ab. Nach der Verschiebung können Nullen in den höchstwertigen Bits erscheinen, und dann unterscheidet sich der Ausdruck ((d >> 24) +1) immer von 0, d. H. wird immer wahrer Wert sein.Daher die Schlussfolgerung: Keine Notwendigkeit, weise zu sein. Der Code wird zuverlässiger und verständlicher, wenn Sie beispielsweise Folgendes schreiben: 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)); }
Wahrscheinlich habe ich nicht die ideale Option vorgeschlagen, aber in diesem Code ist kein Verhalten durch die Implementierung definiert, und es wird für den Leser einfacher zu verstehen, was überprüft wird.Sie verdienen eine Tasse Kaffee oder Tee. Ablenken und weitermachen: Wir warten auf einen interessanten Fall von nicht näher bezeichnetem Verhalten.Während des Interviews stelle ich als eine der ersten Fragen an den Bewerber Folgendes: „Was wird die printf- Funktion drucken und warum?“ int i = 5; printf("%d,%d", i++, i++)
Die richtige Antwort: Dies ist ein nicht spezifiziertes Verhalten. Die Prozedur zur Berechnung der tatsächlichen Argumente beim Aufruf der Funktion ist nicht definiert. Gelegentlich zeige ich sogar, dass dieser Code, der mit Hilfe von Visual C ++ kompiliert wurde, "6.5" auf dem Bildschirm anzeigt, was Neulinge mit schwachem Wissen und Geist zu einem völligen Stillstand bringt :).Dies scheint ein weit hergeholtes Problem zu sein. Aber nein, dieser Code kann in ernsthaften Anwendungen gefunden werden, zum Beispiel in Android-Code. 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 Warnung: V681 CWE-758 Der Sprachstandard definiert keine Reihenfolge, in der die Funktionen 'readSigned' bei der Auswertung von Argumenten aufgerufen werden. ComposerClient.cpp 836Diese Codezeile interessiert uns: mHal.setLayerCursorPosition(...., readSigned(), readSigned());
Durch Aufrufen der Funktion readSigned werden zwei Werte gelesen. Es ist jedoch unmöglich vorherzusagen, in welcher Reihenfolge die Werte gelesen werden. Dies ist ein klassischer Fall von nicht spezifiziertem Verhalten.Vorteile der Verwendung eines statischen Code-Analysators
In diesem gesamten Artikel werden die statische Code-Analyse im Allgemeinen und unser PVS-Studio-Tool im Besonderen populär gemacht. Einige Fehler sind jedoch nur perfekt, um die Möglichkeiten der statischen Analyse zu demonstrieren. Sie sind durch Codeüberprüfungen schwer zu identifizieren, und nur ein nicht müdes Programm bemerkt sie leicht. Betrachten Sie einige solcher Fälle. 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-Warnungen:- V766 CWE-462 Ein Element mit demselben Schlüssel "oem_lpass_cfg" wurde bereits hinzugefügt. bootstat.cpp 264
- V766 CWE-462 Ein Element mit demselben Schlüssel "oem_xpu_ns_error" wurde bereits hinzugefügt. bootstat.cpp 265
Verschiedene Werte mit denselben Schlüsseln werden in den sortierten assoziativen Container ( std :: map ) eingefügt. In Bezug auf die Aufzählung allgemeiner Schwächen ist dies der CWE-462 : Doppelter Schlüssel in der assoziativen Liste.Der Programmtext wird gekürzt und Fehler werden mit Kommentaren markiert, sodass der Fehler offensichtlich erscheint. Wenn Sie diesen Code jedoch nur mit Ihren Augen lesen, ist es sehr schwierig, solche Fehler zu finden.Stellen Sie sich einen anderen Code vor, der sehr schwer zu lesen ist, da er vom gleichen Typ und uninteressant ist. 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 Warnung: V525 CWE-682 Der Code enthält die Sammlung ähnlicher Blöcke. Überprüfen Sie die Elemente 'putInt8', 'putUInt8', 'putInt16', 'putUInt16', 'putInt32', 'putUInt32', 'putInt64', 'putUInt64', 'putInt128', 'putInt128' in den Zeilen 620, 623, 626, 629 , 632, 635, 638, 641, 644, 647. android_mtp_MtpDatabase.cpp 620Wenn MTP_TYPE_UINT128 hatte durch Funktion verursacht werden putUInt128 , nicht putInt128 .Und der letzte in diesem Abschnitt ist ein großartiges Beispiel für erfolgloses Kopieren und Einfügen. 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; .... }
Bevor Sie die Warnung und den weiteren Text des Analysators lesen, empfehle ich, den Fehler selbst zu suchen.Damit Sie die Antwort nicht versehentlich sofort lesen, finden Sie hier ein Bild, mit dem Sie die Aufmerksamkeit ablenken können. Wenn Sie interessiert sind, was ein Ei mit der Aufschrift Java bedeutet, dann sind Sie hier .Ich hoffe, Ihnen hat die Suche nach Tippfehlern gefallen. Jetzt ist es an der Zeit, den Analysator zu warnen: V778 CWE-682 Es wurden zwei ähnliche Codefragmente gefunden. Möglicherweise ist dies ein Tippfehler und die Variable 'abort' sollte anstelle von 'continue' verwendet werden. btif_rc.cc 1554Anscheinend wurde der Code mit der Copy-Paste-Methode geschrieben, und eine Person konnte wie immer beim Bearbeiten des kopierten Codefragments nicht aufmerksam sein. Infolgedessen ersetzte er ganz am Ende nicht " Fortsetzen " durch " Abbrechen ".Das heißt,
im zweiten Block sollte geschrieben werden: avrc_rsp.abort.target_pdu = pavrc_cmd->abort.target_pdu;
Diese Situation fällt vollständig unter die Definition des " Last-Line-Effekts ", da am Ende ein Fehler beim Ersetzen von Namen gemacht wurde.Gesichtspalme
Ein sehr lustiger Fehler hängt mit der Konvertierung zwischen Little-Endian- und Big-Endian-Datenformaten zusammen (siehe Bytereihenfolge ). 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 Warnung: V530 CWE-252 Der Rückgabewert der Funktion 'bswap32' muss verwendet werden. ELFAttribute.cpp 84Es gibt keine Beschwerden über die Funktion bswap32 . Aber es wird falsch verwendet: bswap32(subsection_length);
Der Autor schlug vor, die Variable als Referenz an die Funktion zu übergeben und dort zu ändern. Sie müssen jedoch den von der Funktion zurückgegebenen Wert verwenden. Infolgedessen findet keine Datenkonvertierung statt.Der Analysator hat diesen Fehler als CWE-252 : Unchecked Return Value identifiziert . Tatsächlich ist es jedoch angemessener, hier CWE-198 : Verwendung der falschen Bytereihenfolge aufzurufen. Leider kann der Analysator den Fehler aus einer übergeordneten Sicht nicht verstehen. Dies hindert ihn jedoch nicht daran, diesen schwerwiegenden Fehler im Code zu identifizieren.Der richtige Code lautet: subsection_length = bswap32(subsection_length);
Es gibt drei weitere Stellen im Android-Code mit demselben Fehler:- V530 CWE-252 Der Rückgabewert der Funktion 'bswap32' muss verwendet werden. ELFAttribute.cpp 218
- V530 CWE-252 Der Rückgabewert der Funktion 'bswap32' muss verwendet werden. ELFAttribute.cpp 346
- V530 CWE-252 Der Rückgabewert der Funktion 'bswap32' muss verwendet werden. ELFAttribute.cpp 352
Um solche Fehler zu vermeiden, wird empfohlen, das Attribut [[nodiscard]] zu verwenden . Dieses Attribut gibt an, dass der Rückgabewert einer Funktion beim Aufrufen verwendet werden muss. Wenn Sie also so schreiben: [[nodiscard]] inline uint32_t bswap32(uint32_t pData) { ... }
dann würde der Fehler bereits beim Kompilieren der Datei erkannt. Weitere Informationen zu einigen neuen nützlichen Attributen finden Sie im Artikel " C ++ 17 " meines Kollegen .Nicht erreichbarer Code
In der Programmier- und Compilertheorie ist nicht erreichbarer Code der Teil des Programmcodes, der unter keinen Umständen ausgeführt werden kann, da er im Kontrollflussdiagramm nicht erreichbar ist.Aus Sicht der Common Weakness Enumeration handelt es sich um CWE-561 : Dead Code. virtual sp<IEffect> createEffect(....) { .... if (pDesc == NULL) { return effect; if (status != NULL) { *status = BAD_VALUE; } } .... }
PVS-Studio Warnung: V779 CWE-561 Nicht erreichbarer Code erkannt. Möglicherweise liegt ein Fehler vor. IAudioFlinger.cpp 733Die return-Anweisung muss sich explizit unten befinden.Andere Fehler dieses Typs:- V779 CWE-561 Nicht erreichbarer Code erkannt. Möglicherweise liegt ein Fehler vor. bta_hf_client_main.cc 612
- V779 CWE-561 Nicht erreichbarer Code erkannt. Möglicherweise liegt ein Fehler vor. android_media_ImageReader.cpp 468
- V779 CWE-561 Nicht erreichbarer Code erkannt. Möglicherweise liegt ein Fehler vor. AMPEG4AudioAssembler.cpp 187
Pause
Vergessene Unterbrechung im Schalter ist ein klassischer Fehler von C- und C ++ - Programmierern. Um dem entgegenzuwirken, wurde in C ++ 17 eine nützliche Anmerkung als [[fallthrough]] angezeigt . Sie können mehr über diesen Fehler und [[Fallthrough]] in meinem Artikel " Break and Fallthrough " lesen .Aber während die Welt voll von altem Code ist, in dem Fallthrough nicht verwendet wird, ist PVS-Studio für Sie nützlich. Betrachten Sie einige Fehler in Android gefunden. Gemäß der Common Weakness Enumeration werden diese Fehler als CWE-484 : Ausgelassene Unterbrechungsanweisung in Switch klassifiziert . 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 Warnung: V796 CWE-484 Möglicherweise fehlt die Anweisung 'break' in der Anweisung switch. a2dp_vendor_ldac.cc 912Ich denke, der Fehler bedarf keiner Erklärung. Ich stelle nur fest, dass eine Anomalie im Code häufig auf mehr als eine Weise erkannt wird. Dieser Fehler wird beispielsweise auch durch V519-Warnungen erkannt:- V519 CWE-563 Der Variablen 'codec_capability_.sample_rate' werden zweimal nacheinander Werte zugewiesen. Vielleicht ist das ein Fehler. Überprüfen Sie die Zeilen: 910, 916. a2dp_vendor_ldac.cc 916
- V519 CWE-563 Der Variablen 'codec_config_.sample_rate' werden zweimal hintereinander Werte zugewiesen. Vielleicht ist das ein Fehler. Überprüfen Sie die Zeilen: 911, 917. a2dp_vendor_ldac.cc 917
Und noch ein paar Fehler: Return<void> EffectsFactory::getAllDescriptors(....) { .... switch (status) { case -ENOSYS: {
PVS-Studio Warnung: V796 CWE-484 Möglicherweise fehlt die Anweisung 'break' in der Anweisung 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; .... }
PVS-Studio Warnung: V796 CWE-484 Möglicherweise fehlt die Anweisung 'break' in der Anweisung 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; } .... }
PVS-Studio Warnung: V796 CWE-484 Möglicherweise fehlt die Anweisung 'break' in der Anweisung switch. IAndroidConfiguration.cpp 90Falsche Speicherverwaltung
Hier habe ich Fehler im Zusammenhang mit falscher Speicherverwaltung zusammengestellt. Solche Warnungen werden gemäß der Common Weakness Enumeration wie folgt klassifiziert:- CWE-401 : Unsachgemäße Freigabe des Speichers vor dem Entfernen der letzten Referenz ('Speicherleck')
- CWE-562 : Rückgabe der Stapelvariablenadresse
- CWE-762 : Nicht übereinstimmende Speicherverwaltungsroutinen
Beginnen wir mit Funktionen, die einen Verweis auf eine bereits zerstörte Variable zurückgeben. TransformIterator& operator++(int) { TransformIterator tmp(*this); ++*this; return tmp; } TransformIterator& operator--(int) { TransformIterator tmp(*this); --*this; return tmp; }
PVS-Studio-Warnungen:- V558 CWE-562 Die Funktion gibt den Verweis auf das temporäre lokale Objekt zurück: tmp. transform_iterator.h 77
- V558 CWE-562 Die Funktion gibt den Verweis auf das temporäre lokale Objekt zurück: tmp. transform_iterator.h 92
Wenn die Funktionen ihre Ausführung beendet haben, wird die Variable tmp zerstört, da sie auf dem Stapel erstellt wurde. Folglich geben Funktionen einen Verweis auf ein bereits zerstörtes (nicht vorhandenes) Objekt zurück.Die richtige Lösung wäre, einen Wert von zurückzugeben: TransformIterator operator++(int) { TransformIterator tmp(*this); ++*this; return tmp; } TransformIterator operator--(int) { TransformIterator tmp(*this); --*this; return tmp; }
Betrachten Sie noch traurigeren Code, der besondere Aufmerksamkeit verdient. 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 Warnung: V507 CWE-562 Der Zeiger auf das lokale Array 'buf' wird außerhalb des Bereichs dieses Arrays gespeichert. Ein solcher Zeiger wird ungültig. transport.cpp 1030Dies ist ein gefährlicher Code. Wenn der tatsächliche Wert des seriellen Arguments NULL ist, sollte ein temporärer Puffer auf dem Stapel verwendet werden. Wenn der Hauptteil der if-Anweisung endet, existiert das buf- Array nicht mehr. Der Ort, an dem der Puffer erstellt wurde, kann zum Speichern anderer auf dem Stapel erstellter Variablen verwendet werden. In den Daten beginnt ein höllisches Durcheinander, und die Folgen eines solchen Fehlers werden schlecht vorhergesagt.Die folgenden Fehler beziehen sich auf inkompatible Methoden zum Erstellen und Zerstören von Objekten. 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-Warnung: V611 CWE-762 Der Speicher wurde mit dem Operator 'new T []' zugewiesen, aber mit dem Operator 'delete' freigegeben. Überprüfen Sie diesen Code. Es ist wahrscheinlich besser, 'delete [] mEventCache;' zu verwenden. Überprüfen Sie die Zeilen: 391, 384. SensorEventConnection.cpp 391 Hier istalles einfach. Der Puffer, auf den ein Mitglied der mEventCache- Klasse zeigt, wird mit dem neuen Operator [] zugewiesen . Und geben Sie diesen Speicher mit dem Löschoperator frei . Dies ist falsch und führt zu undefiniertem Programmverhalten.Ähnlicher Fehler: aaudio_result_t AAudioServiceEndpointCapture::open(....) { .... delete mDistributionBuffer; int distributionBufferSizeBytes = getStreamInternal()->getFramesPerBurst() * getStreamInternal()->getBytesPerFrame(); mDistributionBuffer = new uint8_t[distributionBufferSizeBytes]; .... }
PVS-Studio-Warnung: V611 CWE-762 Der Speicher wurde mit dem Operator 'new T []' zugewiesen, aber mit dem Operator 'delete' freigegeben. Überprüfen Sie diesen Code. Es ist wahrscheinlich besser, 'delete [] mDistributionBuffer;' zu verwenden. AAudioServiceEndpointCapture.cpp 50Ich denke, der Fehler bedarf keiner Erklärung.Der folgende Fall ist etwas interessanter, aber das Wesen des Fehlers ist genau das gleiche. struct HeifFrameInfo { .... void set(....) { .... mIccData.reset(new uint8_t[iccSize]); .... } .... std::unique_ptr<uint8_t> mIccData; };
V554 CWE-762 Falsche Verwendung von unique_ptr. Der mit 'new []' zugewiesene Speicher wird mit 'delete' bereinigt.
HeifDecoderAPI.h 62Standardmäßig ruft die Smart-Pointer- Klasse std :: unique_ptr den Löschoperator auf, um ein Objekt zu zerstören . In der Set- Funktion wird der Speicher jedoch mit dem neuen Operator [] zugewiesen .Die richtige Option:
std::unique_ptr<uint8_t[]> mIccData;
Andere Fehler:
- V554 CWE-762 Falsche Verwendung von unique_ptr. Der mit 'new []' zugewiesene Speicher wird mit 'delete' bereinigt. atrace.cpp 949
- V554 CWE-762 Falsche Verwendung von unique_ptr. Der mit 'new []' zugewiesene Speicher wird mit 'delete' bereinigt. atrace.cpp 950
- V554 CWE-762 Falsche Verwendung von unique_ptr. Der mit 'new []' zugewiesene Speicher wird mit 'delete' bereinigt. HeifDecoderImpl.cpp 102
- V554 CWE-762 Falsche Verwendung von unique_ptr. Der mit 'new []' zugewiesene Speicher wird mit 'delete' bereinigt. HeifDecoderImpl.cpp 166
- V554 CWE-762 Falsche Verwendung von unique_ptr. Der mit 'new []' zugewiesene Speicher wird mit 'delete' bereinigt. ColorSpace.cpp 360
Die Speicherverlustfehler vervollständigen diesen Abschnitt. Eine unangenehme Überraschung ist, dass es mehr als 20 solcher Fehler gab. Es scheint mir, dass dies sehr schmerzhafte Fehler sein können, die zu einer allmählichen Verringerung des freien Speichers bei längerem Dauerbetrieb des Betriebssystems führen. 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 Warnung: V773 CWE-401 Die Funktion wurde beendet, ohne den Zeiger 'pAsset' loszulassen. Ein Speicherverlust ist möglich.
Asset.cpp 296Wenn es nicht möglich war, einen bestimmten Block zu öffnen, wird die Funktion beendet, ohne das Objekt zu zerstören, dessen Zeiger in der Variablen pAsset gespeichert ist . Infolgedessen tritt ein Speicherverlust auf.Andere Fehler sind ähnlich, daher sehe ich keinen Grund, sie im Artikel zu berücksichtigen. Interessenten können andere Warnungen in der Datei sehen: Android_V773.txt .Ein Array ins Ausland gehen
Es gibt eine große Anzahl fehlerhafter Muster, die zum Überlauf des Arrays führen. Bei Android habe ich nur ein Fehlermuster des folgenden Typs festgestellt: if (i < 0 || i > MAX) return; A[i] = x;
In C und C ++ sind Array-Zellen von 0 nummeriert, sodass der maximale Index eines Elements im Array um eins kleiner sein muss als die Größe des Arrays. Die richtige Prüfung sollte folgendermaßen aussehen: if (i < 0 || i >= MAX) return; A[i] = x;
Das Überlaufen eines Arrays wird gemäß der Common Weakness Enumeration als CWE-119 : Unsachgemäße Einschränkung von Operationen innerhalb der Grenzen eines Speicherpuffers klassifiziert .Sehen Sie sich an, wie diese Fehler im Android-Code aussehen. 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 Warnung: V557 CWE-119 Array-Überlauf ist möglich. Der Wert des 'idx'-Index könnte 6 erreichen. Btif_hf.cc 277Richtige Prüfoption : if (idx < 0 || idx >= BTA_AG_MAX_NUM_CLIENTS) {
Genau die gleichen Fehler wurden zwei weitere Dinge gefunden:- V557 CWE-119 Array-Überlauf ist möglich. Der Wert des 'idx'-Index könnte 6 erreichen. Btif_hf.cc 869
- V557 CWE-119 Array-Überlauf ist möglich. Der Wert des Index 'index' könnte 6 erreichen. Btif_rc.cc 374
Unterbrochene Zyklen
Es gibt viele Möglichkeiten, einen fehlerhaften Zyklus zu schreiben. Im Android-Code sind Fehler aufgetreten, die gemäß der Common Weakness Enumeration wie folgt klassifiziert werden können:- CWE-20 : Unsachgemäße Eingabevalidierung
- CWE-670 : Immer inkorrekte Implementierung des Kontrollflusses
- CWE-691 : Unzureichendes Kontrollflussmanagement
- CWE-834 : Übermäßige Iteration
Gleichzeitig gibt es natürlich auch andere Möglichkeiten, beim Schreiben von Zyklen „dein eigenes Bein zu schießen“, aber diesmal haben sie mich nicht getroffen. 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 Warnung: V739 CWE-20 EOF sollte nicht mit einem Wert vom Typ 'char' verglichen werden. Das '(c = fgetc (blk_alloc_file))' sollte vom Typ 'int' sein. blk_alloc_to_base_fs.c 61Der Analysator hat festgestellt, dass die EOF- Konstante mit einer Variablen vom Typ char verglichen wird . Mal sehen, warum dieser Code falsch ist.Die Funktion fgetc gibt einen int- Wert zurück , nämlich: Sie kann eine Zahl von 0 bis 255 oder EOF (-1) zurückgeben. Der Lesewert wird in eine Variable vom Typ char eingefügt . Aus diesem Grund wird ein Zeichen mit dem Wert 0xFF (255) zu -1 und wird genauso interpretiert wie das Ende einer Datei (EOF).Aufgrund solcher Fehler stoßen Benutzer, die erweiterte ASCII-Codes verwenden, manchmal auf eine Situation, in der eines der Zeichen in ihrem Alphabet von Programmen falsch verarbeitet wird. Beispielsweise hat der letzte Buchstabe des russischen Alphabets in der Codierung Windows-1251 nur den Code 0xFF und wird von einigen Programmen als das Ende der Datei wahrgenommen.Zusammenfassend können wir sagen, dass die Bedingung zum Stoppen des Zyklus falsch geschrieben ist. Um die Situation zu beheben, muss die Variable c vom Typ int sein.Wir fahren fort und berücksichtigen bekanntere Fehler bei der Verwendung der for- Anweisung . status_t AudioPolicyManager::registerPolicyMixes(....) { .... for (size_t i = 0; i < mixes.size(); i++) { .... for (size_t j = 0; i < mHwModules.size(); j++) {
PVS-Studio Warnung: V534 CWE-691 Es ist wahrscheinlich, dass im Operator 'for' eine falsche Variable verglichen wird. Betrachten Sie die Überprüfung von 'i'. AudioPolicyManager.cpp 2489Aufgrund eines Tippfehlers in einer verschachtelten Schleife verwendet die Bedingung die Variable i , obwohl Sie die Variable j verwenden müssen . Infolgedessen wird die Variable j unkontrolliert inkrementiert, was im Laufe der Zeit dazu führt, dass das mHwModules- Array außerhalb der Grenzen liegt . Was als nächstes passieren wird, kann nicht vorhergesagt werden, da es ein undefiniertes Programmverhalten geben wird.Dieses fehlerhafte Fragment wurde übrigens komplett in eine andere Funktion kopiert. Daher hat der Analysator hier genau den gleichen Fehler gefunden: AudioPolicyManager.cpp 2586.Es gibt 3 weitere Codefragmente, die mir sehr verdächtig sind. Ich gehe jedoch nicht davon aus, dass dieser Code definitiv fehlerhaft ist, da es dort eine komplexe Logik gibt. In jedem Fall muss ich auf diesen Code achten, damit der Autor ihn überprüft.Das erste Fragment: 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 Warnung: V535 CWE-691 Die Variable 'i' wird für diese Schleife und für die äußere Schleife verwendet. Überprüfen Sie die Zeilen: 398, 452. ce_t3t.cc 452Beachten Sie, dass die Variable i sowohl für den externen als auch für den internen Zyklus verwendet wird.Zwei weitere ähnliche Analysator-Trigger:- V535 CWE-691 Die Variable 'xx' wird für diese Schleife und für die äußere Schleife verwendet. Überprüfen Sie die Zeilen: 801, 807. sdp_db.cc 807
- V535 CWE-691 Die Variable 'xx' wird für diese Schleife und für die äußere Schleife verwendet. Überprüfen Sie die Zeilen: 424, 438. nfa_hci_act.cc 438
Bist du noch nicht müde? Ich empfehle, PVS-Studio anzuhalten und herunterzuladen , um zu versuchen, Ihr Projekt zu überprüfen.Jetzt machen wir weiter. #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-Warnung: V654 CWE-834 Die Bedingung 'gate_id <= 0xFF' der Schleife ist immer wahr. nfa_hci_utils.cc 248Beachten Sie Folgendes:- Die Konstante NFA_HCI_LAST_PROP_GATE ist gleich 0xFF.
- Eine Variable vom Typ uint8_t wird als Schleifenzähler verwendet . Daher ist der Wertebereich dieser Variablen [0..0xFF].
Es stellt sich heraus, dass die Bedingung gate_id <= NFA_HCI_LAST_PROP_GATE immer wahr ist und die Ausführung der Schleife nicht stoppen kann.Der Analysator hat diesen Fehler als CWE-834 klassifiziert, kann aber auch als CWE-571 interpretiert werden: Ausdruck ist immer wahr.Der nächste Fehler in der Schleife hängt mit undefiniertem Verhalten zusammen. status_t SimpleDecodingSource::doRead(....) { .... for (int retries = 0; ++retries; ) { .... }
PVS-Studio Warnung: V654 CWE-834 Die Bedingung '++ Wiederholungen' der Schleife ist immer wahr. SimpleDecodingSource.cpp 226Offenbar wollte der Entwickler auf variable Wiederholungen hat alle möglichen Werte für die Variable genommen int und erst dann hat der Zyklus beendet.Die Schleife sollte anhalten, wenn der Ausdruck ++ erneut versucht wird 0. Dies ist nur möglich, wenn die Variable überläuft. Da die Variable vom Typ mit Vorzeichen ist, führt ihr Überlauf zu undefiniertem Verhalten. Daher ist dieser Code falsch und kann zu unvorhersehbaren Konsequenzen führen. Zum Beispiel hat der Compiler das volle Recht, die Prüfung zu entfernen und nur Anweisungen zum Inkrementieren des Zählers zu hinterlassen.Und der letzte Fehler in diesem Abschnitt. 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-Warnung: V696 CWE-670 Der Operator 'continue' beendet die Schleife 'do {...} while (FALSE)', da die Bedingung immer falsch ist. Überprüfen Sie die Zeilen: 105, 121. Vfat.cpp 105Vor uns befindet sich eine Schleife der Form: do { .... if (x) continue; .... } while (0)
Um wiederholte Iterationen durchzuführen, verwendet der Programmierer die continue- Anweisung . Das ist nicht richtig.
Die continue- Anweisung setzt die Schleife nicht sofort fort, sondern überprüft die Bedingung. Da die Bedingung immer falsch ist, wird der Zyklus in jedem Fall nur einmal ausgeführt.Um den Fehler zu beheben, kann der Code beispielsweise wie folgt umgeschrieben werden: for (;;) { .... if (x) continue; .... break; }
Variable Neuzuweisung
Ein sehr häufiger Fehler ist das erneute Schreiben in eine Variable, bevor der vorherige Wert verwendet wurde. Meistens treten solche Fehler aufgrund eines Tippfehlers oder eines erfolglosen Kopierens und Einfügens auf. Gemäß der Common Weakness Enumeration werden solche Fehler als CWE-563 klassifiziert : Zuordnung zu Variablen ohne Verwendung. Nicht ohne solche Fehler in 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 Warnung: V519 CWE-563 Der Variablen 'namespaceExt.prefix.index' werden zweimal nacheinander Werte zugewiesen. Vielleicht ist das ein Fehler. Überprüfen Sie die Zeilen: 1535, 1539. XMLNode.cpp 1539Um das Wesentliche des Fehlers hervorzuheben, schreibe ich einen Pseudocode: if (a > 0) X = 1; else X = 2; X = 1;
Unabhängig von der Bedingung wird der Variablen X (im vorliegenden Code NamespaceExt.prefix.index ) immer ein Wert zugewiesen. bool AudioFlinger::RecordThread::threadLoop() { .... size_t framesToRead = mBufferSize / mFrameSize; framesToRead = min(mRsmpInFramesOA - rear, mRsmpInFramesP2 / 2); .... }
PVS-Studio Warnung: V519 CWE-563 Der Variablen 'frameToRead' werden zweimal nacheinander Werte zugewiesen. Vielleicht ist das ein Fehler. Überprüfen Sie die Zeilen: 6341, 6342. Threads.cpp 6342Es ist nicht klar, warum die Variable während der Deklaration initialisiert werden musste, wenn dann sofort ein anderer Wert in sie geschrieben wird. Hier stimmt etwas nicht. 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 Warnung: V519 CWE-563 Der Variablen 'last_visited_internal_latency_' werden zweimal nacheinander Werte zugewiesen. Vielleicht ist das ein Fehler. Überprüfen Sie die Zeilen: 680, 682. scheduler_arm.cc 682Sehr seltsamer, bedeutungsloser Code. Ich wage vorzuschlagen, dass es hätte geschrieben werden sollen: last_visited_internal_latency_ += kArmMemoryLoadLatency;
Und der letzte Fehler zeigt, wie der Analysator unermüdlich Fehler findet, die selbst bei sorgfältiger Codeüberprüfung am wahrscheinlichsten übersprungen werden. 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 Warnung: V519 CWE-563 Der Variablen 'V' werden zweimal hintereinander Werte zugewiesen. Vielleicht ist das ein Fehler. Überprüfen Sie die Zeilen: 307, 309. p_256_multprecision.cc 309Der Code lautet so "Tränen Sie die Augen aus", dass ich ihn nicht verstehen möchte. Dies ist deutlich sichtbar: Der Code enthält einen Tippfehler, den ich mit Kommentaren hervorgehoben habe.Andere Fehler
Es gibt vereinzelte Fehler, für die es keinen Sinn macht, separate Kapitel zu erstellen. Sie sind jedoch genauso interessant und heimtückisch wie die zuvor betrachteten.Vorrangige Operationen void TagMonitor::parseTagsToMonitor(String8 tagNames) { std::lock_guard<std::mutex> lock(mMonitorMutex);
PVS-Studio Warnung: V593 CWE-783 Überprüfen Sie den Ausdruck der Art 'A = B! = C'. Der Ausdruck wird wie folgt berechnet: 'A = (B! = C)'. TagMonitor.cpp 50Aufzählung allgemeiner Schwächen: CWE-783 : Fehler der Operatorpräzedenzlogik.Der Programmierer hat Folgendes konzipiert. Die Teilzeichenfolge "3a" wird durchsucht und die Position dieser Teilzeichenfolge in die Variable idx geschrieben . Wenn der Teilstring gefunden wird (idx! = -1), wird der Code ausgeführt, der den Wert der Variablen idx verwendet .Leider ist der Programmierer über die Prioritäten der Operationen verwirrt. Tatsächlich funktioniert die Prüfung folgendermaßen: if (ssize_t idx = (tagNames.find("3a") != -1))
Zunächst wird geprüft, ob die Zeichenfolge eine Teilzeichenfolge „3a“ enthält, und das Ergebnis false / true wird in die Variable idx eingefügt . Infolgedessen hat die IDX- Variable den Wert 0 oder 1.Wenn die Bedingung erfüllt ist (die IDX- Variable ist 1), beginnt die Ausführung der Logik, die die IDX- Variable verwendet . Eine Variable, die immer gleich 1 ist, führt zu einem falschen Programmverhalten.Sie können den Fehler beheben, wenn Sie die Initialisierung der Variablen anhand der folgenden Bedingung vornehmen: ssize_t idx = tagNames.find("3a"); if (idx != -1)
Mit der neuen Version von C ++ 17 können Sie außerdem Folgendes schreiben: if (ssize_t idx = tagNames.find("3a"); idx != -1)
Ungültiger Konstruktor struct HearingDevice { .... HearingDevice() { HearingDevice(RawAddress::kEmpty, false); } .... };
PVS-Studio Warnung: V603 CWE-665 Das Objekt wurde erstellt, wird jedoch nicht verwendet. Wenn Sie den Konstruktor aufrufen möchten, sollte 'this-> HearingDevice :: HearingDevice (....)' verwendet werden. Hearing_aid.cc 176Aufzählung allgemeiner Schwächen: CWE-665 : Unsachgemäße Initialisierung.Programmierer irren sich oft, wenn sie versuchen, einen Konstruktor explizit aufzurufen, um ein Objekt zu initialisieren. Es gibt zwei Konstruktoren in der Klasse. Um die Größe des Quellcodes zu verringern, hat der Programmierer beschlossen, einen Konstruktor von einem anderen aufzurufen. Dieser Code entspricht jedoch keineswegs den Erwartungen des Entwicklers.Folgendes passiert. Ein neues unbenanntes Objekt vom Typ HearingDevice wird erstellt und anschließend zerstört. Infolgedessen bleiben die Klassenfelder nicht initialisiert.Um den Fehler zu beheben, können Sie den delegierenden Konstruktor verwenden (diese Funktion wurde in C ++ 11 angezeigt). Der richtige Code lautet: HearingDevice() : HearingDevice(RawAddress::kEmpty, false) { }
Funktion gibt keinen Wert zurück 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 Warnung: V591 CWE-393 Die Funktion "Nicht ungültig" sollte einen Wert zurückgeben. linux_close.cpp 139Aufzählung allgemeiner Schwächen: CWE-393 : Rückgabe des falschen Statuscodes .Die Funktion gibt einen zufälligen Wert zurück. Ein weiterer solcher Fehler: V591 CWE-393 Non-void-Funktion sollte einen Wert zurückgeben. linux_close.cpp 158Falsche Berechnung der Strukturgröße int MtpFfsHandle::handleControlRequest(....) { .... struct mtp_device_status *st = reinterpret_cast<struct mtp_device_status*>(buf.data()); st->wLength = htole16(sizeof(st)); .... }
PVS-Studio Warnung: V568 Es ist seltsam, dass der Operator 'sizeof ()' die Größe eines Zeigers auf eine Klasse auswertet, nicht jedoch die Größe des Klassenobjekts 'st'. MtpFfsHandle.cpp 251Ich bin sicher, dass sie die Größe der Struktur und nicht die Größe des Zeigers in die Mitgliedsvariable wLength einfügen wollten . Dann sollte der richtige Code folgendermaßen aussehen: st->wLength = htole16(sizeof(*st));
Ähnliche Reaktionen des Analysators:- V568 Es ist seltsam, dass der Operator 'sizeof ()' die Größe eines Zeigers auf eine Klasse auswertet, nicht jedoch die Größe des Klassenobjekts 'cacheinfo'. NetlinkEvent.cpp 220
- V568 Es ist seltsam, dass der Operator 'sizeof ()' die Größe eines Zeigers auf eine Klasse auswertet, nicht jedoch die Größe des Klassenobjekts 'page-> next'. linker_block_allocator.cpp 146
- V568 Es ist seltsam, dass das Argument des Operators sizeof () der Ausdruck '& session_id' ist. Referenz-ril.c 1775
Bedeutungslose Bitoperationen #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 Warnung: V617 CWE-480 Überprüfen Sie den Zustand. Das Argument '0x00000001' des '|' Die bitweise Operation enthält einen Wert ungleich Null. egl.cpp 1329Aufzählung der allgemeinen Schwächen: CWE-480 : Verwendung eines falschen Operators.Ein Ausdruck der Form (A | 1) || (A | 2) || (A | 4) macht keinen Sinn, da das Ergebnis immer wahr sein wird. In der Tat müssen Sie den Operator & verwenden , und dann macht der Code Sinn: 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))
Ähnlicher Fehler: V617 CWE-480 Überprüfen Sie den Zustand. Das Argument '0x00000001' des '|' Die bitweise Operation enthält einen Wert ungleich Null. egl.cpp 1338Falsche Bitverschiebung 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 Warnung: V629 CWE-190 Überprüfen Sie den Ausdruck '1 << reg'. Bitverschiebung des 32-Bit-Werts mit anschließender Erweiterung auf den 64-Bit-Typ. RegsInfo.h 47Aufzählung allgemeiner Schwächen: CWE-190 : Integer Overflow oder Wraparound.Bei einer Verschiebung von 1 << reg liegt der Wert der Variablen reg im Bereich [0..63]. Der Ausdruck dient dazu, verschiedene Zweiergrade zu erhalten, beginnend mit 2 ^ 0 und endend mit 2 ^ 63.Der Code funktioniert nicht. Tatsache ist, dass das numerische Literal 1 einen 32-Bit- Int- Typ hat . Daher funktioniert es nicht, einen Wert größer als 1 ^ 31 zu erhalten. Eine Verschiebung zu einem größeren Wert führt zum Überlaufen der Variablen und zum Auftreten eines undefinierten Verhaltens.Der richtige Code lautet: saved_reg_map |= static_cast<uint64_t>(1) << reg;
oder: saved_reg_map |= 1ULL << reg;
Zeichenfolgen werden in sich selbst kopiert. void PCLmGenerator::writeJobTicket() {
PVS-Studio-Warnungen:- V549 CWE-688 Das erste Argument der Funktion 'strcpy' entspricht dem zweiten Argument. genPCLm.cpp 1181
- V549 CWE-688 Das erste Argument der Funktion 'strcpy' entspricht dem zweiten Argument. genPCLm.cpp 1182
Gemäß der Klassifizierung der Common Weakness Enumeration: CWE-688 : Funktionsaufruf mit falscher Variable oder Referenz als Argument.Strings werden aus irgendeinem Grund in sich selbst kopiert. Höchstwahrscheinlich gibt es hier einige Tippfehler.Verwenden einer nicht initialisierten Variablen 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 Warnung: V614 CWE-824 Möglicherweise nicht initialisierter Zeiger 'p_opt' verwendet. Überprüfen Sie das zweite tatsächliche Argument der Funktion 'memcpy'. mca_main.cc 252Aufzählung der allgemeinen Schwächen: CWE-824 : Zugriff auf nicht initialisierten Zeiger.Wenn p_tbl-> tcid! = MCA_CTRL_TCID und p_dcb == nullptr , bleibt der Zeiger p_opt nicht initialisiert.Fremde Verwendung von nicht initialisierten Variablen 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 Warnung: V614 CWE-457 Nicht initialisierte Variable 't.tv_nsec' verwendet. clock_ns.h 55Aufzählung der allgemeinen Schwächen: CWE-457 : Verwendung einer nicht initialisierten Variablen.Zum Zeitpunkt des Dekrementierens der Variablen t.tv_nsec ist sie nicht initialisiert. Die Variable wird später initialisiert: t.tv_nsec = Rest; .
Etwas hier ist eindeutig verwirrt.Über Ausdruck 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 Warnung: V578 Eine ungerade bitweise Operation wurde erkannt. Überprüfen Sie es. bta_dm_co.cc 259Dieser Ausdruck ist redundant. Wenn Sie den Unterausdruck (bte_appl_cfg.ble_auth_req & 0x04) entfernen, ändert sich das Ergebnis des Ausdrucks nicht. Vielleicht gibt es hier einen Tippfehler.Leck behandeln 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 Warnung: V773 CWE-401 Die Funktion wurde beendet, ohne das Handle 'pfin' loszulassen. Ein Ressourcenleck ist möglich.
slang_rs_reflection_cpp.cpp 448Der Analysator klassifizierte diesen Fehler gemäß der Common Weakness Enumeration als: CWE-401 : Unsachgemäße Freigabe des Speichers vor dem Entfernen der letzten Referenz ('Memory Leak'). Hier wäre es jedoch korrekter, CWE-775 auszugeben : Fehlende Veröffentlichung des Dateideskriptors oder des Handles nach der effektiven Lebensdauer. Ich werde meine Kollegen anweisen, diesen Fehler in PVS-Studio zu beheben.Der pfin-Griff wird nirgendwo freigegeben. Ich habe nur vergessen, die fclose- Funktion am Ende aufzurufen . Ein unangenehmer Fehler, der schnell das gesamte Angebot an verfügbaren Deskriptoren erschöpfen kann. Danach ist es unmöglich, neue Dateien zu öffnen.Fazit
Wie Sie sehen können, findet der PVS-Studio-Analysator selbst in einem so bekannten und getesteten Projekt wie Android leicht viele Fehler und potenzielle Schwachstellen. Um zusammenzufassen, welche Schwachstellen (potenzielle Schwachstellen) gefunden wurden:- CWE-14: Compiler-Entfernung von Code zum Löschen von Puffern
- CWE-20: Unsachgemäße Eingabevalidierung
- CWE-119: Unsachgemäße Einschränkung von Operationen innerhalb der Grenzen eines Speicherpuffers
- CWE-190: Integer Overflow oder Wraparound
- CWE-198: Verwendung einer falschen Bytereihenfolge
- CWE-393: Rückgabe des falschen Statuscodes
- CWE-401: Unsachgemäße Freigabe des Speichers vor dem Entfernen der letzten Referenz ('Speicherleck')
- CWE-457: Verwendung einer nicht initialisierten Variablen
- CWE-462: Doppelter Schlüssel in assoziativer Liste
- CWE-480: Verwendung eines falschen Operators
- CWE-484: Unterbrechungsanweisung in Switch weggelassen
- CWE-561: Toter Code
- CWE-562: Rückgabe der Stapelvariablenadresse
- 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
Insgesamt habe ich in dem Artikel 490 potenzielle Schwachstellen beschrieben. Tatsächlich kann der Analysator mehr von ihnen identifizieren, aber wie ich bereits schrieb, fand ich nicht die Kraft, den Bericht genauer zu studieren.Die Größe der getesteten Codebasis beträgt ungefähr 2.168.000 Codezeilen in C und C ++. Davon sind 14,4% Kommentare. Insgesamt erhalten wir ungefähr 1.855.000 Zeilen sauberen Codes.Somit haben wir 490 CWEs für 1.855.000 Codezeilen.Es stellt sich heraus, dass der PVS-Studio-Analysator mehr als eine Schwachstelle (potenzielle Schwachstelle) pro 4000 Codezeilen in einem Android-Projekt erkennen kann. Gutes Ergebnis für einen Code-Analysator, ich bin froh.Vielen Dank für Ihre Aufmerksamkeit!
Ich wünsche allen einen codelosen Code und schlage Folgendes vor:- Laden Sie PVS-Studio herunter und überprüfen Sie den Arbeitsentwurf.
- : : .
- , : twitter , RSS , vk.com .

Wenn Sie diesen Artikel einem englischsprachigen Publikum zugänglich machen möchten, verwenden Sie bitte den Link zur Übersetzung: Andrey Karpov.
We Checked the Android Source Codes by PVS-Studio or Nothing is Perfect