Merge pull request #74 from neilharvey/bug-fix/malicious-file-formats

Fix PDF header vulnerability

src/FileSignatures/FileFormatInspector.cs

@@ -57,12 +57,12 @@ public FileFormatInspector(IEnumerable<FileFormat> formats)
                 RemoveBaseFormats(matches);
             }
 
-            if (matches.Count > 0)
+            if (matches.Count > 1)
             {
                 return matches.OrderByDescending(m => m.HeaderLength).First();
             }
 
-            return null;
+            return matches.Count == 1 ? matches[0] : null;
         }
 
         private List<FileFormat> FindMatchingFormats(Stream stream)

src/FileSignatures/Formats/AdobePdf.cs

@@ -0,0 +1,49 @@
+using System.IO;
+
+namespace FileSignatures.Formats;
+
+public class AdobePdf : Pdf
+{
+    private const byte VersionNumberPlaceholder = 0x00;
+
+    public AdobePdf() : base([
+        0x25, 0x21, 0x50, 0x53, 0x2D, 0x41, 0x64, 0x6F, 0x62, 0x65, 0x2D, VersionNumberPlaceholder, 0x2E,
+        VersionNumberPlaceholder, 0x20, 0x50, 0x44,
+        0x46, 0x2D, VersionNumberPlaceholder, 0x2E, VersionNumberPlaceholder
+    ])
+    {
+    }
+
+    public override bool IsMatch(Stream stream)
+    {
+        if (stream == null || (stream.Length < HeaderLength && HeaderLength < int.MaxValue) || Offset > stream.Length)
+        {
+            return false;
+        }
+
+        stream.Position = Offset;
+
+        for (var i = 0; i < Signature.Count; i++)
+        {
+            var b = stream.ReadByte();
+            if (!IsSignatureByte(b, i))
+            {
+                return false;
+            }
+        }
+
+        return true;
+    }
+
+    protected bool IsSignatureByte(int value, int signatureIndex)
+    {
+        return IsVersionNumber(value, Signature[signatureIndex])
+               || value == Signature[signatureIndex];
+    }
+
+    private static bool IsVersionNumber(int value, byte signatureByte)
+    {
+        var isNumber = value is >= 0x30 and <= 0x39;
+        return signatureByte == VersionNumberPlaceholder && isNumber;
+    }
+}

src/FileSignatures/Formats/OpenDocument.cs

@@ -0,0 +1,45 @@
+using System;
+using System.Collections.Generic;
+using System.Collections.ObjectModel;
+using System.IO;
+using System.Text;
+
+namespace FileSignatures.Formats;
+
+/// <summary>
+/// Specifies the format of an OpenDocument file.
+/// </summary>
+public abstract class OpenDocument : Zip
+{
+    private const int SubsignatureOffset = 30;
+
+    protected OpenDocument(byte[] subsignature, string mediaType, string extension) : base(SubsignatureOffset + subsignature.Length, mediaType, extension)
+    {
+        Subsignature = new ReadOnlyCollection<byte>(subsignature); ;
+    }
+
+    /// <summary>
+    /// Gets the subsignature of the OpenDocument file.
+    /// This will be the media type in byte format e.g. 'mimetypeapplication/vnd.oasis.opendocument.presentation'
+    /// </summary>
+    public ReadOnlyCollection<byte> Subsignature { get; }
+
+    public override bool IsMatch(Stream stream)
+    {
+        if (!base.IsMatch(stream))
+            return false;
+
+        stream.Position = SubsignatureOffset;
+
+        for (int i = 0; i < Subsignature.Count; i++)
+        {
+            var b = stream.ReadByte();
+            if (b != Subsignature[i])
+            {
+                return false;
+            }
+        }
+
+        return true;
+    }
+}

src/FileSignatures/Formats/OpenDocumentPresentation.cs

