Avoid inexact stream reads (CA2022)

Ensures that files are read correctly over unreliable streams Fixes: #191
ironfede · Oct 13, 2024 · e5af41e · e5af41e
1 parent baadaba
commit e5af41e
Show file tree

Hide file tree

Showing 5 changed files with 49 additions and 30 deletions.
diff --git a/sources/OpenMcdf/CFStream.cs b/sources/OpenMcdf/CFStream.cs
@@ -213,14 +213,13 @@ public void CopyFrom(Stream input)
         {
             CheckDisposed();
 
-            byte[] buffer = new byte[input.Length];
-
             if (input.CanSeek)
             {
                 input.Seek(0, SeekOrigin.Begin);
             }
 
-            input.Read(buffer, 0, (int)input.Length);
+            byte[] buffer = new byte[input.Length];
+            input.ReadExactly(buffer, 0, buffer.Length);
             SetData(buffer);
         }
 

diff --git a/sources/OpenMcdf/CompoundFile.cs b/sources/OpenMcdf/CompoundFile.cs
@@ -1433,7 +1433,7 @@ List<Sector> result
                     ms.Type = SectorType.Mini;
 
                     miniStreamView.Seek(nextSecID * Sector.MINISECTOR_SIZE, SeekOrigin.Begin);
-                    miniStreamView.Read(ms.GetData(), 0, Sector.MINISECTOR_SIZE);
+                    miniStreamView.ReadExactly(ms.GetData(), 0, Sector.MINISECTOR_SIZE);
 
                     result.Add(ms);
 
@@ -1886,7 +1886,7 @@ List<Sector> miniStream
                         };
 
                         miniStreamView.Seek(ms.Id * Sector.MINISECTOR_SIZE, SeekOrigin.Begin);
-                        miniStreamView.Read(ms.GetData(), 0, Sector.MINISECTOR_SIZE);
+                        miniStreamView.ReadExactly(ms.GetData(), 0, Sector.MINISECTOR_SIZE);
 
                         freeList.Enqueue(ms);
                     }
@@ -2038,7 +2038,7 @@ internal void SetStreamLength(CFItem cfItem, long length)
                     destSv.Write(buf, 0, cnt);
                 }
 
-                sv.Read(buf, 0, (int)toRead);
+                sv.ReadExactly(buf, 0, (int)toRead);
                 destSv.Write(buf, 0, (int)toRead);
 
                 //Free old chain
@@ -2087,7 +2087,7 @@ internal void SetStreamLength(CFItem cfItem, long length)
                     destSv.Write(buf, 0, cnt);
                 }
 
-                sv.Read(buf, 0, (int)toRead);
+                sv.ReadExactly(buf, 0, (int)toRead);
                 destSv.Write(buf, 0, (int)toRead);
 
                 //Free old mini chain

diff --git a/sources/OpenMcdf/Sector.cs b/sources/OpenMcdf/Sector.cs
@@ -65,8 +65,17 @@ public byte[] GetData()
 
                 if (IsStreamed)
                 {
-                    stream.Seek(Size + Id * (long)Size, SeekOrigin.Begin);
-                    stream.Read(data, 0, Size);
+                    long position = Size + Id * (long)Size;
+                    // Enlarge the stream if necessary and possible
+                    long endPosition = position + Size;
+                    if (endPosition > stream.Length)
+                    {
+                        if (!stream.CanWrite)
+                            return data;
+                        stream.SetLength(endPosition);
+                    }
+                    stream.Seek(position, SeekOrigin.Begin);
+                    stream.ReadExactly(data, 0, Size);
                 }
             }
 

diff --git a/sources/OpenMcdf/StreamExtensions.cs b/sources/OpenMcdf/StreamExtensions.cs
@@ -0,0 +1,24 @@
+using System;
+using System.IO;
+
+namespace OpenMcdf
+{
+    internal static class StreamExtensions
+    {
+        public static void ReadExactly(this Stream stream, byte[] buffer, int offset, int count)
+        {
+            if (stream is null)
+                throw new ArgumentNullException(nameof(stream));
+
+            int totalRead = 0;
+            do
+            {
+                int read = stream.Read(buffer, offset + totalRead, count - totalRead);
+                if (read == 0)
+                    throw new EndOfStreamException();
+
+                totalRead += read;
+            } while (totalRead < count);
+        }
+    }
+}
diff --git a/sources/OpenMcdf/StreamRW.cs b/sources/OpenMcdf/StreamRW.cs
@@ -21,70 +21,57 @@ public StreamRW(Stream stream)
             this.stream = stream;
         }
 
-        void ReadExactly(byte[] buffer, int offset, int count)
-        {
-            int totalRead = 0;
-            do
-            {
-                int read = stream.Read(buffer, offset + totalRead, count - totalRead);
-                if (read == 0)
-                    throw new EndOfStreamException();
-
-                totalRead += read;
-            } while (totalRead < count);
-        }
-
         public long Seek(long count, SeekOrigin origin)
         {
             return stream.Seek(count, origin);
         }
 
         public byte ReadByte()
         {
-            ReadExactly(buffer, 0, sizeof(byte));
+            stream.ReadExactly(buffer, 0, sizeof(byte));
             return buffer[0];
         }
 
         public ushort ReadUInt16()
         {
-            ReadExactly(buffer, 0, sizeof(ushort));
+            stream.ReadExactly(buffer, 0, sizeof(ushort));
             return (ushort)(buffer[0] | (buffer[1] << 8));
         }
 
         public int ReadInt32()
         {
-            ReadExactly(buffer, 0, sizeof(int));
+            stream.ReadExactly(buffer, 0, sizeof(int));
             return buffer[0] | (buffer[1] << 8) | (buffer[2] << 16) | (buffer[3] << 24);
         }
 
         public uint ReadUInt32()
         {
-            ReadExactly(buffer, 0, sizeof(uint));
+            stream.ReadExactly(buffer, 0, sizeof(uint));
             return (uint)(buffer[0] | (buffer[1] << 8) | (buffer[2] << 16) | (buffer[3] << 24));
         }
 
         public long ReadInt64()
         {
-            ReadExactly(buffer, 0, sizeof(long));
+            stream.ReadExactly(buffer, 0, sizeof(long));
             uint ls = (uint)(buffer[0] | (buffer[1] << 8) | (buffer[2] << 16) | (buffer[3] << 24));
             uint ms = (uint)((buffer[4]) | (buffer[5] << 8) | (buffer[6] << 16) | (buffer[7] << 24));
             return (long)(((ulong)ms << 32) | ls);
         }
 
         public ulong ReadUInt64()
         {
-            ReadExactly(buffer, 0, sizeof(ulong));
+            stream.ReadExactly(buffer, 0, sizeof(ulong));
             return (ulong)(buffer[0] | (buffer[1] << 8) | (buffer[2] << 16) | (buffer[3] << 24) | (buffer[4] << 32) | (buffer[5] << 40) | (buffer[6] << 48) | (buffer[7] << 56));
         }
 
         public void ReadBytes(byte[] result)
         {
-            ReadExactly(result, 0, result.Length);
+            stream.ReadExactly(result, 0, result.Length);
         }
 
         public Guid ReadGuid()
         {
-            ReadExactly(buffer, 0, 16);
+            stream.ReadExactly(buffer, 0, 16);
             return new Guid(buffer);
         }