Cod Delphi care merge din întâmplare: cinci bug-uri FPC

PDF Library for Delphi a găsit cinci defecte de decodor în timp ce aducea codul CCITT, TIFF, PNG, Flate și de buffer de stream pe Free Pascal, iar fiecare dintre ele trecuse ani de zile prin suita completă de teste Delphi. Niciunul nu era un bug de compilator. Fiecare era cod Pascal pe care Delphi îl executa corect din întâmplare, din cauza unui detaliu de implementare: un parametru ascuns de rezultat care aliasa tabloul apelantului, o ramură în afara intervalului peste care nu citea nimeni, un buffer de lungime zero a cărui singură pază era un comutator de verificare de interval, un offset bazat pe 1 pe care o singură cale de cod îl pasa ca 1 și un contract TStream.Read pe care stream-urile din memorie nu îl exercită niciodată. Schimbați compilatorul sau dați aceluiași cod un fișier malformat și accidentul nu mai ține

Urmează forma concretă a fiecăruia, reparația și disciplina care a ieșit de aici: același cod sursă trebuie acum să producă aceeași semantică a documentului pe ambele compilatoare, iar un include de test verifică asta. Articolul înrudit despre securizarea unui parser PDF Pascal împotriva fișierelor malițioase a acoperit lățimea întregilor, adâncimea recursiei și bufferele neinițializate. Acesta este despre o altă clasă de defecte: cod care a fost greșit dintotdeauna și a avut un compilator care l-a acoperit în tăcere

De ce merge o funcție care întoarce un tablou dinamic fără SetLength pe Delphi?

Pentru că Delphi trimite variabila apelantului ca parametru ascuns de rezultat, așa că o funcție care nu își alocă niciodată rezultatul poate scrie totuși într-un tablou alocat de apelant. TPLCCITTDecoder.GetNextChangingElement(a0: Integer; IsWhite: Boolean): TCCITTIntegerArray este căutarea în linia de referință din inima decodării bidimensionale Group 3 și Group 4: primește poziția curentă a0 și culoarea rulării curente, caută elementele de schimbare din scanline-ul anterior, b1 și b2 din schema de codare bidimensională ITU-T T.4 și T.6, și le întoarce ca tablou cu două sloturi. Funcția originală scria Result[0] și Result[1] și nu apela deloc SetLength pe Result

Asta ar trebui să dea eroare la prima scriere, iar pe Free Pascal chiar dă. Pe Delphi nu a dat niciodată, pentru că ambele locuri de apel din decodor arată așa: declară b: TCCITTIntegerArray, rulează o dată SetLength(b, 2) înainte de bucla de scanline, apoi în buclă atribuie b := GetNextChangingElement(a0, IsWhite) și citește b[0] și b[1]. Ghidul limbajului Delphi spune că o funcție al cărei rezultat este un șir lung, un tablou dinamic sau alt tip administrat primește acel rezultat ca parametru var suplimentar, iar în practică compilatorul trimite adresa țintei atribuirii. Așa că Result din interiorul funcției este chiar b, deja lung de două elemente, iar fiecare scriere ajunge în memorie deținută de apelant. Free Pascal îi dă funcției un tablou nil nou și îl atribuie apoi lui b, ceea ce este citirea contractului față de care ar fi trebuit scris codul de la bun început

Aliasarea purta și o semantică de care decodorul depinde. Result[0] se atribuie doar când scanarea găsește un element mai mare decât a0, iar Result[1] doar când există un element după el, așa că la o ratare sloturile păstrează ce a lăsat iterația anterioară în b. Reparația evidentă, alocarea a două sloturi și zerorizarea lor la fiecare apel, ar fi distrus acea reportare și ar fi schimbat ieșirea decodată pe Delphi. Reparația livrată este o pază în loc de o resetare: pe Delphi este cod mort și calea de decodare rămâne octet cu octet ce era, iar pe Free Pascal transformă o eroare în comportamentul dorit. Asimetria asta este tot rostul, pentru că reparația trebuia să fie un no-op pe compilatorul unde codul producea deja ieșire verificată

Function TPLCCITTDecoder.GetNextChangingElement(a0: Integer;
  IsWhite: Boolean): TCCITTIntegerArray;
Begin
  // Delphi ajunge aici cu tabloul de două elemente al apelantului aliasat
  // ca Result, deci aici este un no-op. FPC ajunge cu nil.
  If (Length(Result) < 2) Then
    SetLength(Result, 2);
  ...
  // Result[0] / Result[1] se scriu tot doar la o potrivire, deci o ratare
  // păstrează valorile iterației anterioare exact ca înainte
End;

Un contor care a supraviețuit datelor sale: intrarea de director TIFF

