Merge pull request #456 from Washi1337/issue/versioninfo-alignments

BUGFIX: Align all VersionInfo tables to 4-byte boundary

src/AsmResolver.PE.Win32Resources/Icon/IconResource.cs

@@ -98,7 +98,7 @@ public void WriteToDirectory(IResourceDirectory rootDirectory)
                 foreach (var (groupEntry, iconEntry) in entry.Value.GetIconEntries())
                 {
                     newIconDirectory.Entries.Add(new ResourceDirectory(groupEntry.Id)
-                        {Entries = {new ResourceData(0u, iconEntry)}});
+                        {Entries = {new ResourceData(1033, iconEntry)}});
                 }
             }
 

src/AsmResolver.PE.Win32Resources/Version/StringTable.cs

@@ -73,6 +73,53 @@ public class StringTable : VersionTableEntry, IEnumerable<KeyValuePair<string, s
         /// </summary>
         public const string SpecialBuildKey = "SpecialBuild";
 
+        private readonly Dictionary<string, string> _entries = new();
+
+        /// <summary>
+        /// Creates a new string table.
+        /// </summary>
+        /// <param name="languageIdentifier">The language identifier.</param>
+        /// <param name="codePage">The code page.</param>
+        public StringTable(ushort languageIdentifier, ushort codePage)
+        {
+            LanguageIdentifier = languageIdentifier;
+            CodePage = codePage;
+        }
+
+        /// <inheritdoc />
+        public override string Key => $"{LanguageIdentifier:x4}{CodePage:x4}";
+
+        /// <summary>
+        /// Gets or sets the language identifier of this string table.
+        /// </summary>
+        public ushort LanguageIdentifier
+        {
+            get;
+            set;
+        }
+
+        /// <summary>
+        /// Gets or sets the code page of this string table.
+        /// </summary>
+        public ushort CodePage
+        {
+            get;
+            set;
+        }
+
+        /// <inheritdoc />
+        protected override VersionTableValueType ValueType => VersionTableValueType.Binary;
+
+        /// <summary>
+        /// Gets or sets the value of a single field in the string table.
+        /// </summary>
+        /// <param name="key">The name of the field in the string table.</param>
+        public string this[string key]
+        {
+            get => _entries[key];
+            set => _entries[key] = value;
+        }
+
         /// <summary>
         /// Reads a single StringTable structure from the provided input stream.
         /// </summary>
@@ -122,53 +169,6 @@ private static KeyValuePair<string, string> ReadEntry(ref BinaryStreamReader rea
             return new KeyValuePair<string, string>(header.Key, value);
         }
 
-        private readonly Dictionary<string, string> _entries = new Dictionary<string, string>();
-
-        /// <summary>
-        /// Creates a new string table.
-        /// </summary>
-        /// <param name="languageIdentifier">The language identifier.</param>
-        /// <param name="codePage">The code page.</param>
-        public StringTable(ushort languageIdentifier, ushort codePage)
-        {
-            LanguageIdentifier = languageIdentifier;
-            CodePage = codePage;
-        }
-
-        /// <inheritdoc />
-        public override string Key => $"{LanguageIdentifier:X4}{CodePage:X4}";
-
-        /// <summary>
-        /// Gets or sets the language identifier of this string table.
-        /// </summary>
-        public ushort LanguageIdentifier
-        {
-            get;
-            set;
-        }
-
-        /// <summary>
-        /// Gets or sets the code page of this string table.
-        /// </summary>
-        public ushort CodePage
-        {
-            get;
-            set;
-        }
-
-        /// <inheritdoc />
-        protected override VersionTableValueType ValueType => VersionTableValueType.Binary;
-
-        /// <summary>
-        /// Gets or sets the value of a single field in the string table.
-        /// </summary>
-        /// <param name="key">The name of the field in the string table.</param>
-        public string this[string key]
-        {
-            get => _entries[key];
-            set => _entries[key] = value;
-        }
-
         /// <summary>
         /// Adds (or overrides) a field to the string table.
         /// </summary>

