Delphi-code die per ongeluk werkt: vijf FPC-portingbugs

PDF Library for Delphi vond vijf decoderdefecten toen de CCITT-, TIFF-, PNG-, Flate- en streambuffercode naar Free Pascal werd overgezet, en elk ervan was al jaren door de volledige Delphi-testsuite gekomen. Geen enkele was een compilerbug. Het was Pascal die Delphi toevallig correct uitvoerde dankzij een implementatiedetail: een verborgen result-parameter die de array van de aanroeper aliast, een tak buiten bereik waar niemand ooit voorbij las, een buffer van lengte nul waarvan de enige bewaking een range-check-schakelaar was, een 1-based offset die slechts één codepad ooit als 1 doorgaf, en een TStream.Read-contract dat in-memory streams nooit raken. Verander de compiler, of voer dezelfde code een beschadigd bestand, en het ongeluk houdt op

Hierna volgt de concrete vorm van elk defect, de fix, en de discipline die eruit voortkwam: dezelfde source moet nu op beide compilers dezelfde documentsemantiek opleveren, en een test-include controleert dat. Het zusterartikel over het verharden van een Pascal PDF-parser tegen kwaadaardige bestanden ging over integerbreedte, recursiediepte en ongeïnitialiseerde buffers. Dit artikel gaat over een andere faalklasse: code die altijd al fout was en waarbij een compiler het stilzwijgend rechtbreide

Waarom werkt een functie die een dynamische array teruggeeft op Delphi zonder SetLength?

Omdat Delphi de variabele van de aanroeper zelf doorgeeft als verborgen result-parameter, kan een functie die zijn result nooit alloceert toch schrijven in een array die de aanroeper heeft gealloceerd. TPLCCITTDecoder.GetNextChangingElement(a0: Integer; IsWhite: Boolean): TCCITTIntegerArray is de referentieregel-lookup in het hart van tweedimensionale Group 3- en Group 4-decodering: op basis van de huidige positie a0 en de kleur van de huidige run doorzoekt hij de changing elements van de vorige scanline, de b1 en b2 van het ITU-T T.4- en T.6-tweedimensionale codeschema, en geeft ze terug als een array met twee slots. De oorspronkelijke functie schreef Result[0] en Result[1] en riep helemaal nooit SetLength aan op Result

Dat hoort bij de eerste write te crashen, en op Free Pascal gebeurt dat ook. Op Delphi gebeurde het nooit, omdat beide aanroepplekken in de decoder er zo uitzien: declareer b: TCCITTIntegerArray, voer één keer SetLength(b, 2) uit vóór de scanline-lus, en wijs binnen de lus b := GetNextChangingElement(a0, IsWhite) toe en lees b[0] en b[1]. De Delphi-taalgids zegt dat een functie waarvan het result een long string, dynamische array of ander managed type is, dat result als extra var-parameter ontvangt, en in de praktijk geeft de compiler het adres van het toewijzingsdoel mee. Result binnen de functie is dus b zelf, al twee elementen lang, en elke write landt in geheugen dat de aanroeper bezit. Free Pascal geeft de functie een verse nil-array en wijst die daarna aan b toe: de lezing van het contract waartegen de code vanaf het begin geschreven had moeten zijn

Die aliasing droeg ook een semantiek die de decoder nodig heeft. Result[0] wordt alleen toegewezen als de scan een element groter dan a0 vindt, en Result[1] alleen als er daarna nog een element is, dus bij een miss houden de slots wat de vorige iteratie in b achterliet. De voor de hand liggende fix, twee slots alloceren en ze bij elke aanroep op nul zetten, zou die carry-over hebben vernietigd en de gedecodeerde output op Delphi hebben veranderd. De fix die is uitgebracht is een guard in plaats van een reset: op Delphi is het dode code en blijft het decodepad byte voor byte wat het was, en op Free Pascal verandert hij een crash in het bedoelde gedrag. Die asymmetrie is precies het punt, want de fix moest een no-op zijn op de compiler waar de code al geverifieerde output produceerde

Function TPLCCITTDecoder.GetNextChangingElement(a0: Integer;
  IsWhite: Boolean): TCCITTIntegerArray;
Begin
  // Delphi komt hier met de array van twee elementen van de aanroeper
  // gealiast als Result, dus dit is daar een no-op. FPC komt hier met nil.
  If (Length(Result) < 2) Then
    SetLength(Result, 2);
  ...
  // Result[0] / Result[1] worden nog steeds alleen bij een hit geschreven,
  // dus een miss houdt de waarden van de vorige iteratie precies zoals eerst
