Kategorie: C++

Dann will ich mal das Problem lüften, dass sich mit diesem Code ergibt, dein ich meinem letzten Artikel vorgestellt habe:

template <class T>
void SecureClearString(T &strText)
{
  ::SecureZeroMemory(strText.GetBuffer(0),strText.GetAllocLength());
  strText.Empty();
}

Zuerst einmal liegt es nicht daran, dass es hier Template verwendet wurde.
Ein Template wurde verwendet, weil in dem Code nicht nur CString, sondern implizit CStringA und CStringW verwendet wurde. Der Code sollte also mit beiden Typen funktionieren.

Und damit sind wir bei Problem 1, das auch gelöst wurde:
Wenn ein CStringW verwendet wird, dann wird nur die Hälfte des Strings gelöscht, und nicht alles.

Das Szenario, dass zu einem miesen Crash führen kann, will ich nun in den einzelnen Schritten schildern (es wurde ja vermutet, dass es mit GetBuffer zusammenhängt und die Vermutung ist richtig):

1. Der CString der mit diesem template behandelt wurde enthielt einen größeren CString und anschließend wurde ein kürzerer CString zugewiesen. Damit ist GetAllocLength>GetLength.
2. Dieser CString wird nun an eine weitere Variable zugewiesen. Durch die Referenzzählung wird keine volle Kopie erzeugt.
3. Nun kommt unsere schöne Funktion ins Spiel und einer der beiden Strings wird mit dieser Template Funktion behandelt.
4. Die Funktion hat zwei Argumente, die von rechts nach links berechnet und auf den Stack geschoben werden.
5. D.h. Zuerst wird GetAllocLength ausgeführt. Und dies ergibt einen Wert für die Länge, der ursprünglich einmal in diese Variable passte.
6. Als zweites erfolgt nun der Aufruf von GetBuffer. Da wir aber einen CString haben, der mehrfach benutzt wird, muss nun ein Copy on Write erfolgen. D..h. der String wird kopiert und mit der jetzt benötigten Länge neu alloziert und der Zeiger auf diesen Speicher wird zurückgegeben, dieser ist aber eben kürzer als der ursprüngliche Puffer.
7. Und nun erfolgt der memset, auf einen Speicher der nur noch so groß ist wie der kurze String. Folgerichtig wird der Heap zerstört, weil der Speicher hinter dem String überschrieben wird.
8. Peng ❗ Wir haben hier einen ganz miesen Seiteneffekt.

Hier der Code, mit dem man den Crash gezielt nachbauen kann:

void Crash()
{
  CString str1 = _T("12345678901234567890");
  str1 = _T("123");
  CString str2 = str1;
  SecureClearString(str1); // Crash
  SecureClearString(str2);
}

Der Vollständigkeit halber will ich aber auch noch ein Stück Code zegen, der es richtig macht:

template <class T>
void SecureClearString(T &strText)
{
  // We need this only if there is a private buffer
  if (strText.GetAllocLength()!=0)
  {
    // Execute GetBuffer first. This might cause a fork and may change
    // GetAllocLength.
    T::XCHAR *pBuffer = strText.GetBuffer(0);
    size_t iLen =strText.GetAllocLength();
    ::SecureZeroMemory(pBuffer,iLen*sizeof(T::XCHAR));
  }
  strText.Empty();
}

PS: Der Leser kann sich denken, dass mich dieser Bug und die entsprechende Reproduktion einige Nerven gekostet haben.  Denn es war nicht einfach die Vorbedingung (erst langer String, dann kurzer String, dann Zuweisung) zu ermitteln. Und wie es oft so ist führen Heap-Fehler erst sehr verzögert zu einem Problem.
Wen es genau interessiert: Ich habe ca 7 Stunden an dem Fall geknobelt und hatte 3 verschiedene Crashdumps zur Verfügung. Selbst konnte ich diesen Fehler in unserem Testfeld zuvor nicht erzeugen, weil eben nie alle Bedingungen erfüllt waren. Erst als mir klar war wo das Problem lag, gelang es mir natürlich auch sofort Eingaben zu erzeugen, die den Crash reproduzierten.