Delphi-Code, der nur zufällig läuft: Fünf FPC-Portier-Bugs

Beim Portieren seines CCITT-, TIFF-, PNG-, Flate- und Stream-Buffer-Codes auf Free Pascal fand die PDF Library for Delphi fünf Decoder-Defekte, und jeder einzelne davon hatte die komplette Delphi-Testsuite jahrelang passiert. Keiner war ein Compiler-Bug. Jeder war Pascal, das Delphi zufällig korrekt ausgeführt hat, weil ein Implementierungsdetail mitspielte: ein versteckter Result-Parameter, der ein Alias auf das Array des Aufrufers war, ein Out-of-Range-Zweig, an dessen Grenze nie jemand vorbeigelesen hat, ein Buffer der Länge null, dessen einziger Schutz ein Range-Check-Schalter war, ein 1-basierter Offset, den genau ein Codepfad je als 1 übergeben hat, und ein TStream.Read-Kontrakt, den In-Memory-Streams nie beanspruchen. Wechseln Sie den Compiler oder füttern Sie denselben Code mit einer defekten Datei, und der Zufall hält nicht mehr

Im Folgenden geht es um die konkrete Form jedes einzelnen Defekts, um den Fix und um die Disziplin, die daraus entstanden ist: Derselbe Quelltext muss auf beiden Compilern dieselbe Dokumentsemantik erzeugen, und ein Test-Include prüft, dass das stimmt. Der Schwesterartikel zum Härten eines Pascal-PDF-Parsers gegen bösartige Dateien hat Integer-Breite, Rekursionstiefe und uninitialisierte Buffer abgehandelt. Hier geht es um eine andere Fehlerklasse: Code, der die ganze Zeit über falsch war und bei dem ein Compiler stillschweigend zugedeckt hat

Warum funktioniert eine Funktion mit dynamischem Array-Ergebnis unter Delphi ohne SetLength?

Weil Delphi die eigene Variable des Aufrufers als versteckten Result-Parameter hereingibt, kann eine Funktion, die ihr Ergebnis nie alloziert, trotzdem in ein Array schreiben, das der Aufrufer angelegt hat. TPLCCITTDecoder.GetNextChangingElement(a0: Integer; IsWhite: Boolean): TCCITTIntegerArray ist die Referenzlinien-Suche im Kern der zweidimensionalen Group-3- und Group-4-Dekodierung: Bei gegebener aktueller Position a0 und der Farbe des aktuellen Laufs sucht sie die Changing Elements der vorherigen Scanline, die b1 und b2 des zweidimensionalen Codierungsschemas nach ITU-T T.4 und T.6, und liefert sie als Array mit zwei Slots. Die ursprüngliche Funktion schrieb Result[0] und Result[1] und rief SetLength für Result nie auf

Das müsste beim ersten Schreiben krachen, und unter Free Pascal tut es das auch. Unter Delphi ist es nie passiert, weil beide Aufrufstellen im Decoder so aussehen: b: TCCITTIntegerArray deklarieren, einmal SetLength(b, 2) vor der Scanline-Schleife ausführen und dann in der Schleife b := GetNextChangingElement(a0, IsWhite) zuweisen sowie b[0] und b[1] lesen. Der Delphi-Sprachführer schreibt, dass eine Funktion, deren Ergebnis ein Long String, ein dynamisches Array oder ein anderer managed Type ist, dieses Ergebnis als zusätzlichen var-Parameter erhält, und in der Praxis übergibt der Compiler die Adresse des Zuweisungsziels. Result innerhalb der Funktion ist also b selbst, bereits zwei Elemente lang, und jeder Schreibzugriff landet in Speicher, der dem Aufrufer gehört. Free Pascal reicht der Funktion ein frisches nil-Array und weist es danach b zu – das ist die Lesart des Kontrakts, gegen die der Code von Anfang an geschrieben sein sollte

Das Aliasing trug auch eine Semantik, von der der Decoder abhängt. Result[0] wird nur zugewiesen, wenn der Scan ein Element größer als a0 findet, und Result[1] nur, wenn dahinter noch ein Element kommt, also behalten die Slots bei einem Fehltreffer genau das, was die vorherige Iteration in b hinterlassen hat. Der naheliegende Fix, zwei Slots allozieren und sie bei jedem Aufruf nullen, hätte dieses Übertragen zerstört und die Dekodierausgabe unter Delphi verändert. Der ausgelieferte Fix ist ein Guard statt eines Resets: Unter Delphi ist er toter Code, und der Dekodierpfad bleibt byte für byte, was er war, während er unter Free Pascal aus einem Absturz das beabsichtigte Verhalten macht. Diese Asymmetrie ist der ganze Punkt, denn der Fix musste auf dem Compiler ein No-op sein, auf dem der Code bereits verifizierte Ausgabe produziert

