Heap beschädigt nach sprintf_s



  • SeppJ schrieb:

    sOpenURL dürfte interessant sein.

    char	*sOpenURL = NULL;
    sOpenURL = calloc (1024, sizeof(char));
    

    Ja, sorry. Code ist zu minimalistisch gehalten. Aber ich denk, ich hab den Fehler schon gefunden 😃

    Nen Block vorher im Code:

    curl_easy_getinfo(curl, CURLINFO_EFFECTIVE_URL, &sOpenURL);
    

    Der biegt mir natürlich sOpenURL auf nen ganz anderen Speicherbereich. 🙄
    Kein Wunder, dass ich den Heap korrumpiere wenn ich dann dort schreiben will ...

    Richtige Lösung:

    curl_easy_getinfo(curl, CURLINFO_EFFECTIVE_URL, &sTempURL);
    strcpy (sOpenURL, sTempURL);
    

    Gibts ne Möglichkeit hier ohne Hilfszeiger (sTempURL) auszukommen ?


  • Mod

    Pyro Phoenix schrieb:

    Aber: Gibts ne Möglichkeit ohne Hilfszeiger (sTempURL) zu hantieren ?

    Nicht wirklich. Man sollte halt allgemein keine Variablen recyceln. Nenn den Rückgabewert daher nach dem, was er darstellt. Zum Beispiel last_effective_url . Effektiv ist es natürlich immer noch der gleiche Code, aber halt philosophisch schöner. Und natürlich heißt das auch, dass du nicht einmal sOpenURL als Rückgabewert einer Funktion benutzt, dann den Wert mit ganz was anderem überschreibst und dann den neuen Wert an eine ganz andere Funktion übergibst.

    Ein paar Bemerkungen:

    char    *sOpenURL = NULL;
    sOpenURL = calloc (1024, sizeof(char));
    
    • sizeof(char) ist per Definition immer 1
      -Warum sOpenURL zuerst einen falschen Wert zuweisen? Warum nicht direkt
    char *sOpenURL = calloc (1024, 1);
    

    ?
    Selbst wenn der Wert von sOpenURL am Blockbeginn noch nicht feststeht, dann solltest du ihm nicht grundlos einen Wert zuweisen, bevor es seinen eigentlichen Wert bekommt. Vielleicht ist auch dein Block zu groß und du solltest feiner unterteilen.
    -Man könnte natürlich auch char sOpenURL[1024]; schreiben, wenn die 1024 ohnehin eine Konstante ist. Wobei das schon hart an der Grenze für ein Objekt ist, dass ich auf den Stack legen würde. Vielleicht war das ja der Grund für dich, calloc zu nutzen. Aber wo wir schon von der Größe dieses Objekts reden:
    -Warum eigentlich 1024? Es gibt doch keine Maximallänge für URLs. Du kannst dir doch anhand des Rückgabewerts von curl_easy_getinfo und der Länge von sDomainName, sSearchID und dPage recht genau ausrechnen, wie viel Platz du wirklich brauchen wirst. Keine Verschwendung mehr und es ist auch nicht mehr möglich, dass du zu wenig Platz hast.
    -Ich hoffe, du prüfst auch immer brav die Rückgabewerte?



  • SeppJ schrieb:

    Selbst wenn der Wert von sOpenURL am Blockbeginn noch nicht feststeht, dann solltest du ihm nicht grundlos einen Wert zuweisen, bevor es seinen eigentlichen Wert bekommt.

    Was ist schlecht daran ? Ich meine, außer das es CPU Zeit kostet.

    SeppJ schrieb:

    Warum eigentlich 1024? Es gibt doch keine Maximallänge für URLs.

    Das Problem war, dass ich nicht wusste, wie ich am besten die Gesamtlänge der verwendeten URL berechne. Diese besteht ja aus mindestens 3 Einzelstrings. Alle Methoden dazu kamen mir recht aufwändig und zeitintensiv vor. Daher hab ich der Einfachheit halber einen Wert genommen, welcher in der Praxis kaum erreicht wird. Zu dem Zeitpunkt war es mir wichtiger, Erfolge zu erzielen um motiviert zu bleiben - die Schwachstellen könnte man hinterher noch ausbessern.

    (Dynamische allokierung steht auf der ToDo.)

    SeppJ schrieb:

    Ich hoffe, du prüfst auch immer brav die Rückgabewerte?

    Steht auf der ToDo Liste 😃



  • Pyro Phoenix schrieb:

    Was ist schlecht daran ? Ich meine, außer das es CPU Zeit kostet.

    Ein guter Compiler würde den Ausdruck vielleicht sogar ganz entfernen, weil er sieht, dass danach wieder eine unbedingte Zuweisung erfolgt. Da hättest du keine Probleme mit der CPU-Zeit.

    1. Grund: es ist einfach unnötig.
    2. Grund: du hast mehr Code.
    3. Grund: es kann die Fehleranfälligkeit erhöhen, wenn du einen Ausdruck hast, den der Compiler nicht wegoptimieren kann (z.B. ein memset auf 0 vor jedem strxxx-Call - ich reviewe gerade eine Software, wo die Programmier aus weiß Gott welchen Gründen immer memset machen), und da hast du dann gegebenenfalls eine fehlerhafte Länge drin und schreibst in Speicher, in den du gar nicht schreiben willst. Ist dir ja bereits passiert.

    Pyro Phoenix schrieb:

    Das Problem war, dass ich nicht wusste, wie ich am besten die Gesamtlänge der verwendeten URL berechne. Diese besteht ja aus mindestens 3 Einzelstrings. Alle Methoden dazu kamen mir recht aufwändig und zeitintensiv vor. Daher hab ich der Einfachheit halber einen Wert genommen, welcher in der Praxis kaum erreicht wird. Zu dem Zeitpunkt war es mir wichtiger, Erfolge zu erzielen um motiviert zu bleiben - die Schwachstellen könnte man hinterher noch ausbessern.

    Faustregel, nach denen ich programmiere: wenn ich es JETZT nicht fixe, ein dickes, fettes "TODO:" vor die Funktion schreiben, und dann vor jedem Commit frep 'TODO:' -r src/ machen, oder SOFORT fixen.

    Zur Information.



  • Wenn dein Heap beschädigt ist, solltest du ihn erst mal reparieren, bevor du weiter machst. Du kannst nämlich erst mal nicht unterscheiden, ob dein Programm wegen falschem Code abstürzt oder weil halt dein Heap hinüber ist.
    Du kannst deinen Heap fixen, in dem du mit memset deinen Heap mit Nullen über deinen kompletten Adressraum schreibst. Allerdings musst du hier im Kernelmode sein, da du nicht ausschließen kannst, dass durch eine fehlerhafte Bereichsgrenzenüberprüfung vom Betriebssystem, nicht ebenfalls andere Teile vom Heap beschädigt wurden. Treiber sind i. d. R. im Kernelmode.
    Unter Linux schau dir folgenden Link an:

    ftp://tm.informatik.uni-frankfurt.de/pub/papers/ir/kernelbook.pdf

    Falls du Windows benutzt, wird das etwas komplizierter, da Windows einen Hybridkernel besitzt. Du wirst nur den monolithischen Teil des Kernels bereinigen können. Das ist auch der Grund, weshalb Windows allgemein als instabiler gilt, weil wenn erst mal der Heap vom Mikrokernel beschädigt ist, hast du als End-Anwender kaum noch Chancen ihn zu bereinigen. Du brauchst dazu richtig teure Software, die über Sicherheitslücken auch den Hybridkernel erreichen. Diese Lücken, die jedoch immer wieder von MS geschlossen werden, müssen teilweise teuer im Schwarzmarkt erkauft werden. Das ist auch der Grund, weshalb die Software so teuer ist.
    Aber schau dir trotzdem mal vorerst folgenden Link an und wenn du nicht weiterkommst, kannst du dich immer noch melden:
    http://msdn.microsoft.com/en-us/library/windows/hardware/ff553208(v=vs.85).aspx

    Ein ähnliches Problem hast du übrigens auch bei Mac OS X. 😕 Das Vorgehen ist hier analog.

    L. G.,
    IBV



  • @IBV: Wieso musste ich bei deinem Post direkt an die Supercodes von autoexe.bat denken ... :p



  • dachschaden schrieb:

    @IBV: Wieso musste ich bei deinem Post direkt an die Supercodes von autoexe.bat denken ... :p

    Weiss nicht. Sagt mir nichts. 🙂



  • IBV schrieb:

    dachschaden schrieb:

    @IBV: Wieso musste ich bei deinem Post direkt an die Supercodes von autoexe.bat denken ... :p

    Weiss nicht. Sagt mir nichts. 🙂

    http://www.c-plusplus.net/forum/41456

    Betreibst super Satire 👍



  • Hab grad nochmal ein wenig nach einer simplen Lösung für die zusammengesetzten Strings gesucht. Ab C99 gäbe es ja eigentlich folgende Lösung:

    dStringLaenge = snprintf (NULL, 0, "%s, %s", sStringA, sStringB);
    

    Leider Befindet sich Visual Studio noch in der Steinzeit was C anbelangt - kein C99 Support.

    Microsoft bietet aber eine hauseigene Lösung: _snprintf (Identisch zum C99 Pendant - nur ohne garantierten Null Terminator)

    Habe zur Bestimmung der String-Länge und der anschließenden allokierung nun folgenden Code zusammengeschrieben:

    char	sStringA[] = "Hallo Welt";
    char	sStringB[] = "Variabler Text";
    char	*sStringC;
    int		dStringLaenge = 0;
    
    dStringLaenge = _snprintf (NULL, 0, "%s, %s", sStringA, sStringB);
    sStringC = calloc (dStringLaenge + 1, 1);
    sprintf (sStringC, "%s, %s", sStringA, sStringB);
    printf (sStringC);
    free(sStringC);
    

    Funktioniert so wie es soll - Problem dabei: Microsoft only !


  • Mod

    Du koenntest per ifdef und defines eine Loesung fuer beide Plattformen (C99 faehig vs. C89 + _snprintf) machen, wenn dir der Aufwand das Wert ist.



  • Ob da strlen und etwas Zugabe nicht ausricht?



  • Funktioniert so wie es soll - Problem dabei: Microsoft only !

    Dann verwende doch gleich _scprintf bzw. _vscprintf. Außerdem bei VS2015 gibt es snprintf .

    Einige Standard-C-Bibliotheken für Linux oder BSD haben übrigens asprintf, welches ich verwenden würde und für mich portabel genug ist. Wo sie fehlt, nimmt man sich eine bereits existierende Implementierung oder schreibt sie sich selbst.



  • DirkB schrieb:

    Ob da strlen und etwas Zugabe nicht ausricht?

    Theoretisch schon - allerdings ist das bei mehreren Strings recht unhandlich und bläht den Code auf, da ja jeder einzelne String einer separaten Anweisung bedarf. Bei _snprintf geschieht das in einem Rutsch.

    EinGast schrieb:

    Außerdem bei VS2015 gibt es snprintf .

    Ja, weil es seit C++11 Standard ist. Leider wird es in den älteren IDEs (VS2010) nicht nachträglich hinzugefügt.


Anmelden zum Antworten