ironfede / openmcdf

Microsoft Compound File .net component - pure C# - netstandard 2.0
Mozilla Public License 2.0
297 stars 73 forks source link

Bugs when reading DictionaryProperty #84

Closed provegard closed 1 year ago

provegard commented 3 years ago

Hi!

I encountered a couple of bugs that prevents me from reading OLE properties. The bugs relate to reading a DictionaryProperty.

The first two problems are in DictionaryEntry.cs:

nameBytes = br.ReadBytes(Length << 2);

This code path is taken when the property name consists of Unicode-16 characters. However, Length << 2 multiplies with 4, not 2.

Furthermore, the reading of padding uses Length rather than the actual bytes length:

int m = Length % 4;

Here are the necessary changes:

             }
             else
             {
-                nameBytes = br.ReadBytes(Length << 2);
+                int bytesLen = Length * 2;
+                nameBytes = br.ReadBytes(bytesLen);

-                int m = Length % 4;
+                int m = bytesLen % 4;
                 if (m > 0)
                     br.ReadBytes(m);
             }

The last problem is in OLEPropertiesContainer.cs:

         this.PropertyNames = (Dictionary<uint, string>)pStream.PropertySet0.Properties      
             .Where(p => p.PropertyType == PropertyType.DictionaryProperty).FirstOrDefault();

This results in InvalidCastException because DictionaryProperty is not a Dictionary<uint, string>.

The necessary change:


             this.PropertyNames = (Dictionary<uint, string>)pStream.PropertySet0.Properties
-                .Where(p => p.PropertyType == PropertyType.DictionaryProperty).FirstOrDefault();
+                .Where(p => p.PropertyType == PropertyType.DictionaryProperty).FirstOrDefault()?.Value;

             this.Context = new PropertyContext()
             {

If you like, I can prepare a PR with a test case and the changes.

ironfede commented 1 year ago

Thank you @provegard and sorry for response after all this time. I've pushed your fix to master branch. I haven't still found a "real world" file example to check so there is no specific test added. If you're still using OpenMcdf please open a new ticket if you find something wrong with the implementation or attach a file (with no sensitive info obviously) to add to test files. Thank you very much, Best Regards, Federico