@@ -1,15 +1,15 @@
 namespace FileSignatures.Formats
 {
     /// <summary>
-    /// Specifies the format of a OffsetFileFormat file.
+    /// Specifies the format of a OpenDocument Presentation file.
     /// </summary>
-    public class OpenDocumentPresentation : FileFormat
+    public class OpenDocumentPresentation : OpenDocument
     {
-        public OpenDocumentPresentation() : base(new byte[] {
+        public OpenDocumentPresentation() : base([
             0x6D, 0x69, 0x6D, 0x65, 0x74, 0x79, 0x70, 0x65, 0x61, 0x70, 0x70, 0x6C, 0x69, 0x63, 0x61, 0x74, 0x69, 0x6F, 0x6E,
             0x2F, 0x76, 0x6E, 0x64, 0x2E, 0x6F, 0x61, 0x73, 0x69, 0x73, 0x2E, 0x6F, 0x70, 0x65, 0x6E, 0x64, 0x6F, 0x63, 0x75,
             0x6D, 0x65, 0x6E, 0x74, 0x2E, 0x70, 0x72, 0x65, 0x73, 0x65, 0x6E, 0x74, 0x61, 0x74, 0x69, 0x6F, 0x6E
-        }, "application/vnd.oasis.opendocument.presentation", "odp", 30)
+        ], "application/vnd.oasis.opendocument.presentation", "odp")
         {
         }
     }

src/FileSignatures/Formats/OpenDocumentSpreadsheet.cs

@@ -3,13 +3,13 @@
     /// <summary>
     /// Specifies the format of a OffsetFileFormat file.
     /// </summary>
-    public class OpenDocumentSpreadsheet : FileFormat
+    public class OpenDocumentSpreadsheet : OpenDocument
     {
-        public OpenDocumentSpreadsheet() : base(new byte[] {
+        public OpenDocumentSpreadsheet() : base([
             0x6D, 0x69, 0x6D, 0x65, 0x74, 0x79, 0x70, 0x65, 0x61, 0x70, 0x70, 0x6C, 0x69, 0x63, 0x61, 0x74, 0x69, 0x6F,
             0x6E, 0x2F, 0x76, 0x6E, 0x64, 0x2E, 0x6F, 0x61, 0x73, 0x69, 0x73, 0x2E, 0x6F, 0x70, 0x65, 0x6E, 0x64, 0x6F,
             0x63, 0x75, 0x6D, 0x65, 0x6E, 0x74, 0x2E, 0x73, 0x70, 0x72, 0x65, 0x61, 0x64, 0x73, 0x68, 0x65, 0x65, 0x74
-        }, "application/vnd.oasis.opendocument.spreadsheet", "ods", 30)
+        ], "application/vnd.oasis.opendocument.spreadsheet", "ods")
         {
         }
     }

src/FileSignatures/Formats/OpenDocumentText.cs

@@ -3,13 +3,13 @@
     /// <summary>
     /// Specifies the format of a OpenDocumentText file.
     /// </summary>
-    public class OpenDocumentText : FileFormat
+    public class OpenDocumentText : OpenDocument
     {
-        public OpenDocumentText() : base(new byte[] {
+        public OpenDocumentText() : base([
             0x6D, 0x69, 0x6D, 0x65, 0x74, 0x79, 0x70, 0x65, 0x61, 0x70, 0x70, 0x6C, 0x69, 0x63, 0x61, 0x74,
             0x69, 0x6F, 0x6E, 0x2F, 0x76, 0x6E, 0x64, 0x2E, 0x6F, 0x61, 0x73, 0x69, 0x73, 0x2E, 0x6F, 0x70,
             0x65, 0x6E, 0x64, 0x6F, 0x63, 0x75, 0x6D, 0x65, 0x6E, 0x74, 0x2E, 0x74, 0x65, 0x78, 0x74
-        }, "application/vnd.oasis.opendocument.text", "odt", 30)
+        ], "application/vnd.oasis.opendocument.text", "odt")
         {
         }
     }

src/FileSignatures/Formats/Pdf.cs

@@ -7,76 +7,13 @@ namespace FileSignatures.Formats
     /// </summary>
     public class Pdf : FileFormat
     {
-        private const uint MaxFileHeaderSize = 1024;
-
         public Pdf() : this([0x25, 0x50, 0x44, 0x46])
         {
         }
 
         protected Pdf(byte[] signature) : base(signature, "application/pdf", "pdf", 0)
         {
         }
-
-        public override bool IsMatch(Stream stream)
-        {
-            if (stream == null || stream.Length < HeaderLength)
-            {
-                return false;
-            }
-
-            stream.Position = 0;
-            var signatureValidationIndex = 0;
-            int fileByte;
-
-            while (stream.Position < MaxFileHeaderSize && (fileByte = stream.ReadByte()) != -1)
-            {
-                if (CompareFileByteToSignatureAt((byte)fileByte, signatureValidationIndex))
-                {
-                    signatureValidationIndex++;
-                }
-                else
-                {
-                    signatureValidationIndex = 0;
-                }
-
-                if (signatureValidationIndex == Signature.Count)
-                    return true;
-            }
-
-            return false;
-        }
-
-        protected virtual bool CompareFileByteToSignatureAt(byte fileByte, int signatureIndex)
-        {
-            return fileByte == Signature[signatureIndex];
-        }
-    }
-
-    public class AdobePdf : Pdf
-    {
-        private const byte VersionNumberPlaceholder = 0x00;
-
-        public AdobePdf() : base([
-            0x25, 0x21, 0x50, 0x53, 0x2D, 0x41, 0x64, 0x6F, 0x62, 0x65, 0x2D, VersionNumberPlaceholder, 0x2E,
-            VersionNumberPlaceholder, 0x20, 0x50, 0x44,
-            0x46, 0x2D, VersionNumberPlaceholder, 0x2E, VersionNumberPlaceholder
-        ])
-        {
-        }
-
-        protected override bool CompareFileByteToSignatureAt(byte fileByte, int signatureIndex)
-        {
-            return base.CompareFileByteToSignatureAt(fileByte, signatureIndex) || IsVersionNumber(fileByte, Signature[signatureIndex]);
-        }
-
-        private static bool IsVersionNumber(byte fileByte, byte signatureByte)
-        {
-            return signatureByte == VersionNumberPlaceholder && IsNumber(fileByte);
-        }
-
-        private static bool IsNumber(byte @byte)
-        {
-            return @byte is >= 0x30 and <= 0x39;
-        }
+
     }
 }

