Code Delphi qui ne marche que par accident : cinq bugs FPC

En portant son code CCITT, TIFF, PNG, Flate et de tampon de flux sur Free Pascal, PDF Library for Delphi a mis au jour cinq défauts de décodeur, et tous les cinq passaient la suite de tests Delphi complète depuis des années. Aucun n'était un bug du compilateur. C'était du Pascal que Delphi exécutait correctement par accident, à cause d'un détail d'implémentation : un paramètre Result caché qui aliasait le tableau de l'appelant, une branche hors bornes au-delà de laquelle personne ne lisait jamais, un tampon de longueur nulle dont le seul garde-fou était un commutateur de contrôle de bornes, un décalage 1-based qu'un seul chemin de code passait jamais à 1, et un contrat TStream.Read que les flux en mémoire n'exercent jamais. Changez de compilateur, ou faites lire au même code un fichier malformé, et l'accident cesse de tenir

Ce qui suit décrit la forme exacte de chacun, le correctif, et la discipline qui en est sortie : la même source doit désormais produire les mêmes sémantiques de document sur les deux compilateurs, et un include de test vérifie que c'est bien le cas. L'article voisin sur durcir un analyseur PDF Pascal contre les fichiers malveillants couvrait la largeur des entiers, la profondeur de récursion et les tampons non initialisés. Celui-ci traite d'une autre classe de panne : du code faux depuis toujours, qu'un compilateur couvrait en silence

Pourquoi une fonction qui renvoie un tableau dynamique marche-t-elle sans SetLength sous Delphi ?

Parce que Delphi passe la variable de l'appelant elle-même comme paramètre Result caché, si bien qu'une fonction qui n'alloue jamais son résultat peut quand même écrire dans un tableau alloué par l'appelant. TPLCCITTDecoder.GetNextChangingElement(a0: Integer; IsWhite: Boolean): TCCITTIntegerArray est la recherche de ligne de référence au cœur du décodage bidimensionnel Group 3 et Group 4 : à partir de la position courante a0 et de la couleur du run courant, elle parcourt les éléments changeants de la ligne de balayage précédente, les b1 et b2 du schéma de codage bidimensionnel ITU-T T.4 et T.6, et les renvoie sous forme de tableau à deux cases. La fonction d'origine écrivait Result[0] et Result[1] et n'appelait jamais SetLength sur Result

Cela devrait planter dès la première écriture, et sous Free Pascal c'est le cas. Sous Delphi, jamais : les deux sites d'appel du décodeur ressemblent à ceci — déclarer b: TCCITTIntegerArray, appeler une fois SetLength(b, 2) avant la boucle des lignes, puis dans la boucle affecter b := GetNextChangingElement(a0, IsWhite) et lire b[0] et b[1]. Le guide du langage Delphi précise qu'une fonction dont le résultat est une long string, un tableau dynamique ou un autre type géré reçoit ce résultat comme paramètre var supplémentaire, et en pratique le compilateur passe l'adresse de la cible d'affectation. Result à l'intérieur de la fonction est donc b lui-même, déjà long de deux éléments, et chaque écriture atterrit dans de la mémoire qui appartient à l'appelant. Free Pascal remet à la fonction un tableau nil tout neuf et l'affecte à b ensuite, ce qui est la lecture du contrat contre laquelle le code aurait dû être écrit depuis le début

L'aliasing portait aussi une sémantique dont le décodeur dépend. Result[0] n'est affecté que si le balayage trouve un élément supérieur à a0, et Result[1] que s'il existe un élément après, donc en cas d'échec les cases gardent ce que l'itération précédente avait laissé dans b. Le correctif évident — allouer deux cases et les remettre à zéro à chaque appel — aurait détruit ce report et changé la sortie décodée sous Delphi. Le correctif livré est un garde au lieu d'une remise à zéro : sous Delphi c'est du code mort et le chemin de décodage reste identique octet pour octet, et sous Free Pascal il transforme un plantage en comportement voulu. Cette asymétrie est tout l'intérêt de la chose, puisque le correctif devait être sans effet sur le compilateur où le code produisait déjà une sortie vérifiée

Function TPLCCITTDecoder.GetNextChangingElement(a0: Integer;
  IsWhite: Boolean): TCCITTIntegerArray;
Begin
  // Delphi arrive ici avec le tableau à deux éléments de l'appelant aliasé
  // comme Result, donc ceci ne fait rien ici. FPC arrive avec nil.
  If (Length(Result) < 2) Then
    SetLength(Result, 2);
  ...
  // Result[0] / Result[1] restent écrits seulement sur un hit, donc un miss
  // garde exactement les valeurs de l'itération précédente
End;

