קוד דלפי שעובד במקרה: חמישה באגי פורט ל-FPC

PDF Library for Delphi מצאה חמישה פגמים במפענחים בזמן שהעבירה את קוד ה-CCITT, TIFF, PNG, Flate ו-stream buffer שלה ל-Free Pascal, וכולם עברו את מערך הבדיקות המלא של דלפי במשך שנים. אף אחד מהם לא היה באג של מהדר. כל אחד מהם היה Pascal שדלפי פשוט הרצה נכון בגלל פרט מימוש: פרמטר Result נסתר שעשה alias למערך של הקורא, ענף מחוץ לטווח שאף אחד לא קרא מעבר לו, מאגר באורך אפס שהשומר היחיד שלו היה מתג של בדיקת טווחים, היסט מבוסס-1 שרק נתיב קוד אחד העביר בתור 1, וחוזה של TStream.Read שזרמים בזיכרון אף פעם לא מפעילים. החליפו מהדר, או האכילו את אותו קוד בקובץ פגום, והמקרה מפסיק להתקיים

בהמשך מפורטת הצורה המדויקת של כל אחד מהם, התיקון, והמשמעת שיצאה מזה: אותו קוד מקור חייב כעת להפיק את אותן סמנטיקות מסמך בשני המהדרים, וקובץ include של בדיקות מוודא שזה אכן קורה. המאמר האח על הקשחת מפענח PDF ב-Pascal מול קבצים זדוניים עסק ברוחב מספרים שלמים, בעומק רקורסיה ובמאגרים לא מאותחלים. המאמר הזה עוסק בסוג כשל אחר: קוד שהיה שגוי מלכתחילה והיה לו מהדר שכיסה עליו בשקט

למה פונקציה שמחזירה מערך דינמי עובדת בלי SetLength בדלפי?

כי דלפי מעבירה את המשתנה של הקורא עצמו כפרמטר ה-Result הנסתר, כך שפונקציה שלעולם לא מקצה את התוצאה שלה יכולה עדיין לכתוב לתוך מערך שהקורא הקצה. TPLCCITTDecoder.GetNextChangingElement(a0: Integer; IsWhite: Boolean): TCCITTIntegerArray הוא חיפוש שורת הייחוס בלב הפענוח הדו-ממדי של Group 3 ו-Group 4: בהינתן המיקום הנוכחי a0 והצבע של הריצה הנוכחית, הוא מחפש את איברי השינוי של שורת הסריקה הקודמת, ה-b1 וה-b2 של סכמת הקידוד הדו-ממדית ITU-T T.4 ו-T.6, ומחזיר אותם כמערך בן שני תאים. הפונקציה המקורית כתבה ל-Result[0] ול-Result[1] ואף פעם לא קראה ל-SetLength על Result בכלל

זה אמור לגרום לתקלה כבר בכתיבה הראשונה, וב-Free Pascal זה אכן קורה. בדלפי זה אף פעם לא קרה, כי שני אתרי הקריאה במפענח נראים כך: הכריזו b: TCCITTIntegerArray, הריצו SetLength(b, 2) פעם אחת לפני לולאת שורות הסריקה, ואז בתוך הלולאה השמה b := GetNextChangingElement(a0, IsWhite) וקריאה של b[0] ו-b[1]. מדריך השפה של דלפי קובע שפונקציה שהתוצאה שלה היא מחרוזת ארוכה, מערך דינמי או טיפוס מנוהל אחר מקבלת את התוצאה כפרמטר var נוסף, ובפועל המהדר מעביר את הכתובת של יעד ההשמה. כך ש-Result בתוך הפונקציה הוא b עצמו, באורך שני איברים ממילא, וכל כתיבה נוחתת בזיכרון שנמצא בבעלות הקורא. Free Pascal מוסרת לפונקציה מערך nil טרי ומשימה אותו ל-b לאחר מכן, וזו הקריאה הנכונה של החוזה שמולו הקוד היה צריך להיכתב מלכתחילה