src/AsmResolver.PE.Win32Resources/Version/VersionInfoResource.cs

@@ -15,6 +15,34 @@ public class VersionInfoResource : VersionTableEntry, IWin32Resource
         /// </summary>
         public const string VsVersionInfoKey = "VS_VERSION_INFO";
 
+        private FixedVersionInfo _fixedVersionInfo = new();
+        private readonly Dictionary<string, VersionTableEntry> _entries = new();
+
+        /// <inheritdoc />
+        public override string Key => VsVersionInfoKey;
+
+        /// <inheritdoc />
+        protected override VersionTableValueType ValueType => VersionTableValueType.Binary;
+
+        /// <summary>
+        /// Gets the fixed version info stored in this version resource.
+        /// </summary>
+        public FixedVersionInfo FixedVersionInfo
+        {
+            get => _fixedVersionInfo;
+            set => _fixedVersionInfo = value ?? throw new ArgumentNullException(nameof(value));
+        }
+
+        /// <summary>
+        /// Gets or sets a version table entry by its name.
+        /// </summary>
+        /// <param name="name">The name of the child.</param>
+        public VersionTableEntry this[string name]
+        {
+            get => _entries[name];
+            set => _entries[name] = value;
+        }
+
         /// <summary>
         /// Obtains the version info resource from the provided root win32 resources directory.
         /// </summary>
@@ -97,34 +125,6 @@ private static VersionTableEntry ReadNextEntry(ref BinaryStreamReader reader)
             };
         }
 
-        private FixedVersionInfo _fixedVersionInfo = new FixedVersionInfo();
-        private readonly Dictionary<string, VersionTableEntry> _entries = new Dictionary<string, VersionTableEntry>();
-
-        /// <inheritdoc />
-        public override string Key => VsVersionInfoKey;
-
-        /// <inheritdoc />
-        protected override VersionTableValueType ValueType => VersionTableValueType.Binary;
-
-        /// <summary>
-        /// Gets the fixed version info stored in this version resource.
-        /// </summary>
-        public FixedVersionInfo FixedVersionInfo
-        {
-            get => _fixedVersionInfo;
-            set => _fixedVersionInfo = value ?? throw new ArgumentNullException(nameof(value));
-        }
-
-        /// <summary>
-        /// Gets or sets a version table entry by its name.
-        /// </summary>
-        /// <param name="name">The name of the child.</param>
-        public VersionTableEntry this[string name]
-        {
-            get => _entries[name];
-            set => _entries[name] = value;
-        }
-
         /// <summary>
         /// Gets a collection of entries stored in the version resource.
         /// </summary>
@@ -181,9 +181,11 @@ public override uint GetPhysicalSize()
         protected override void WriteValue(IBinaryStreamWriter writer)
         {
             FixedVersionInfo.Write(writer);
-            writer.Align(4);
             foreach (var entry in _entries.Values)
+            {
+                writer.Align(4);
                 entry.Write(writer);
+            }
         }
 
         /// <inheritdoc />
@@ -196,7 +198,7 @@ public void WriteToDirectory(IResourceDirectory rootDirectory)
                 {
                     new ResourceDirectory(1)
                     {
-                        Entries = {new ResourceData(0u, this)}
+                        Entries = {new ResourceData(1033, this)}
                     }
                 }
             };

test/AsmResolver.PE.Win32Resources.Tests/AsmResolver.PE.Win32Resources.Tests.csproj

@@ -23,6 +23,7 @@
     <ItemGroup>
       <ProjectReference Include="..\..\src\AsmResolver.PE.Win32Resources\AsmResolver.PE.Win32Resources.csproj" />
       <ProjectReference Include="..\..\src\AsmResolver.PE\AsmResolver.PE.csproj" />
+      <ProjectReference Include="..\AsmResolver.Tests\AsmResolver.Tests.csproj" />
     </ItemGroup>
 
     <ItemGroup>
@@ -40,8 +41,4 @@
       </Compile>
     </ItemGroup>
 
-    <ItemGroup>
-      <Folder Include="Resources" />
-    </ItemGroup>
-
 </Project>