Function TPLCCITTDecoder.GetNextChangingElement(a0: Integer;
  IsWhite: Boolean): TCCITTIntegerArray;
Begin
  // Delphi kommt hier mit dem als Result untergelegten Array des
  // Aufrufers an, daher ist das dort ein No-op. FPC kommt mit nil.
  If (Length(Result) < 2) Then
    SetLength(Result, 2);
  ...
  // Result[0] / Result[1] werden weiterhin nur bei einem Treffer
  // geschrieben, ein Fehltreffer behält die Werte der letzten Iteration
End;

Ein Zähler, der seine Daten überlebte: der TIFF-Directory-Eintrag

Wer ein Array ungültig macht, muss seinen Zähler im selben Statement mit ungültig machen, sonst glaubt jeder Code dem Zähler, der das Array nie zu sehen bekommt. Ein TIFF Image File Directory Entry (TIFF 6.0 §2, das 12-Byte-Layout aus Tag, Type, Count und Value-or-Offset) trägt einen 32-Bit-Zähler direkt aus der Datei, und die PDF Library for Delphi liest jeden über PopDE: TTIFFEntry ein, ein Record mit Tag, TagType, Length, Offset und den dekodierten Arrays IntegerValues und DoubleValues. Der ursprüngliche Code prüfte, ob Offset + TypeSize * Length über das Dateiende hinausläuft, und setzte in dem Fall beide Arrays auf die Länge null. Result.Length ließ er auf dem Wert aus der Datei stehen

Von da an gingen zwei Dinge schief. Die Funktion endet mit einem Fallback, der besagt: Ist Length null, bekommt der Eintrag ein einziges Element mit Wert null, damit Aufrufer immer Element null lesen können. Weil Length auf dem Out-of-Range-Pfad nie gelöscht wurde, sprang dieser Fallback nie für den einen Fall an, für den er existiert. Und die Aufrufer lesen Element null durchaus, bedingungslos: Width, Height, BitsPerSample, PhotometricInterpretation, FillOrder, SamplesPerPixel, RowsPerStrip und ein Dutzend weitere nehmen E.IntegerValues[0], und die Strip-Tabellen machen Move(E.IntegerValues[0], StripOffsets[0], E.Length * 4) und kopieren Length mal vier Bytes aus einem Array, das keines hat. Ein geleertes Array mit lebendigem Zähler ist strikt gefährlicher als ein ungeprüftes, denn das ungeprüfte enthält wenigstens die Bytes, die es behauptet

Das zweite Problem war die Reihenfolge. Die beiden SetLength-Aufrufe liefen vor der Range-Prüfung und dimensionierten nach dem Zähler aus der Datei, sodass ein feindlicher Eintrag eine Allokation von mehreren Gigabyte anfordern konnte, bevor eine einzige Gültigkeitsprüfung stattfand. Unter Delphi fing ein Handler weiter oben im Bildladepfad die resultierende Exception ab, und die Datei lud schlicht nicht – deshalb ist es niemandem aufgefallen; tatsächlich passiert ist ein Out-of-Memory-Ereignis, das sich die Datei ausgesucht hatte. Der Fix verlegt die Allokation hinter die Prüfung und lässt den Zähler mit den Daten reisen

OutOfRange := Int64(ValueOffset) + Int64(TypeSize) * Result.Length
              > Length(Source);
If OutOfRange Then
Begin
  Result.Length := 0;              // der Zähler geht mit den Werten
  SetLength(Result.IntegerValues, 0);
  SetLength(Result.DoubleValues, 0);
End
Else
Begin
  SetLength(Result.IntegerValues, Result.Length);  // erst jetzt
  SetLength(Result.DoubleValues, Result.Length);
End;
// ... später erreicht der bestehende Fallback endlich den Fall, für den er da ist:
If (Result.Length = 0) Then
Begin
  SetLength(Result.IntegerValues, 1);
  Result.IntegerValues[0] := 0;