ה-alias הזה נשא גם סמנטיקה שהמפענח תלוי בה. Result[0] מושם רק כשהסריקה מוצאת איבר גדול מ-a0, ו-Result[1] רק כשיש איבר אחריו, כך שבהחמצה התאים שומרים על מה שהאיטרציה הקודמת השאירה ב-b. התיקון המובן מאליו — להקצות שני תאים ולאפס אותם בכל קריאה — היה הורס את ההמשכיות הזו ומשנה את הפלט המפוענח בדלפי. התיקון שנשלח הוא שומר במקום איפוס: בדלפי זה קוד מת ונתיב הפענוח נשאר בדיוק כפי שהיה, וב-Free Pascal הוא הופך תקלה להתנהגות המיועדת. הא-סימטריה הזו היא כל העניין, כי התיקון היה חייב להיות no-op במהדר שבו הקוד כבר הפיק פלט מאומת

Function TPLCCITTDecoder.GetNextChangingElement(a0: Integer;
  IsWhite: Boolean): TCCITTIntegerArray;
Begin
  // דלפי מגיעה לכאן כשהמערך בן שני האיברים של הקורא עשה alias
  // ל-Result, כך שזה no-op שם. FPC מגיעה עם nil.
  If (Length(Result) < 2) Then
    SetLength(Result, 2);
  ...
  // Result[0] / Result[1] נכתבים עדיין רק בהצלחה, כך שהחמצה
  // משמרת את הערכים מהאיטרציה הקודמת בדיוק כמו קודם
End;

מונה שהאריך ימים אחרי הנתונים שלו: רשומת ספריית TIFF

כשאתם מבטלים תוקף של מערך, עליכם לבטל את תוקף המונה שלו באותה הצהרה, אחרת קוד שלעולם לא רואה את המערך יאמין למונה. רשומת ספרייה של קובץ תמונה TIFF (‏TIFF 6.0 §2, הפריסה בת 12 הבתים של tag, type, count ו-value-or-offset) נושאת מונה של 32 סיביות היישר מהקובץ, ו-PDF Library for Delphi קוראת כל אחד מהם דרך PopDE: TTIFFEntry, רשומה עם Tag, TagType, Length, Offset, והמערכים המפוענחים IntegerValues ו-DoubleValues. הקוד המקורי בדק אם Offset + TypeSize * Length חורג מסוף הקובץ, ואם כן, הוא הגדיר את שני המערכים באורך אפס. הוא השאיר את Result.Length בערך מהקובץ

משם שני דברים השתבשו. הפונקציה מסתיימת ב-fallback שאומר "אם Length הוא אפס, תנו לרשומה איבר אחד שערכו אפס" כדי שקוראים תמיד יוכלו לקרוא את איבר אפס. מכיוון ש-Length מעולם לא אופס בנתיב שמחוץ לטווח, ה-fallback הזה אף פעם לא נורה במקרה היחיד שלשמו נוצר. והקוראים אכן קוראים את איבר אפס, בלי תנאי: Width, Height, BitsPerSample, PhotometricInterpretation, FillOrder, SamplesPerPixel, RowsPerStrip, ועוד תריסר לוקחים E.IntegerValues[0], וטבלאות ה-strip עושות Move(E.IntegerValues[0], StripOffsets[0], E.Length * 4), ומעתיקות Length כפול ארבעה בתים מתוך מערך שאין בו כלום. מערך מאופס עם מונה חי הוא מסוכן בהחלט יותר ממערך בלי בדיקה, כי זה שבלי בדיקה לפחות מחזיק את הבתים שהוא מצהיר עליהם

הבעיה השנייה הייתה סדר הפעולות. שתי הקריאות ל-SetLength רצו לפני בדיקת הטווח, בגודל שנגזר מהמונה של הקובץ, כך שרשומה עוינת יכלה לבקש הקצאה של כמה גיגהבתים עוד לפני בדיקת תקינות אחת. בדלפי החריגה שנוצרה נתפסה ב-handler שנמצא גבוה יותר בנתיב טעינת התמונה והקובץ פשוט לא נטען, וזו הסיבה שלא שם לב לכך אף אחד; מה שקרה בפועל היה אירוע out-of-memory שהקובץ בחר. התיקון מעביר את ההקצאה לאחר הבדיקה וגורם למונה לנסוע יחד עם הנתונים

OutOfRange := Int64(ValueOffset) + Int64(TypeSize) * Result.Length
              > Length(Source);
If OutOfRange Then
Begin
  Result.Length := 0;              // המונה הולך יחד עם הערכים
  SetLength(Result.IntegerValues, 0);
  SetLength(Result.DoubleValues, 0);