Când invalidați un tablou, trebuie să-i invalidați contorul în aceeași instrucțiune, altfel contorul va fi crezut de cod care nu vede niciodată tabloul. O intrare de director de fișier imagine TIFF (TIFF 6.0 §2, aspectul de 12 octeți al câmpurilor tag, tip, contor și valoare-sau-offset) cară un contor pe 32 de biți direct din fișier, iar PDF Library for Delphi le citește pe fiecare prin PopDE: TTIFFEntry, o înregistrare cu Tag, TagType, Length, Offset și tablourile decodate IntegerValues și DoubleValues. Codul original verifica dacă Offset + TypeSize * Length trece dincolo de sfârșitul fișierului, iar dacă trecea, punea ambele tablouri la lungime zero. Lăsa Result.Length la valoarea din fișier

De acolo au mers prost două lucruri. Funcția se termină cu o plasă de siguranță care spune „dacă Length este zero, dă intrării un element cu valoarea zero”, ca apelanții să poată citi mereu elementul zero. Pentru că Length nu era niciodată curățat pe calea în afara intervalului, plasa aceea nu se declanșa niciodată exact pentru cazul pentru care exista. Iar apelanții chiar citesc elementul zero, necondiționat: Width, Height, BitsPerSample, PhotometricInterpretation, FillOrder, SamplesPerPixel, RowsPerStrip și încă vreo doisprezece iau E.IntegerValues[0], iar tabelele de strip fac Move(E.IntegerValues[0], StripOffsets[0], E.Length * 4), copiind de Length ori câte patru octeți dintr-un tablou care nu are niciunul. Un tablou curățat cu un contor viu este strict mai periculos decât unul neverificat, pentru că cel neverificat măcar ține octeții pe care pretinde că îi ține

A doua problemă a fost ordinea. Cele două apeluri SetLength rulau înainte de testul de interval, dimensionate după contorul din fișier, așa că o intrare ostilă putea cere o alocare de mai mulți gigaocteți înainte de orice verificare de validitate. Pe Delphi excepția rezultată era prinsă de un handler mai sus pe calea de încărcare a imaginii și fișierul pur și simplu nu se încărca, de aceea nu a observat nimeni; ce se întâmpla de fapt era un eveniment de memorie epuizată ales de fișier. Reparația mută alocarea după test și face contorul să călătorească împreună cu datele

OutOfRange := Int64(ValueOffset) + Int64(TypeSize) * Result.Length
              > Length(Source);
If OutOfRange Then
Begin
  Result.Length := 0;              // contorul pleacă împreună cu valorile
  SetLength(Result.IntegerValues, 0);
  SetLength(Result.DoubleValues, 0);
End
Else
Begin
  SetLength(Result.IntegerValues, Result.Length);  // abia acum
  SetLength(Result.DoubleValues, Result.Length);
End;
// ... mai târziu, plasa de siguranță existentă ajunge la cazul pentru care exista:
If (Result.Length = 0) Then
Begin
  SetLength(Result.IntegerValues, 1);
  Result.IntegerValues[0] := 0;
End;

Reparația asta nu are nimic specific unui compilator, și exact de asta își are locul în lista de față. Defectul era latent pe Delphi din același motiv din care era latent și pe Free Pascal: niciun fișier de test nu avea o intrare de director care să arate dincolo de sfârșitul fișierului. Portarea nu l-a scos la iveală. Citirea codului cu întrebarea „ce face Delphi aici pentru mine în locul meu” a scos

Ce se întâmplă când un IHDR PNG declară un tip de culoare pe care formatul nu îl definește?

PDF Library for Delphi respinge acum imaginea înainte să ruleze filtrele de rând; înainte de v3.539.2 calcula un scanline de zero octeți și îi dădea buclelor de defiltrare un buffer gol. ISO 15948 §11.2.2 definește chunk-ul IHDR, iar tabelul 11.1 listează cele șase combinații legale de tip de culoare și adâncime de biți: grayscale la 1, 2, 4, 8 sau 16 biți, culoare indexată la 1, 2, 4 sau 8, plus truecolor, grayscale cu alpha și truecolor cu alpha la 8 sau 16. TPNGReader valida câmpurile compression method și filter method din IHDR și lăsa FColorType și adâncimea de biți să treacă neatinse