End;

Nichts an diesem Fix ist compilerspezifisch, und genau deshalb gehört er in diese Liste. Der Defekt war unter Delphi latent aus demselben Grund wie unter Free Pascal: Keine Testdatei hatte einen Directory-Eintrag, der über das Dateiende hinaus zeigte. Der Port hat ihn nicht aufgedeckt. Das tat das Lesen des Codes mit der Frage, was Delphi hier für mich erledigt, das ich nicht selbst erledige

Was passiert, wenn ein PNG-IHDR einen Color Type behauptet, den das Format nicht definiert?

Die PDF Library for Delphi weist das Bild inzwischen ab, bevor die Row-Filter laufen; vor v3.539.2 rechnete sie eine Scanline mit null Bytes aus und reichte den Unfilter-Schleifen einen leeren Buffer. ISO 15948 §11.2.2 definiert den IHDR-Chunk, und Tabelle 11.1 listet die sechs legalen Kombinationen aus Color Type und Bit Depth: Grayscale mit 1, 2, 4, 8 oder 16 Bits, Indexed Color mit 1, 2, 4 oder 8 sowie Truecolor, Grayscale mit Alpha und Truecolor mit Alpha mit 8 oder 16. TPNGReader validierte die Felder für Kompressionsmethode und Filtermethode des IHDR und reichte FColorType und die Bit-Tiefe unverändert durch

Der Row-Filter-Code dimensioniert alles über ein Case FColorType Of, das jeden Color Type auf eine Komponentenzahl abbildet. Ein Color Type außerhalb der sechs fällt in den Else-Zweig, dort ist SourceComponents 0, also ist ScanlineByteCount 0, und auf SetLength(PreviousScanline, 0) folgt unmittelbar FillChar(PreviousScanline[0], ScanlineByteCount, 0). Element null eines leeren dynamischen Arrays zu indizieren ist eine aus nil berechnete Adresse. Bei ausgeschaltetem Range Checking ist eine Null-Byte-Füllung über diese Adresse ein stilles No-op, und der Decoder marschiert durch Reihen, die nicht existieren; bei eingeschaltetem Range Checking ist es ein ERangeError schon beim ersten Bild; und die danach folgenden Move-Aufrufe sind einen Schritt von einer Access Violation entfernt. Was davon Sie bekommen, hängt vom Compiler und von Build-Schaltern ab statt von irgendetwas, das der Decoder entschieden hätte – und genau das verrät, dass der Decoder nie entschieden hat

Der Fix ist die Tabelle aus der Spezifikation, angewendet an der Stelle, an der die übrigen IHDR-Felder schon geprüft wurden: COLOR_GRAYSCALE akzeptiert FSourceBitDepth in [1, 2, 4, 8, 16], COLOR_PALETTE akzeptiert [1, 2, 4, 8], und COLOR_RGB, COLOR_GRAYSCALEALPHA und COLOR_RGBALPHA akzeptieren [8, 16]; alles andere löscht ValidImage, und das Bild wird abgelehnt, während Breite und Höhe für die Diagnose erhalten bleiben. Ein kürzerer als neun Bytes langer pHYs-Chunk wurde im selben Durchgang geschlossen, denn der DPI-Reader indizierte S[1] bis S[8] einer Zeichenkette, die der kurze Chunk leer hinterlassen hatte

Ein 1-basierter Offset, behandelt als 0-basierter Zeiger

InflateStrFromPosition(Const Input: AnsiString; StartPos, MaxOutput: Integer; Out Consumed: Integer): AnsiString nimmt einen 1-basierten StartPos, denn ihre Eingabe ist ein AnsiString, und die Delphi-Implementierung adressiert die zlib-Eingabe als @Input[StartPos]. Die Free-Pascal-Implementierung, geschrieben gegen paszlib, damit beide Windows-Ziele die Kompression statisch linken, setzte next_in auf PAnsiChar(Input) + StartPos und avail_in auf Length(Input) - StartPos. Das ist Zeigerarithmetik, und die ist 0-basiert. Übergeben Sie 1 – das heißt für diese Funktion am Anfang starten –, beginnt der FPC-Build schon beim zweiten Byte zu inflaten und hört ein Byte vor dem Ende auf