test/FileSignatures.Tests/FunctionalTests.cs

@@ -19,8 +19,7 @@ public class FunctionalTests
         [InlineData("dragndrop.msg", "application/vnd.ms-outlook")]
         [InlineData("nonstandard.docx","application/vnd.openxmlformats-officedocument.wordprocessingml.document")]
         [InlineData("test.pdf", "application/pdf")]
-        [InlineData("test_header_somewhere_in_1024_first_bytes.pdf", "application/pdf")]
-        [InlineData("test_header_adobe.pdf", "application/pdf")]
+        [InlineData("adobe.pdf", "application/pdf")]
         [InlineData("test.rtf", "application/rtf")]
         [InlineData("test.png", "image/png")]
         [InlineData("test.ppt", "application/vnd.ms-powerpoint")]
@@ -52,6 +51,7 @@ public class FunctionalTests
         [InlineData("test.ogg", "audio/ogg")]
         [InlineData("test.amr", "audio/amr")]
         [InlineData("test.ico", "image/vnd.microsoft.icon")]
+        [InlineData("malicious.pdf", "application/vnd.microsoft.portable-executable")]
         public void SamplesAreRecognised(string sample, string expected)
         {
             var result = InspectSample(sample);
@@ -63,14 +63,11 @@ public void SamplesAreRecognised(string sample, string expected)
         private static FileFormat InspectSample(string fileName)
         {
             var inspector = new FileFormatInspector();
-            var buildDirectoryPath = Path.GetDirectoryName(typeof(FunctionalTests).GetTypeInfo().Assembly.Location);
+            var buildDirectoryPath = Path.GetDirectoryName(typeof(FunctionalTests).GetTypeInfo().Assembly.Location)!;
             var sample = new FileInfo(Path.Combine(buildDirectoryPath, "Samples", fileName));
-            FileFormat result;
 
-            using (var stream = sample.OpenRead())
-            {
-                result = inspector.DetermineFileFormat(stream);
-            }
+            using var stream = sample.OpenRead();
+            var result = inspector.DetermineFileFormat(stream);
 
             return result;
         }