Un compteur qui a survécu à ses données : l'entrée de répertoire TIFF

Quand vous invalidez un tableau, vous devez invalider son compteur dans la même instruction, sinon le compteur sera cru par du code qui ne voit jamais le tableau. Une entrée de répertoire d'image TIFF (TIFF 6.0 §2, la disposition de 12 octets avec tag, type, count et valeur-ou-décalage) transporte un compteur 32 bits lu tel quel dans le fichier, et PDF Library for Delphi lit chacune via PopDE: TTIFFEntry, un enregistrement avec Tag, TagType, Length, Offset, et les tableaux décodés IntegerValues et DoubleValues. Le code d'origine vérifiait si Offset + TypeSize * Length dépassait la fin du fichier, et dans ce cas mettait les deux tableaux à longueur nulle. Il laissait Result.Length à la valeur venue du fichier

À partir de là, deux choses ont mal tourné. La fonction se termine par un repli qui dit « si Length vaut zéro, donnez à l'entrée un élément de valeur zéro » pour que les appelants puissent toujours lire l'élément zéro. Comme Length n'était jamais remis à zéro sur le chemin hors bornes, ce repli ne se déclenchait jamais dans le seul cas pour lequel il existait. Or les appelants lisent bien l'élément zéro, sans condition : Width, Height, BitsPerSample, PhotometricInterpretation, FillOrder, SamplesPerPixel, RowsPerStrip et une douzaine d'autres prennent E.IntegerValues[0], et les tables de bandes font Move(E.IntegerValues[0], StripOffsets[0], E.Length * 4), copiant Length fois quatre octets depuis un tableau qui n'en contient aucun. Un tableau vidé dont le compteur est toujours vivant est strictement plus dangereux qu'un tableau non contrôlé, parce que le non contrôlé contient au moins les octets qu'il annonce

Le second problème était l'ordre. Les deux appels à SetLength s'exécutaient avant le test de bornes, dimensionnés d'après le compteur du fichier, donc une entrée hostile pouvait réclamer une allocation de plusieurs gigaoctets avant le moindre contrôle de validité. Sous Delphi, l'exception qui en résultait était attrapée par un gestionnaire plus haut dans le chemin de chargement d'image et le fichier échouait simplement à se charger, ce qui explique que personne ne l'ait vu ; ce qui se passait réellement était un manque de mémoire choisi par le fichier. Le correctif déplace l'allocation après le test et fait voyager le compteur avec les données

OutOfRange := Int64(ValueOffset) + Int64(TypeSize) * Result.Length
              > Length(Source);
If OutOfRange Then
Begin
  Result.Length := 0;              // le compteur part avec les valeurs
  SetLength(Result.IntegerValues, 0);
  SetLength(Result.DoubleValues, 0);
End
Else
Begin
  SetLength(Result.IntegerValues, Result.Length);  // seulement maintenant
  SetLength(Result.DoubleValues, Result.Length);
End;
// ... plus loin, le repli existant atteint enfin le cas pour lequel il était prévu :
If (Result.Length = 0) Then
Begin
  SetLength(Result.IntegerValues, 1);
  Result.IntegerValues[0] := 0;
End;

Rien dans ce correctif n'est propre à un compilateur, et c'est justement ce qui lui vaut sa place dans cette liste. Le défaut était latent sous Delphi pour la même raison qu'il était latent sous Free Pascal : aucun fichier de test n'avait d'entrée de répertoire pointant au-delà de la fin du fichier. Le portage ne l'a pas révélé. C'est la relecture du code avec la question « qu'est-ce que Delphi fait ici pour moi que je ne fais pas moi-même » qui l'a fait

Que se passe-t-il quand un IHDR PNG annonce un type de couleur que le format ne définit pas ?

PDF Library for Delphi rejette désormais l'image avant que les filtres de ligne ne s'exécutent ; avant la v3.539.2, il calculait une ligne de balayage de zéro octet et passait un tampon vide aux boucles de dé-filtrage. ISO 15948 §11.2.2 définit le chunk IHDR et la Table 11.1 énumère les six combinaisons légales de type de couleur et de profondeur de bits : niveaux de gris sur 1, 2, 4, 8 ou 16 bits, couleur indexée sur 1, 2, 4 ou 8, et truecolor, niveaux de gris avec alpha et truecolor avec alpha sur 8 ou 16. TPNGReader validait les champs compression method et filter method de l'IHDR et laissait passer FColorType et la profondeur de bits sans y toucher