Warum das überlebte: Der einzige Aufrufer, den die meisten Tests erreichen, ist InflateStr, und der übergibt 0. Null ist zufällig der korrekte 0-basierte Offset, also waren sich die beiden Builds bei jedem simplen InflateStr-Aufruf und jedem Test, der durch ihn lief, einig. TPDFDocument.DecodeAllStreams, die Routine, mit der SaveQDFToFile und ConvertFileToQDF einzelne FlateDecode-Streams in lesbare Form expandieren, übergibt 1. Im FPC-Build ließ der übersprungene zlib-Header das Inflate fehlschlagen, aber der zlib-Stream meldete für die von ihm betrachteten Bytes weiterhin ein Consumed ungleich null, also wertete DecodeAllStreams die leere Nutzlast als erfolgreiche Dekodierung und ersetzte jeden Content-Stream durch eine leere Zeichenkette. Das resultierende QDF hatte die richtige Seitenzahl, eine gültige Struktur und keinen Seiteninhalt – eine Datei, die in jedem Viewer fehlerfrei aufgeht und nichts zeigt

// FPC-Zweig von InflateStrFromPosition, ab v3.539.16.
// StartPos ist wie im Delphi-Zweig 1-basiert; erst klemmen, dann
// an der Grenze genau einmal in einen 0-basierten Zeiger-Offset wandeln
If (StartPos < 1) Then
  StartPos := 1;
If (Length(Input) = 0) Or (StartPos > Length(Input)) Then
  Exit;
...
strm.next_in  := Pointer(PAnsiChar(Input) + StartPos - 1);
strm.avail_in := Length(Input) - StartPos + 1;

Die Regression, die das festnagelt, ist die kleinstmögliche: eine Nutzlast deflaten, von Position 0 und von Position 1 aus inflaten und prüfen, dass beide dieselbe Nutzlast zurückgeben und beide ein Consumed in Höhe der vollen Streamlänge melden. Ein RFC-1950-Stream hat einen Zwei-Byte-Header und einen Vier-Byte-Adler-32-Trailer, also ist ein Off-by-one an einem der Enden keine subtile Korruption, sondern ein Stream, der entweder nicht anläuft oder nicht fertig wird. Die Lehre handelt von der Grenze, nicht von zlib: Ist der Parameter einer Funktion in einer Indexbasis definiert und die Implementierung darunter nutzt die andere, gehört die Konvertierung in genau eine Zeile, und ein Test muss ihn mit dem Wert aufrufen, der die beiden Basen unterscheidet

Warum ist ein kurzes TStream.Read nicht das Ende des Streams?

Weil TStream.Read nach Belieben weniger Bytes zurückgeben darf, als angefordert wurden, und nur eine Rückgabe von 0 bedeutet, dass nichts mehr kommt. TMemoryStream und TFileStream auf einer lokalen Platte füllen die Anforderung fast immer, weshalb Code, der „weniger zurückgegeben als angefordert“ als Dateiende behandelt, jeden Test besteht, der sie benutzt. Netzwerkbasierte Streams, Dekompressions-Streams und jeder TStream-Nachfahre, den ein Kunde geschrieben hat, können auf eine Anforderung von vierundsechzigtausend zwei Bytes zurückgeben und hinten noch Gigabyte haben

TPLBuffer ist der Reader, durch den jeder Parser in der PDF Library for Delphi läuft, und er kann einen AnsiString, einen Zeiger, ein Byte-Array oder einen TStream umschließen. Seine vier Scan-Queries DistanceToByte, DistanceToOtherByte, DistanceToAnyByte und DistanceToOtherBytes, alle mit Int64 als Ergebnis, lesen die Quelle in 64-KB-Blöcken, suchen ein Trennzeichen und melden, wie weit es entfernt ist, ohne die logische Position zu bewegen. Jede Schleife endete mit Until ReadCount < BlockSize. Für die drei In-Memory-Quellen ist das korrekt, denn ReadIntoBuffer liefert bis auf den letzten immer den vollen Block. Für die Stream-Quelle bedeutet es, dass der Scan beim ersten kurzen Read aufgibt, das Trennzeichen als abwesend meldet und der Tokenizer darüber entscheidet, das Objekt ende dort, wo es nicht endet