Codul filtrului de rând dimensionează totul dintr-un Case FColorType Of care mapează fiecare tip de culoare la un număr de componente. Un tip de culoare din afara celor șase cade în ramura Else, unde SourceComponents este 0, deci ScanlineByteCount este 0, deci SetLength(PreviousScanline, 0) este urmat imediat de FillChar(PreviousScanline[0], ScanlineByteCount, 0). Indexarea elementului zero al unui tablou dinamic gol este o adresă calculată din nil. Cu verificarea de interval oprită, o umplere de zero octeți prin acea adresă este un no-op tăcut și decodorul înaintează prin rânduri care nu există; cu verificarea de interval pornită este un ERangeError la prima imagine; iar apelurile Move care urmează sunt la un pas de o încălcare de acces. Care dintre ele se întâmplă depinde de compilator și de comutatoarele de build, nu de ceva ce a decis decodorul, și asta este semnul că decodorul nu a decis nimic

Reparația este tabelul din specificație, aplicat acolo unde celelalte câmpuri IHDR erau deja verificate: COLOR_GRAYSCALE acceptă FSourceBitDepth in [1, 2, 4, 8, 16], COLOR_PALETTE acceptă [1, 2, 4, 8], iar COLOR_RGB, COLOR_GRAYSCALEALPHA și COLOR_RGBALPHA acceptă [8, 16]; orice altceva curăță ValidImage și imaginea este refuzată cu lățimea și înălțimea intacte pentru diagnostic. Un chunk pHYs mai scurt decât cei nouă octeți ai săi a fost închis în aceeași trecere, pentru că cititorul de DPI indexa S[1] până la S[8] dintr-un șir pe care chunk-ul scurt îl lăsase gol

Un offset bazat pe 1 tratat ca pointer bazat pe 0

InflateStrFromPosition(Const Input: AnsiString; StartPos, MaxOutput: Integer; Out Consumed: Integer): AnsiString primește un StartPos bazat pe 1, pentru că intrarea sa este un AnsiString, iar implementarea Delphi adresează intrarea zlib ca @Input[StartPos]. Implementarea Free Pascal, scrisă peste paszlib ca ambele ținte Windows să linkeze compresia static, punea next_in la PAnsiChar(Input) + StartPos și avail_in la Length(Input) - StartPos. Asta este aritmetică de pointeri, și este bazată pe 0. Trimiteți 1, care este ce înseamnă „începe de la capăt” pentru funcția asta, și build-ul FPC începe decompresia de la al doilea octet și se oprește cu un octet înainte de sfârșit

Motivul pentru care a supraviețuit este că singurul apelant la care ajung majoritatea testelor este InflateStr, care trimite 0. Zero se nimerește să fie offsetul corect bazat pe 0, așa că cele două build-uri cădeau de acord la fiecare apel simplu InflateStr și la fiecare test care trecea prin el. TPDFDocument.DecodeAllStreams, rutina pe care SaveQDFToFile și ConvertFileToQDF o folosesc ca să expandeze stream-uri cu un singur FlateDecode într-o formă citibilă, trimite 1. Pe build-ul FPC, header-ul zlib sărit făcea inflate-ul să eșueze, dar stream-ul zlib raporta totuși un Consumed diferit de zero pentru octeții pe care îi examinase, așa că DecodeAllStreams lua payload-ul gol drept decodare reușită și înlocuia fiecare stream de conținut cu un șir gol. QDF-ul rezultat avea numărul corect de pagini, structură validă și niciun conținut de pagină, adică un fișier care se deschide fără eroare în orice vizualizator și nu arată nimic

// Ramura FPC a lui InflateStrFromPosition, după v3.539.16.
// StartPos este bazat pe 1 ca în ramura Delphi; limitați-l, apoi convertiți
// la un offset de pointer bazat pe 0 exact o dată, la graniță.
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;

Testul de regresie care îl păzește este cel mai mic posibil: comprimați un payload, decomprimați-l din poziția 0 și din poziția 1, și verificați că ambele întorc același payload și ambele raportează Consumed egal cu lungimea completă a stream-ului. Un stream RFC 1950 are un header de doi octeți și un trailer Adler-32 de patru octeți, așa că o eroare de unul la oricare capăt nu este o corupere subtilă, este un stream care fie nu pornește, fie nu se termină. Lecția este despre graniță, nu despre zlib: când parametrul unei funcții este definit într-o bază de indexare, iar implementarea de dedesubt o folosește pe cealaltă, conversia își are locul într-o singură linie, iar un test trebuie să o apeleze cu valoarea care distinge cele două baze

De ce nu înseamnă o citire scurtă din TStream.Read sfârșitul stream-ului?

Pentru că TStream.Read are voie să întoarcă mai puțini octeți decât s-au cerut, din orice motiv vrea, și doar întoarcerea lui 0 înseamnă că nu mai este nimic. TMemoryStream și TFileStream pe un disc local umplu aproape întotdeauna cererea, de aceea codul care tratează „a întors mai puțin decât am cerut” drept sfârșit de fișier trece toate testele care le folosesc. Stream-uri pe rețea, stream-uri de decomprimare și orice descendent TStream scris de un client pot întoarce doi octeți când li se cer șaizeci și patru de mii și să aibă totuși gigaocteți în spate