End
Else
Begin
  SetLength(Result.IntegerValues, Result.Length);  // רק עכשיו
  SetLength(Result.DoubleValues, Result.Length);
End;
// ... מאוחר יותר, ה-fallback הקיים מגיע סוף סוף למקרה שלשמו נוצר:
If (Result.Length = 0) Then
Begin
  SetLength(Result.IntegerValues, 1);
  Result.IntegerValues[0] := 0;
End;

אין בתיקון הזה שום דבר ספציפי למהדר, וזו בדיוק הסיבה שהוא שייך לרשימה הזו. הפגם היה רדום בדלפי מאותה סיבה שהוא היה רדום ב-Free Pascal: לאף קובץ בדיקה לא הייתה רשומת ספרייה שמצביעה מעבר לסוף הקובץ. הפורט לא חשף אותו. מה שחשף אותו הוא קריאה של הקוד עם השאלה "מה דלפי עושה כאן בשבילי ואני לא עושה בעצמי"

מה קורה כשה-IHDR של PNG מצהיר על סוג צבע שהפורמט לא מגדיר?

PDF Library for Delphi דוחה כעת את התמונה לפני שפילטרי השורות רצים; לפני v3.539.2 היא חישבה שורת סריקה באורך אפס והעבירה ללולאות ה-unfilter מאגר ריק. ISO 15948 §11.2.2 מגדיר את מקטע ה-IHDR, וטבלה 11.1 מפרטת את ששת השילובים החוקיים של סוג צבע ועומק סיביות: גווני אפור ב-1, 2, 4, 8 או 16 סיביות, צבע מאונדקס ב-1, 2, 4 או 8, ו-truecolor, גווני אפור עם alpha ו-truecolor עם alpha ב-8 או 16. TPNGReader וידאה את שדות שיטת הדחיסה ושיטת הסינון של IHDR והעבירה את FColorType ואת עומק הסיביות כמו שהם

קוד פילטר השורות גוזר את כל הגדלים מ-Case FColorType Of שממפה כל סוג צבע למספר רכיבים. סוג צבע מחוץ לששת אלו נופל לענף ה-Else, שבו SourceComponents הוא 0, כך ש-ScanlineByteCount הוא 0, כך ש-SetLength(PreviousScanline, 0) ואחריו מיד FillChar(PreviousScanline[0], ScanlineByteCount, 0). אינדוקס של איבר אפס במערך דינמי ריק הוא כתובת שחושבה מ-nil. כשבדיקת הטווחים כבויה, מילוי של אפס בתים דרך הכתובת הזו הוא no-op שקט והמפענח ממשיך לצעוד דרך שורות שלא קיימות; כשהיא דלוקה זה ERangeError כבר בתמונה הראשונה; וקריאות ה-Move שאחריו נמצאות במרחק צעד אחד מהפרת גישה. מה מכל אלו יקרה לכם תלוי במהדר ובמתגי הבנייה ולא במשהו שהמפענח החליט, וזה הסימן שהמפענח לא החליט דבר

התיקון הוא הטבלה מהמפרט, מיושמת במקום שבו שאר שדות ה-IHDR כבר נבדקו: COLOR_GRAYSCALE מקבל FSourceBitDepth in [1, 2, 4, 8, 16], COLOR_PALETTE מקבל [1, 2, 4, 8], ו-COLOR_RGB, COLOR_GRAYSCALEALPHA ו-COLOR_RGBALPHA מקבלים [8, 16]; כל דבר אחר מאפס את ValidImage והתמונה נדחית כשהרוחב והגובה שלה נשמרים לצורכי אבחון. מקטע pHYs קצר מתשעת הבתים שלו נסגר באותו סבב, כי קורא ה-DPI אינדקס את S[1] עד S[8] במחרוזת שהמקטע הקצר השאיר ריקה

היסט מבוסס-1 שטופל כמצביע מבוסס-0

InflateStrFromPosition(Const Input: AnsiString; StartPos, MaxOutput: Integer; Out Consumed: Integer): AnsiString מקבל StartPos מבוסס-1, כי הקלט שלו הוא AnsiString והמימוש בדלפי מפנה לקלט של zlib בתור @Input[StartPos]. המימוש ב-Free Pascal, שנכתב מול paszlib כדי ששתי מטרות ה-Windows יקשרו דחיסה סטטית, הגדיר את next_in בתור PAnsiChar(Input) + StartPos ואת avail_in בתור Length(Input) - StartPos. זה חשבון מצביעים, והוא מבוסס-0. העבירו 1, שזה מה ש"להתחיל מההתחלה" אומר לפונקציה הזו, ובניית ה-FPC מתחילה לנפח מהבית השני ונעצרת בית אחד לפני הסוף