test/AsmResolver.PE.Win32Resources.Tests/Properties/Resources.resx

@@ -3,7 +3,7 @@
 <root>
     <xsd:schema id="root" xmlns="" xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:msdata="urn:schemas-microsoft-com:xml-msdata">
         <xsd:element name="root" msdata:IsDataSet="true">
-            
+
         </xsd:element>
     </xsd:schema>
     <resheader name="resmimetype">
@@ -21,4 +21,7 @@
     <data name="HelloWorld" type="System.Resources.ResXFileRef, System.Windows.Forms">
         <value>..\Resources\HelloWorld.exe;System.Byte[], mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089</value>
     </data>
-</root>
+    <data name="HelloWorld_PaddedVersionInfo" type="System.Resources.ResXFileRef, System.Windows.Forms">
+        <value>..\Resources\HelloWorld.PaddedVersionInfo.exe;System.Byte[], mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089</value>
+    </data>
+</root>

test/AsmResolver.PE.Win32Resources.Tests/Version/VersionInfoResourceTest.cs

@@ -2,13 +2,24 @@
 using System.IO;
 using AsmResolver.IO;
 using AsmResolver.PE.DotNet.Builder;
+using AsmResolver.PE.File;
+using AsmResolver.PE.File.Headers;
+using AsmResolver.PE.Win32Resources.Builder;
 using AsmResolver.PE.Win32Resources.Version;
+using AsmResolver.Tests.Runners;
 using Xunit;
 
 namespace AsmResolver.PE.Win32Resources.Tests.Version
 {
-    public class VersionInfoSegmentTest
+    public class VersionInfoResourceTest : IClassFixture<TemporaryDirectoryFixture>
     {
+        private readonly TemporaryDirectoryFixture _fixture;
+
+        public VersionInfoResourceTest(TemporaryDirectoryFixture fixture)
+        {
+            _fixture = fixture;
+        }
+
         [Fact]
         public void ReadFixedVersion()
         {
@@ -192,5 +203,41 @@ public void PersistentVersionResource()
             Assert.Equal(versionInfo.FixedVersionInfo.ProductVersion, newVersionInfo.FixedVersionInfo.ProductVersion);
         }
 
+        [Fact]
+        public void VersionInfoAlignment()
+        {
+            // https://github.com/Washi1337/AsmResolver/issues/202
+
+            // Open dummy
+            var file = PEFile.FromBytes(Properties.Resources.HelloWorld_PaddedVersionInfo);
+            var image = PEImage.FromFile(file);
+
+            // Update version info.
+            var versionInfo = VersionInfoResource.FromDirectory(image.Resources!)!;
+            var info = versionInfo.GetChild<StringFileInfo>(StringFileInfo.StringFileInfoKey);
+            info.Tables[0][StringTable.FileDescriptionKey] = "This is a test application";
+            versionInfo.WriteToDirectory(image.Resources!);
+
+            // Replace section.
+            var resourceBuffer = new ResourceDirectoryBuffer();
+            resourceBuffer.AddDirectory(image.Resources!);
+
+            var section = file.GetSectionContainingRva(file.OptionalHeader.GetDataDirectory(DataDirectoryIndex.ResourceDirectory).VirtualAddress);
+            section.Contents = resourceBuffer;
+
+            file.UpdateHeaders();
+            file.OptionalHeader.SetDataDirectory(DataDirectoryIndex.ResourceDirectory,
+                new DataDirectory(resourceBuffer.Rva, resourceBuffer.GetPhysicalSize()));
+
+            // Rebuild
+            using var stream = new MemoryStream();
+            file.Write(stream);
+
+            // Reopen and verify.
+            var newImage = PEImage.FromBytes(stream.ToArray());
+            var newVersionInfo = VersionInfoResource.FromDirectory(newImage.Resources!)!;
+            var newInfo = newVersionInfo.GetChild<StringFileInfo>(StringFileInfo.StringFileInfoKey);
+            Assert.Equal("This is a test application", newInfo.Tables[0][StringTable.FileDescriptionKey]);
+        }
     }
 }