End;

Een count die zijn data overleeft: het TIFF-directory-item

Wanneer je een array ongeldig maakt, moet je in dezelfde statement ook de bijbehorende count ongeldig maken, anders wordt die count geloofd door code die de array nooit ziet. Een TIFF image file directory-item (TIFF 6.0 §2, de lay-out van 12 bytes met tag, type, count en waarde-of-offset) draagt een 32-bits count rechtstreeks uit het bestand, en PDF Library for Delphi leest elk item via PopDE: TTIFFEntry, een record met Tag, TagType, Length, Offset en de gedecodeerde arrays IntegerValues en DoubleValues. De oorspronkelijke code controleerde of Offset + TypeSize * Length voorbij het einde van het bestand liep, en zette dan beide arrays op lengte nul. Result.Length liet hij staan op de waarde uit het bestand

Vanaf daar ging het op twee punten mis. De functie eindigt met een fallback die zegt: "als Length nul is, geef het item één element met waarde nul", zodat aanroepers altijd element nul kunnen lezen. Omdat Length op het buiten-bereik-pad nooit werd gewist, vuurde die fallback nooit voor het enige geval waarvoor hij bestond. En de aanroepers lezen element nul wel degelijk, onvoorwaardelijk: Width, Height, BitsPerSample, PhotometricInterpretation, FillOrder, SamplesPerPixel, RowsPerStrip en nog een dozijn andere nemen E.IntegerValues[0], en de striptabellen doen Move(E.IntegerValues[0], StripOffsets[0], E.Length * 4), waarbij ze Length keer vier bytes kopiëren uit een array die er geen heeft. Een gewiste array met een levende count is strikt genomen gevaarlijker dan een ongecontroleerde, want die laatste bevat tenminste de bytes die hij claimt

Het tweede probleem was de volgorde. De twee SetLength-aanroepen liepen vóór de bereikcontrole, met de count uit het bestand als maat, dus een vijandig item kon een allocatie van meerdere gigabytes aanvragen voordat er ook maar één geldigheidscheck was gedaan. Op Delphi werd de resulterende exception opgevangen door een handler hoger in het image-loadingpad en laadde het bestand simpelweg niet, waardoor niemand het merkte; wat er werkelijk gebeurde was een out-of-memory-event dat het bestand zelf koos. De fix verplaatst de allocatie naar ná de controle en laat de count met de data meereizen

OutOfRange := Int64(ValueOffset) + Int64(TypeSize) * Result.Length
              > Length(Source);
If OutOfRange Then
Begin
  Result.Length := 0;              // de count gaat mee met de waarden
  SetLength(Result.IntegerValues, 0);
  SetLength(Result.DoubleValues, 0);
End
Else
Begin
  SetLength(Result.IntegerValues, Result.Length);  // nu pas
  SetLength(Result.DoubleValues, Result.Length);
End;
// ... verderop bereikt de bestaande fallback eindelijk het geval waarvoor hij bedoeld was:
If (Result.Length = 0) Then
Begin
  SetLength(Result.IntegerValues, 1);
  Result.IntegerValues[0] := 0;
End;

Niets aan deze fix is compilerspecifiek, en juist daarom hoort hij in dit lijstje. Het defect was latent op Delphi om dezelfde reden als op Free Pascal: geen enkel testbestand had een directory-item dat voorbij het einde van het bestand wees. De port heeft het niet blootgelegd. De code lezen met de vraag "wat doet Delphi hier voor mij dat ik zelf niet doe" wel

Wat gebeurt er als een PNG-IHDR een kleurtype claimt dat het formaat niet definieert?

PDF Library for Delphi weigert de afbeelding nu voordat de rowfilters draaien; vóór v3.539.2 berekende hij een scanline van nul bytes en gaf hij de unfilter-lussen een lege buffer. ISO 15948 §11.2.2 definieert de IHDR-chunk en tabel 11.1 somt de zes legale combinaties van kleurtype en bitdiepte op: grijswaarden met 1, 2, 4, 8 of 16 bits, indexed color met 1, 2, 4 of 8, en truecolor, grijswaarden met alpha en truecolor met alpha met 8 of 16. TPNGReader valideerde de velden compression method en filter method van IHDR en liet FColorType en de bitdiepte ongewijzigd door

