Added protection for compound files with cyclic fat structure. Addres…

…ses ironfede#30, ironfede#39 and ironfede#40

sources/OpenMcdf/CompoundFile.cs

@@ -1271,6 +1271,8 @@ List<Sector> result
             int nextSecID
                = Sector.ENDOFCHAIN;
 
+			HashSet<int> processedSectors = new HashSet<int>();
+
             if (header.DIFATSectorsNumber != 0)
             {
                 validationCount = (int)header.DIFATSectorsNumber;
@@ -1290,6 +1292,7 @@ int nextSecID
                 while (true && validationCount >= 0)
                 {
                     nextSecID = BitConverter.ToInt32(s.GetData(), GetSectorSize() - 4);
+                    EnsureUniqueSectorIndex(nextSecID, processedSectors);
 
                     // Strictly speaking, the following condition is not correct from
                     // a specification point of view:
@@ -1321,6 +1324,16 @@ int nextSecID
             return result;
         }
 
+        private static void EnsureUniqueSectorIndex(int nextSecID, HashSet<int> processedSectors)
+        {
+	        if (processedSectors.Contains(nextSecID))
+	        {
+		        throw new CFCorruptedFileException("The file is corrupted.");
+	        }
+
+	        processedSectors.Add(nextSecID);
+        }
+
         /// <summary>
         /// Get the FAT sector chain
         /// </summary>
@@ -1361,6 +1374,7 @@ int nextSecID
             //Is there any DIFAT sector containing other FAT entries ?
             if (difatSectors.Count > 0)
             {
+	            HashSet<int> processedSectors = new HashSet<int>();
                 StreamView difatStream
                     = new StreamView
                         (
@@ -1404,6 +1418,7 @@ StreamView difatStream
 
                     difatStream.Read(nextDIFATSectorBuffer, 0, 4);
                     nextSecID = BitConverter.ToInt32(nextDIFATSectorBuffer, 0);
+                    EnsureUniqueSectorIndex(nextSecID, processedSectors);
                     nFat++;
                 }
             }
@@ -1425,6 +1440,7 @@ List<Sector> result
             int nextSecID = secID;
 
             List<Sector> fatSectors = GetFatSectorChain();
+			HashSet<int> processedSectors = new HashSet<int>();
 
             StreamView fatStream
                 = new StreamView(fatSectors, GetSectorSize(), fatSectors.Count * GetSectorSize(), null, sourceStream);
@@ -1453,10 +1469,9 @@ StreamView fatStream
                 fatStream.Seek(nextSecID * 4, SeekOrigin.Begin);
                 int next = fatStream.ReadInt32();
 
-                if (next != nextSecID)
-                    nextSecID = next;
-                else
-                    throw new CFCorruptedFileException("Cyclic sector chain found. File is corrupted");
+                EnsureUniqueSectorIndex(next, processedSectors);
+                nextSecID = next;
+
             }
 
 
@@ -1490,6 +1505,8 @@ StreamView miniFATView
 
                 nextSecID = secID;
 
+                HashSet<int> processedSectors = new HashSet<int>();
+
                 while (true)
                 {
                     if (nextSecID == Sector.ENDOFCHAIN)
@@ -1509,10 +1526,8 @@ StreamView miniFATView
                     miniFATView.Seek(nextSecID * 4, SeekOrigin.Begin);
                     int next = miniFATReader.ReadInt32();
 
-                    if (next != nextSecID)
-                        nextSecID = next;
-                    else
-                        throw new CFCorruptedFileException("Cyclic sector chain found. File is corrupted");
+                    nextSecID = next;
+					EnsureUniqueSectorIndex(nextSecID, processedSectors);
                 }
             }
             return result;

sources/OpenMcdf/StreamView.cs

@@ -176,6 +176,8 @@ public override int Read(byte[] buffer, int offset, int count)
 
                 if (nToRead != 0)
                 {
+					if (secIndex > sectorChain.Count) throw new CFCorruptedFileException("The file is probably corrupted.");
+
                     Buffer.BlockCopy(
                         sectorChain[secIndex].GetData(),
                         0,

sources/Test/OpenMcdf.Test/CompoundFileTest.cs

@@ -1050,6 +1050,33 @@ public void Test_FIX_GH_38_B()
             }
         }
 
+        [TestMethod]
+        [ExpectedException(typeof(CFCorruptedFileException))]
+        public void Test_CorruptedSectorChain_Doc()
+        {
+	        var f = new CompoundFile("corrupted-sector-chain.doc");
+
+	        f.Close();
+        } 
+
+        [TestMethod]
+        [ExpectedException(typeof(CFCorruptedFileException))]
+        public void Test_CorruptedSectorChain_Cfs()
+        {
+	        var f = new CompoundFile("corrupted-sector-chain.cfs");
+
+	        f.Close();
+        }
+
+        [TestMethod]
+        [ExpectedException(typeof(CFCorruptedFileException))]
+        public void Test_CorruptedSectorChain_Doc2()
+        {
+	        var f = new CompoundFile("corrupted-sector-chain-2.doc");
+
+	        f.Close();
+        }
+
         //[TestMethod]
         //public void Test_CORRUPTED_CYCLIC_DIFAT_VALIDATION_CHECK()
         //{

sources/Test/OpenMcdf.Test/SectorCollectionTest.cs

@@ -121,7 +121,7 @@ public void ItemTest()
             }
             catch (Exception ex)
             {
-                Assert.IsTrue(ex is ArgumentOutOfRangeException);
+                Assert.IsTrue(ex is CFException);
             }
 
             try
@@ -130,7 +130,7 @@ public void ItemTest()
             }
             catch (Exception ex)
             {
-                Assert.IsTrue(ex is ArgumentOutOfRangeException);
+                Assert.IsTrue(ex is CFException);
             }
         }