TPLBuffer este cititorul prin care trece fiecare parser din PDF Library for Delphi și poate împacheta un AnsiString, un pointer, un tablou de octeți sau un TStream. Cele patru interogări de scanare ale sale, DistanceToByte, DistanceToOtherByte, DistanceToAnyByte și DistanceToOtherBytes, toate întorcând Int64, citesc sursa în blocuri de 64 KB căutând un delimitator și raportează cât de departe este fără să miște poziția logică. Fiecare buclă se termina cu Until ReadCount < BlockSize. Pentru cele trei surse din memorie asta este corect, pentru că ReadIntoBuffer livrează mereu blocul complet până la ultimul. Pentru sursa de tip stream înseamnă că scanarea renunță la prima citire scurtă, raportează delimitatorul ca absent, iar tokenizer-ul de deasupra decide că obiectul se termină unde nu se termină

// TPLBuffer.DistanceToByte, bucla după v3.539.6.
// Zero este singurul semnal de sfârșit de date pe care îl definește TStream.Read.
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;   // o citire de probă nu trebuie să miște cititorul
End;

Testul care fixează asta este un descendent de TMemoryStream a cărui suprascriere Read limitează fiecare cerere la doi octeți. Împachetați șirul aaaaaX în el, puneți poziția buffer-ului la 1, și toate cele patru interogări trebuie să raporteze o distanță de 4 până la X, să lase poziția la 1 după aceea și să raporteze -1 pentru un octet care nu este acolo. Înainte de reparație prima interogare vedea doi octeți, concluziona că stream-ul s-a terminat și întorcea -1. finally contează la fel de mult ca și condiția buclei: un Exit din interiorul scanării este calea normală de succes, iar poziția logică trebuie restaurată și pe acea cale, nu doar când bucla rulează până la capăt

Un cod sursă, două compilatoare, un singur set de aserțiuni

Disciplina care a ieșit din aceste cinci este că „build-ul Delphi trece” este o dovadă despre Delphi, nu despre codul sursă. Din v3.539.16, suita DUnitX Delphi și suita de consolă Free Pascal includ amândouă același Tests\CrossCompilerSemantics.inc, o singură rutină, RunCrossCompilerFileSemantics, care construiește un document de două pagini cu conținut comprimat prin TPDFlib, îl salvează, îl salvează din nou ca QDF prin SaveQDFToFile, repară QDF-ul cu RepairQDFFile, criptează fișierul simplu cu AES-128 prin EncryptFile și o mască de permisiuni din EncodePermissions, apoi reîncarcă fiecare artefact și verifică aceleași lucruri pe ambele compilatoare: numărul de pagini este 2, titlul supraviețuiește, textul paginii a doua se extrage intact din fișierele simplu, reparat și criptat, parola greșită este refuzată cu un LastErrorCode diferit de zero, EncryptionStrength este 128, EncryptionAlgorithm este 2, iar biții individuali de permisiuni din GetUserPermissions se întorc exact așa cum au fost codificați

Comparația este deliberat normalizată, nu octet cu octet. Criptarea trage salt-uri aleatoare, iar writer-ul atribuie identificatori de document, așa că cele două build-uri nu sunt așteptate să emită fișiere identice; sunt așteptate să emită fișiere care înseamnă același lucru, iar aserțiunile sunt formulate la acel nivel. Segmentul QDF există exact din cauza bug-ului de offset: un QDF cu două pagini și fără conținut trece de o verificare de număr de pagini și cade la o verificare de extragere de text, iar matricea o asertează pe a doua. Orice reparație viitoare care este un no-op pe un compilator și o schimbare de comportament pe celălalt, ceea ce descrie patru din cele cinci de mai sus, trebuie acum să treacă de aceleași aserțiuni de două ori înainte de a fi livrată

Jumătatea de link-editare a aceleiași portări, adică să faci obiectele OMF ale Delphi și așteptările COFF ale Free Pascal să cadă de acord, are propria poveste în link-editarea obiectelor statice FPC Win32 OMF către COFF, iar securizarea structurală a aceluiași cititor TIFF împotriva BigTIFF și a fișierelor cu plăci este în notele despre decodorul TIFF integrat. Decodoarele din acest articol, și testul cross-compilator care stă acum sub ele, sunt livrate în PDF Library for Delphi pentru Delphi, C++Builder și Free Pascal, unde același cod sursă este așteptat să câștige același rezultat pe fiecare compilator țintit, în loc să i se acorde de unul singur