De rowfiltercode bepaalt alle maten uit een Case FColorType Of die elk kleurtype op een aantal componenten afbeeldt. Een kleurtype buiten de zes valt in de Else-tak, waar SourceComponents 0 is, dus ScanlineByteCount is 0, dus op SetLength(PreviousScanline, 0) volgt direct FillChar(PreviousScanline[0], ScanlineByteCount, 0). Element nul indexeren van een lege dynamische array is een adres dat uit nil is berekend. Met range checking uit is een fill van nul bytes via dat adres een stille no-op en marcheert de decoder verder door rijen die niet bestaan; met range checking aan is het een ERangeError op de eerste afbeelding; en de Move-aanroepen die erop volgen zijn één stap verwijderd van een access violation. Welke van die drie je krijgt hangt af van de compiler en van build switches, niet van iets wat de decoder heeft besloten, en dat is precies het teken dat de decoder helemaal niets besliste

De fix is de tabel uit de specificatie, toegepast op de plek waar de andere IHDR-velden al werden gecontroleerd: COLOR_GRAYSCALE accepteert FSourceBitDepth in [1, 2, 4, 8, 16], COLOR_PALETTE accepteert [1, 2, 4, 8], en COLOR_RGB, COLOR_GRAYSCALEALPHA en COLOR_RGBALPHA accepteren [8, 16]; al het andere wist ValidImage en de afbeelding wordt geweigerd met breedte en hoogte intact voor diagnostiek. Een pHYs-chunk korter dan zijn negen bytes is in dezelfde ronde dichtgezet, omdat de DPI-lezer S[1] tot en met S[8] indexeerde van een string die de korte chunk leeg had gelaten

Een 1-based offset die als 0-based pointer wordt behandeld

InflateStrFromPosition(Const Input: AnsiString; StartPos, MaxOutput: Integer; Out Consumed: Integer): AnsiString neemt een 1-based StartPos, omdat de input een AnsiString is en de Delphi-implementatie de zlib-input adresseert als @Input[StartPos]. De Free Pascal-implementatie, geschreven tegen paszlib zodat beide Windows-targets compressie statisch linken, zette next_in op PAnsiChar(Input) + StartPos en avail_in op Length(Input) - StartPos. Dat is pointerrekenkunde, en die is 0-based. Geef 1 door, wat voor deze functie "begin bij het begin" betekent, en de FPC-build begint met inflaten bij de tweede byte en stopt één byte voor het einde

De reden dat het overleefde is dat de enige aanroeper die de meeste tests raken InflateStr is, en die geeft 0 door. Nul is toevallig de correcte 0-based offset, dus de twee builds waren het eens bij elke gewone InflateStr-aanroep en elke test die daarlangs ging. TPDFDocument.DecodeAllStreams, de routine die SaveQDFToFile en ConvertFileToQDF gebruiken om streams met alleen FlateDecode naar leesbare vorm uit te pakken, geeft 1 door. Op de FPC-build liet de overgeslagen zlib-header het inflaten mislukken, maar de zlib-stream rapporteerde nog steeds een Consumed ongelijk nul voor de bytes die hij had bekeken, dus nam DecodeAllStreams de lege payload aan als een geslaagde decode en verving hij elke contentstream door een lege string. De resulterende QDF had het juiste aantal pagina's, een geldige structuur en geen pagina-inhoud: een bestand dat in elke viewer zonder fout opent en niets toont

// FPC-tak van InflateStrFromPosition, na v3.539.16.
// StartPos is 1-based net als in de Delphi-tak; begrens hem en reken hem
// precies één keer om naar een 0-based pointeroffset, op de grens.
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;

De regressietest die dit bewaakt is de kleinst mogelijke: deflate een payload, inflate hem vanaf positie 0 en vanaf positie 1, en stel vast dat beide dezelfde payload teruggeven en beide Consumed rapporteren gelijk aan de volledige streamlengte. Een RFC 1950-stream heeft een header van twee bytes en een Adler-32-trailer van vier bytes, dus een off-by-one aan een van beide uiteinden is geen subtiele corruptie maar een stream die niet start of niet afmaakt. De les gaat over de grens, niet over zlib: als de parameter van een functie in het ene indexstelsel is gedefinieerd en de implementatie eronder het andere gebruikt, hoort de conversie in precies één regel te staan, en moet een test hem aanroepen met de waarde die de twee stelsels onderscheidt

Waarom is een korte TStream.Read niet het einde van de stream?

Omdat TStream.Read minder bytes mag teruggeven dan gevraagd, om welke reden dan ook, en alleen een return van 0 betekent dat er niets meer is. TMemoryStream en TFileStream op een lokale schijf vullen de aanvraag bijna altijd, en daarom haalt code die "minder teruggekregen dan gevraagd" als end-of-file behandelt elke test die deze klassen gebruikt. Network-backed streams, decompressiestreams en elke TStream-afstammeling die een klant schreef kunnen twee bytes teruggeven bij een vraag om vierenzestigduizend en nog steeds gigabytes achter zich hebben