Le code des filtres de ligne dimensionne tout à partir d'un Case FColorType Of qui associe chaque type de couleur à un nombre de composantes. Un type de couleur hors des six tombe dans la branche Else, où SourceComponents vaut 0, donc ScanlineByteCount vaut 0, donc SetLength(PreviousScanline, 0) est suivi immédiatement de FillChar(PreviousScanline[0], ScanlineByteCount, 0). Indexer l'élément zéro d'un tableau dynamique vide, c'est une adresse calculée depuis nil. Avec le contrôle de bornes désactivé, un remplissage de zéro octet via cette adresse ne fait rien en silence et le décodeur avance dans des lignes qui n'existent pas ; avec le contrôle de bornes activé, c'est une ERangeError dès la première image ; et les appels Move qui suivent sont à un pas d'une access violation. Ce que vous obtenez dépend du compilateur et des commutateurs de compilation plutôt que d'une décision du décodeur, et c'est bien là le signe que le décodeur n'a jamais rien décidé

Le correctif est la table de la spécification, appliquée là où les autres champs de l'IHDR étaient déjà contrôlés : COLOR_GRAYSCALE accepte FSourceBitDepth in [1, 2, 4, 8, 16], COLOR_PALETTE accepte [1, 2, 4, 8], et COLOR_RGB, COLOR_GRAYSCALEALPHA et COLOR_RGBALPHA acceptent [8, 16] ; tout le reste remet ValidImage à zéro et l'image est refusée, largeur et hauteur intactes pour le diagnostic. Un chunk pHYs plus court que ses neuf octets a été colmaté dans la même passe, le lecteur de DPI indexant S[1] à S[8] d'une chaîne que le chunk trop court avait laissée vide

Un décalage 1-based traité comme un pointeur 0-based

InflateStrFromPosition(Const Input: AnsiString; StartPos, MaxOutput: Integer; Out Consumed: Integer): AnsiString prend un StartPos 1-based, parce que son entrée est une AnsiString et que l'implémentation Delphi adresse l'entrée zlib via @Input[StartPos]. L'implémentation Free Pascal, écrite contre paszlib pour que les deux cibles Windows lient la compression statiquement, mettait next_in à PAnsiChar(Input) + StartPos et avail_in à Length(Input) - StartPos. C'est de l'arithmétique de pointeurs, donc du 0-based. Passez 1 — ce que veut dire « démarrer au début » pour cette fonction — et la compilation FPC commence à décompresser au deuxième octet et s'arrête un octet avant la fin

Si le bug a survécu, c'est que le seul appelant que la plupart des tests atteignent est InflateStr, qui passe 0. Zéro se trouve être le décalage 0-based correct, donc les deux compilations tombaient d'accord sur chaque appel InflateStr ordinaire et sur chaque test qui passait par là. TPDFDocument.DecodeAllStreams, la routine que SaveQDFToFile et ConvertFileToQDF utilisent pour déplier les flux à FlateDecode unique sous forme lisible, passe 1. Sur la compilation FPC, l'en-tête zlib sauté faisait échouer le inflate, mais le flux zlib rapportait quand même un Consumed non nul pour les octets qu'il avait examinés, donc DecodeAllStreams prenait la charge utile vide pour un décodage réussi et remplaçait chaque flux de contenu par une chaîne vide. Le QDF obtenu avait le bon nombre de pages, une structure valide et aucun contenu de page : un fichier qui s'ouvre sans erreur dans toutes les visionneuses et n'affiche rien

// Branche FPC de InflateStrFromPosition, après la v3.539.16.
// StartPos est 1-based comme dans la branche Delphi ; bornez-le, puis convertissez-le
// en décalage de pointeur 0-based une seule fois, à la frontière.
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;

La régression qui le protège est la plus petite possible : compressez une charge utile, décompressez-la depuis la position 0 puis depuis la position 1, et vérifiez que les deux renvoient la même charge utile et rapportent toutes deux un Consumed égal à la longueur totale du flux. Un flux RFC 1950 a un en-tête de deux octets et une queue Adler-32 de quatre octets, donc un décalage d'un octet à l'une ou l'autre extrémité n'est pas une corruption subtile, c'est un flux qui refuse de démarrer ou refuse de finir. La leçon porte sur la frontière, pas sur zlib : quand le paramètre d'une fonction est défini dans une base d'index et que l'implémentation en dessous utilise l'autre, la conversion tient sur exactement une ligne, et un test doit appeler la fonction avec la valeur qui distingue les deux bases

Pourquoi une lecture TStream.Read courte n'est-elle pas la fin du flux ?

Parce que TStream.Read a le droit de renvoyer moins d'octets que demandé, pour la raison qu'il veut, et seule une valeur de retour de 0 signifie qu'il n'y a plus rien. TMemoryStream et TFileStream sur un disque local remplissent presque toujours la demande, et c'est pourquoi du code qui traite « il a renvoyé moins que demandé » comme une fin de fichier passe tous les tests qui les utilisent. Les flux adossés au réseau, les flux de décompression et n'importe quel descendant de TStream écrit par un client peuvent renvoyer deux octets quand on en demande soixante-quatre mille et avoir encore des gigaoctets derrière