הסיבה שזה שרד היא שהקורא היחיד שרוב הבדיקות מגיעות אליו הוא InflateStr, שמעביר 0. אפס הוא במקרה ההיסט הנכון מבוסס-0, כך ששתי הבניות הסכימו בכל קריאת InflateStr רגילה ובכל בדיקה שעברה דרכה. TPDFDocument.DecodeAllStreams, השגרה ש-SaveQDFToFile ו-ConvertFileToQDF משתמשות בה כדי לפרוש זרמי FlateDecode בודדים לצורה קריאה, מעבירה 1. בבניית ה-FPC הכותרת המדולגת של zlib גרמה ל-inflate להיכשל, אבל זרם ה-zlib עדיין דיווח Consumed שאינו אפס על הבתים שבדק, כך ש-DecodeAllStreams קיבל את המטען הריק כפענוח מוצלח והחליף כל זרם תוכן במחרוזת ריקה. ה-QDF שנוצר היה עם מספר העמודים הנכון, מבנה תקין, ובלי תוכן עמודים — קובץ שנפתח בלי שגיאה בכל צופה ולא מציג כלום

// ענף ה-FPC של InflateStrFromPosition, אחרי v3.539.16.
// StartPos מבוסס-1 כמו בענף דלפי; הצמידו אותו לטווח, ואז המירו
// להיסט מצביע מבוסס-0 בדיוק פעם אחת, בגבול.
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;

רגרסיית השמירה היא הקטנה ביותר האפשרית: דחסו מטען, נפחו אותו ממיקום 0 וממיקום 1, ואמתו ששניהם מחזירים את אותו מטען וששניהם מדווחים Consumed ששווה לאורך הזרם המלא. לזרם RFC 1950 יש כותרת בת שני בתים ו-trailer של Adler-32 בארבעה בתים, כך שהיסט של אחד באחד מהקצוות אינו השחתה עדינה, אלא זרם שאו נכשל להתחיל או נכשל לסיים. הלקח הוא על הגבול, לא על zlib: כשהפרמטר של פונקציה מוגדר בבסיס אינדקס אחד והמימוש שמתחת משתמש באחר, ההמרה שייכת לשורה אחת בדיוק, ובדיקה חייבת לקרוא לו עם הערך שמבדיל בין שני הבסיסים

למה Read קצר של TStream אינו סוף הזרם?

כי TStream.Read רשאי להחזיר פחות בתים מהמבוקש מכל סיבה שתרצה, ורק החזרה של 0 משמעה שאין עוד. TMemoryStream ו-TFileStream על דיסק מקומי כמעט תמיד ממלאים את הבקשה, ולכן קוד שמתייחס ל"הוחזר פחות ממה שביקשתי" כאל סוף קובץ עובר כל בדיקה שמשתמשת בהם. זרמים מבוססי רשת, זרמי דחיסה, וכל צאצא של TStream שלקוח כתב יכולים להחזיר שני בתים כשמבקשים שישים וארבעה אלף ועדיין להחזיק גיגהבתים מאחוריהם

TPLBuffer הוא הקורא שכל מפענח ב-PDF Library for Delphi עובר דרכו, והוא יכול לעטוף AnsiString, מצביע, מערך בתים או TStream. ארבע שאילתות הסריקה שלו — DistanceToByte, DistanceToOtherByte, DistanceToAnyByte ו-DistanceToOtherBytes, כולן מחזירות Int64 — קוראות את המקור במקטעים של 64 KB בחיפוש אחר תו מפריד ומדווחות כמה רחוק הוא בלי להזיז את המיקום הלוגי. כל לולאה הסתיימה ב-Until ReadCount < BlockSize. עבור שלושת המקורות שבזיכרון זה נכון, כי ReadIntoBuffer תמיד מספק את המקטע המלא עד האחרון. עבור מקור הזרם זה אומר שהסריקה מוותרת בקריאה הקצרה הראשונה, מדווחת שהתו המפריד חסר, והמפרק שמעליה מחליט שהאובייקט נגמר במקום שבו הוא לא נגמר