// TPLBuffer.DistanceToByte, die Schleife ab v3.539.6.
// Null ist das einzige Datenende-Signal, das TStream.Read definiert.
TempPosition := FPosition;
Try
  Repeat
    ReadCount := ReadIntoBuffer(@TempBuffer[0], BlockSize);
    For TestPos := 0 To ReadCount - 1 Do
      If TempBuffer[TestPos] = Value Then
      Begin
        Result := TotalSkipped + TestPos;
        Exit;
      End;
    Inc(TotalSkipped, ReadCount);
  Until ReadCount = 0;
Finally
  FPosition := TempPosition;   // ein Peek darf den Reader nicht bewegen
End;

Der Test, der das festhält, ist ein TMemoryStream-Nachfahre, dessen Read-Override jede Anforderung auf zwei Bytes kappt. Wickeln Sie die Zeichenkette aaaaaX darin ein, setzen Sie die Buffer-Position auf 1, und alle vier Queries müssen einen Abstand von 4 zum X melden, die Position danach auf 1 lassen und für ein nicht vorhandenes Byte -1 melden. Vor dem Fix sah die erste Query zwei Bytes, schloss auf ein erschöpftes Stream-Ende und gab -1 zurück. Das finally ist genauso wichtig wie die Schleifenbedingung: Ein Exit aus dem Scan heraus ist der normale Erfolgspfad, und auch auf diesem Pfad muss die logische Position wiederhergestellt werden, nicht nur, wenn die Schleife bis zum Ende läuft

Ein Quelltext, zwei Compiler, ein Satz Assertions

Die Disziplin, die aus diesen fünf Fällen entstand: „Der Delphi-Build läuft durch“ ist ein Beweis über Delphi, nicht über den Quelltext. Seit v3.539.16 enthalten sowohl die Delphi-DUnitX-Suite als auch die Free-Pascal-Konsolen-Suite dasselbe Tests\CrossCompilerSemantics.inc, eine einzige Routine RunCrossCompilerFileSemantics, die über TPDFlib ein zweiseitiges Dokument mit komprimiertem Inhalt baut, es speichert, es erneut als QDF über SaveQDFToFile speichert, das QDF mit RepairQDFFile repariert, die Klartextdatei mit AES-128 über EncryptFile und einer Berechtigungsmaske aus EncodePermissions verschlüsselt und dann jedes Artefakt neu lädt und auf beiden Compilern dieselben Dinge prüft: Die Seitenzahl ist 2, der Titel überlebt, der Text von Seite zwei extrahiert intakt aus den Klartext-, reparierten und verschlüsselten Dateien, das falsche Passwort wird mit einem LastErrorCode ungleich null abgewiesen, EncryptionStrength ist 128, EncryptionAlgorithm ist 2, und die einzelnen Berechtigungsbits aus GetUserPermissions kommen exakt so codiert zurück

Der Vergleich ist bewusst normalisiert statt byte für byte. Die Verschlüsselung zieht zufällige Salts, und der Writer vergibt Dokument-Identifikatoren, also wird nicht erwartet, dass die beiden Builds identische Dateien emitieren; erwartet wird, dass sie Dateien mit derselben Bedeutung emittieren, und die Assertions sind auf dieser Ebene formuliert. Der QDF-Ast ist gezielt wegen des Offset-Bugs da: Ein QDF mit zwei Seiten und ohne Inhalt besteht eine Seitenzahl-Prüfung und fällt durch eine Textextraktions-Prüfung, und die Matrix prüft die zweite. Jeder künftige Fix, der auf dem einen Compiler ein No-op ist und auf dem anderen eine Verhaltensänderung – was auf vier der fünf oben genannten zutrifft –, muss jetzt dieselben Assertions zweimal bestehen, bevor er ausgeliefert wird

Die Link-Zeit-Hälfte desselben Ports – Delphis OMF-Objekte und Free Pascals COFF-Erwartungen zur Übereinstimmung zu bringen – ist eine eigene Geschichte unter Objekt-Linking von OMF zu COFF unter FPC Win32, und die strukturelle Härtung desselben TIFF-Readers gegen BigTIFF und getile Dateien steht in den Notizen zum eingebauten TIFF-Decoder. Die Decoder in diesem Artikel und der Cross-Compiler-Test, der jetzt unter ihnen sitzt, kommen in der PDF Library for Delphi für Delphi, C++Builder und Free Pascal daher, wo derselbe Quelltext auf jedem anvisierten Compiler dasselbe Ergebnis verdienen soll, statt es von einem geschenkt zu bekommen