TPLBuffer is de reader waar elke parser in PDF Library for Delphi doorheen gaat, en hij kan een AnsiString, een pointer, een bytearray of een TStream omwikkelen. Zijn vier scanqueries DistanceToByte, DistanceToOtherByte, DistanceToAnyByte en DistanceToOtherBytes, alle vier met Int64 als resultaat, lezen de bron in blokken van 64 KB op zoek naar een delimiter en melden hoe ver die weg is zonder de logische positie te verplaatsen. Elke lus eindigde met Until ReadCount < BlockSize. Voor de drie in-memory bronnen is dat correct, want ReadIntoBuffer levert altijd het volledige blok tot het laatste. Voor de stroombron betekent het dat de scan opgeeft bij de eerste korte read, de delimiter als afwezig rapporteert, en de tokenizer erboven beslist dat het object eindigt waar dat niet zo is

// TPLBuffer.DistanceToByte, de lus na v3.539.6.
// Nul is het enige einde-van-data-signaal dat TStream.Read definieert.
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;   // een peek mag de reader niet verplaatsen
End;

De test die dit vastpint is een TMemoryStream-afstammeling waarvan de Read-override elke aanvraag op twee bytes begrenst. Wikkel de string aaaaaX erin, zet de bufferpositie op 1, en alle vier de queries moeten een afstand van 4 tot de X rapporteren, de positie daarna op 1 laten staan, en -1 teruggeven voor een byte die er niet is. Vóór de fix zag de eerste query twee bytes, concludeerde dat de stream uitgeput was en gaf -1 terug. De finally is net zo belangrijk als de lusconditie: een Exit van binnenuit de scan is het normale succespad, en op dat pad moet de logische positie ook worden hersteld, niet alleen wanneer de lus tot het einde doorloopt

Eén source, twee compilers, één set asserties

De discipline die hieruit voortkwam is dat "de Delphi-build slaagt" iets zegt over Delphi, niet over de source. Sinds v3.539.16 bevatten zowel de Delphi DUnitX-suite als de Free Pascal-console-suite dezelfde Tests\CrossCompilerSemantics.inc: één routine, RunCrossCompilerFileSemantics, die via TPDFlib een document van twee pagina's met gecomprimeerde inhoud bouwt, het opslaat, het opnieuw als QDF opslaat via SaveQDFToFile, de QDF repareert met RepairQDFFile, het platte bestand met AES-128 versleutelt via EncryptFile en een permissiemasker uit EncodePermissions, en daarna elk artefact opnieuw laadt en op beide compilers hetzelfde vaststelt: het aantal pagina's is 2, de titel overleeft, de tekst van pagina twee komt intact uit het platte, het gerepareerde en het versleutelde bestand, het verkeerde wachtwoord wordt geweigerd met een LastErrorCode ongelijk nul, EncryptionStrength is 128, EncryptionAlgorithm is 2, en de individuele permissiebits uit GetUserPermissions komen precies terug zoals gecodeerd

De vergelijking is bewust genormaliseerd in plaats van byte voor byte. Versleuteling trekt willekeurige salts en de writer kent documentidentifiers toe, dus van de twee builds wordt niet verwacht dat ze identieke bestanden produceren; wel dat ze bestanden produceren die hetzelfde betekenen, en de asserties zijn op dat niveau geformuleerd. Het QDF-gedeelte zit er juist vanwege de offsetbug: een QDF met twee pagina's en geen inhoud slaagt voor een paginanummer-check en zakt voor een tekstextractie-check, en de matrix stelt het tweede vast. Elke toekomstige fix die op de ene compiler een no-op is en op de andere een gedragsverandering, wat voor vier van de vijf hierboven geldt, moet nu dezelfde asserties twee keer halen voordat hij meegaat

De link-time-helft van dezelfde port, het laten overeenkomen van de OMF-objecten van Delphi en de COFF-verwachtingen van Free Pascal, is een verhaal apart in FPC Win32 OMF naar COFF object linking, en de structurele verharding van dezelfde TIFF-lezer tegen BigTIFF en getegelde bestanden staat in de notities over de ingebouwde TIFF-decoder. De decoders uit dit artikel, en de cross-compilertest die er nu onder ligt, zitten in de PDF Library for Delphi voor Delphi, C++Builder en Free Pascal, waar dezelfde source op elke compiler die hij target hetzelfde resultaat hoort te verdienen in plaats van het van één compiler te krijgen