// TPLBuffer.DistanceToByte, הלולאה שאחרי v3.539.6.
// אפס הוא אות סוף הנתונים היחיד ש-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;   // הצצה אסור שתזיז את הקורא
End;

הבדיקה שמקבעת את זה היא צאצא של TMemoryStream שדריסת ה-Read שלו מגבילה כל בקשה לשני בתים. עטפו בה את המחרוזת aaaaaX, הגדירו את מיקום המאגר ל-1, וכל ארבע השאילתות חייבות לדווח מרחק של 4 עד ה-X, להשאיר את המיקום על 1 לאחר מכן, ולדווח 1- על בית שאינו שם. לפני התיקון השאילתה הראשונה ראתה שני בתים, הסיקה שהזרם נגמר, והחזירה 1-. ל-finally יש אותה חשיבות כמו לתנאי הלולאה: Exit מתוך הסריקה הוא נתיב ההצלחה הרגיל, והמיקום הלוגי חייב להשתחזר גם בנתיב הזה, ולא רק כשהלולאה רצה עד הסוף

קוד מקור אחד, שני מהדרים, מערכת אחת של בדיקות

המשמעת שיצאה מחמשת אלו היא ש"בניית דלפי עוברת" היא ראיה על דלפי, לא על קוד המקור. מאז v3.539.16 גם מערך הבדיקות DUnitX של דלפי וגם מערך הבדיקות הקונסולי של Free Pascal כוללים את אותו Tests\CrossCompilerSemantics.inc, שגרה אחת, RunCrossCompilerFileSemantics, שבונה מסמך בן שני עמודים עם תוכן דחוס דרך TPDFlib, שומרת אותו, שומרת אותו שוב כ-QDF דרך SaveQDFToFile, מתקנת את ה-QDF עם RepairQDFFile, מצפינה את הקובץ הרגיל ב-AES-128 דרך EncryptFile ומסכת הרשאות מ-EncodePermissions, ואז טוענת מחדש כל תוצר ומאמתת את אותם דברים בשני המהדרים: מספר העמודים הוא 2, הכותרת שורדת, הטקסט של העמוד השני מחולץ בשלמותו מהקובץ הרגיל, המתוקן והמוצפן, הסיסמה השגויה נדחית עם LastErrorCode שאינו אפס, EncryptionStrength הוא 128, EncryptionAlgorithm הוא 2, וסיביות ההרשאה הבודדות מ-GetUserPermissions חוזרות בדיוק כפי שאונקדו

ההשוואה מנורמלת בכוונה ולא בית-מול-בית. ההצפנה שואבת salts אקראיים והכותב מקצה מזהי מסמך, כך ששתי הבניות לא אמורות לפלוט קבצים זהים; הן אמורות לפלוט קבצים שמשמעותם זהה, והבדיקות מנוסחות ברמה הזו. רגל ה-QDF נמצאת שם במיוחד בגלל באג ההיסט: QDF עם שני עמודים ובלי תוכן עובר בדיקת מספר עמודים ונכשל בבדיקת חילוץ טקסט, והמטריצה מאמתת את השנייה. כל תיקון עתידי שהוא no-op במהדר אחד ושינוי התנהגות באחר — וזה מתאר ארבעה מחמשת שלמעלה — חייב כעת לעבור את אותן בדיקות פעמיים לפני שהוא נשלח

חצי הקישור (link-time) של אותו פורט — לגרום לאובייקטי ה-OMF של דלפי ולהציפיות ה-COFF של Free Pascal להסכים — הוא סיפור בפני עצמו בקישור אובייקטים OMF ל-COFF ב-FPC Win32, וההקשחה המבנית של אותו קורא TIFF מול BigTIFF וקבצים באריחים נמצאת בהערות המפענח TIFF המובנה. המפענחים במאמר הזה, ובדיקת החוצה-מהדרים שיושבת כעת מתחתיהם, נשלחים בPDF Library for Delphi עבור Delphi, C++Builder ו-Free Pascal, שם אותו קוד מקור אמור להרוויח את אותה תוצאה בכל מהדר שאליו הוא מכוון, ולא לקבל אותה במתנה מאחד מהם