TPLBuffer est le lecteur par lequel passe chaque analyseur de PDF Library for Delphi, et il sait envelopper une AnsiString, un pointeur, un tableau d'octets ou un TStream. Ses quatre requêtes de balayage, DistanceToByte, DistanceToOtherByte, DistanceToAnyByte et DistanceToOtherBytes, toutes de retour Int64, lisent la source par blocs de 64 Ko à la recherche d'un délimiteur et indiquent sa distance sans déplacer la position logique. Chaque boucle se terminait par Until ReadCount < BlockSize. Pour les trois sources en mémoire, c'est correct, puisque ReadIntoBuffer livre toujours le bloc entier jusqu'au dernier. Pour la source de type flux, cela signifie que le balayage abandonne dès la première lecture courte, annonce le délimiteur absent, et que le tokenizer au-dessus décide que l'objet se termine là où ce n'est pas le cas

// TPLBuffer.DistanceToByte, la boucle après la v3.539.6.
// Zéro est le seul signal de fin de données que définit 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;   // un peek ne doit pas déplacer le lecteur
End;

Le test qui verrouille tout ça est un descendant de TMemoryStream dont la surcharge de Read plafonne chaque demande à deux octets. Enveloppez-y la chaîne aaaaaX, placez la position du tampon à 1, et les quatre requêtes doivent rapporter une distance de 4 jusqu'au X, laisser la position à 1 ensuite, et renvoyer -1 pour un octet absent. Avant le correctif, la première requête voyait deux octets, en concluait que le flux était épuisé et renvoyait -1. Le finally compte autant que la condition de boucle : un Exit depuis l'intérieur du balayage est le chemin de succès normal, et la position logique doit être restaurée sur ce chemin aussi, pas seulement quand la boucle va jusqu'au bout

Une source, deux compilateurs, un seul jeu d'assertions

La discipline qui est sortie de ces cinq cas, c'est que « la compilation Delphi passe » est une preuve au sujet de Delphi, pas au sujet de la source. Depuis la v3.539.16, la suite DUnitX Delphi et la suite console Free Pascal incluent toutes deux le même Tests\CrossCompilerSemantics.inc, une seule routine, RunCrossCompilerFileSemantics, qui construit un document de deux pages à contenu compressé via TPDFlib, l'enregistre, le réenregistre en QDF via SaveQDFToFile, répare le QDF avec RepairQDFFile, chiffre le fichier en clair en AES-128 via EncryptFile et un masque de permissions issu de EncodePermissions, puis recharge chaque artefact et vérifie les mêmes choses sur les deux compilateurs : le nombre de pages vaut 2, le titre survit, le texte de la page deux s'extrait intact depuis les fichiers en clair, réparé et chiffré, le mauvais mot de passe est refusé avec un LastErrorCode non nul, EncryptionStrength vaut 128, EncryptionAlgorithm vaut 2, et les bits de permission individuels de GetUserPermissions reviennent exactement tels qu'encodés

La comparaison est volontairement normalisée plutôt qu'octet pour octet. Le chiffrement tire des sels aléatoires et l'écrivain attribue des identifiants de document, donc les deux compilations ne sont pas censées produire des fichiers identiques ; elles sont censées produire des fichiers qui veulent dire la même chose, et les assertions sont formulées à ce niveau. Le volet QDF existe précisément à cause du bug de décalage : un QDF avec deux pages et aucun contenu passe un contrôle de nombre de pages et échoue à un contrôle d'extraction de texte, et la matrice vérifie le second. Tout futur correctif sans effet sur un compilateur et qui change le comportement sur l'autre — ce qui décrit quatre des cinq cas ci-dessus — doit désormais franchir deux fois les mêmes assertions avant de partir

La moitié « édition de liens » de ce même portage, faire s'accorder les objets OMF de Delphi et les attentes COFF de Free Pascal, a son propre article dans l'édition de liens d'objets OMF vers COFF en FPC Win32, et le durcissement structurel du même lecteur TIFF face aux fichiers BigTIFF et tuilés se trouve dans les notes sur le décodeur TIFF intégré. Les décodeurs de cet article, et le test inter-compilateurs qui les sous-tend désormais, sont livrés dans PDF Library for Delphi pour Delphi, C++Builder et Free Pascal, où la même source est censée mériter le même résultat sur chaque compilateur visé plutôt que de le recevoir en cadeau de l'un d'eux