diff --git a/src/Tasks.UnitTests/ResourceHandling/GenerateResource_Tests.cs b/src/Tasks.UnitTests/ResourceHandling/GenerateResource_Tests.cs index 94131f1e4b6..f704ef169a1 100644 --- a/src/Tasks.UnitTests/ResourceHandling/GenerateResource_Tests.cs +++ b/src/Tasks.UnitTests/ResourceHandling/GenerateResource_Tests.cs @@ -1940,6 +1940,48 @@ public void InvalidStateFile() } } + [Fact] + public void GenerateResourceWarnsWhenUsingBinaryFormatter() + { + using TestEnvironment env = TestEnvironment.Create(); + TransientTestFile resource = env.CreateFile(".resx", @" + + + + AAABAAEAEBAAAAAAIABoBAAAFgAAACgAAAAQAAAAIAAAAAEAIAAAAAAAQAQAAAAAAAAAAAAAAAAAAAAA + AAD///8BoqKiDaKiotmioqL5oqKiK////wH///8B////Af///wH///8B////AaKioiGioqLxoqKi5aKi + ohn///8B////AbS0tBW0tLTz29vb/7Ozsu18Wi+Be1gswXtYLO17WCzte1gswXtYLIGzs7Lz2dnZ/7S0 + tPu0tLQj////Af///wH///8BxsbGQdPT0//Cv739nGs7/6ZsNf+ubzf/rm83/6ZsNf+hdkr/xcTD/8bG + xf/GxsY/////Af///wH///8B////AYxlNmejiGn1r3hE/7uMXv/Ck3H/xJF0/8OPcf+/kGz/uIpd/7SG + Wf+hhWT1jGU2Z////wH///8B////AZZtOzWWbTvVs31G/8KZcf/Yqon/79/P//r28//69fP/79/R/9en + hf++lGz/s31G/5ZtO9WWbTs1////Af///wGhdUGBsIBK/8abb//Zqoj///7r///67v///fL///7y///8 + 7////ev/2aN6/8KZbP+wgEr/oXVBgf///wH///8BrH5Iwb+PWP/No4H/8NvB///35v/68uP/xcC2//Ht + 3v///Oj///Xf/+/Ur//ImXL/v49Y/6x+SMH///8B////AbeHTu3JnGb/z5+A//rz4v/99un/8vDj/42M + hP+Bf3f/0s/C///76//67Mz/x5Bt/8mcZv+3h07t////Af///wHCkFTtzqZx/9Glif/69un//fju//// + +f+BgHn/sa6k/4F/d//Jxrr/+vDT/8mWcv/OpnH/wpBU7f///wH///8BzZlbwdOsdf/Zt5j/8ePW//77 + 9f/19fP/n56V//Dw6f/4+PL/vrmt//Dawv/Sqof/06x1/82ZW8H///8B////AbOddIvTrXf/38Sa/969 + qv//////8PDu/+fl2v////f////3///+8//ctJj/28CW/8Kqfv/Gn2qF////AQCZ3T0KmtjZLpzF9d6/ + iv/iyaf/37+u//Hj3P/z8ez/9PHr//Hi2f/cuqP/38Oe/4yxqf84ptH5DprWzwCZ3ScAoON9fNHy7WHD + 6O86pMb74seS/+bRqf/gwqb/1a6W/9Wrkv/evaD/5M+m/7/Bnv9Hstf9q+P2/Smw6NkAoOMnAKfpe13J + 8eW16Pn/Ycfr7zqqzPPsxIj/6cuU/+fQnf/n0J3/6cuU/97Cjv8yqtD1gdPw9XPQ8+sAp+nNAKfpBQCu + 7wUAru+LW8v05b/s+v9cy/HpTbLJxfq8dMH6vHTt+rx07fq8dMFRssjDac/y7XzW9u0Aru/JAK7vHf// + /wH///8BALX0AwC19IEAtfTRALX0ywC19Af///8B////Af///wH///8BALX0FwC19NEAtfTJALX0J/// + /wH///8BAAD//wAA//8AAP//AAD//wAA//8AAP//AAD//wAA//8AAP//AAD//wAA//8AAP//AAD//wAA + //8AAP//AAD//w== + + + +"); + + GenerateResource gr = Utilities.CreateTask(_output, usePreserialized: true, env: env); + gr.Sources = new ITaskItem[] { new TaskItem(resource.Path) }; + gr.WarnOnBinaryFormatterUse = true; + + gr.Execute().ShouldBeTrue(); + + Utilities.AssertLogContainsResource(gr, "GenerateResource.BinaryFormatterUse", "$this.Icon", "System.Drawing.Icon, System.Drawing"); + } + /// /// Cause failures in ResourceReader /// diff --git a/src/Tasks.UnitTests/ResourceHandling/MSBuildResXReader_Tests.cs b/src/Tasks.UnitTests/ResourceHandling/MSBuildResXReader_Tests.cs index d1c0b84fdc0..c161b862071 100644 --- a/src/Tasks.UnitTests/ResourceHandling/MSBuildResXReader_Tests.cs +++ b/src/Tasks.UnitTests/ResourceHandling/MSBuildResXReader_Tests.cs @@ -32,7 +32,7 @@ public void ParsesSingleStringAsString() @" StringValue Comment - ")); + "), null, false); AssertSingleStringResource(resxWithSingleString, "StringResource", "StringValue"); } @@ -45,7 +45,7 @@ public void ParsesSingleStringWithoutPreserveAsString() @" StringValue Comment - ")); + "), null, false); AssertSingleStringResource(resxWithSingleString, "StringResource", " StringValue "); } @@ -58,7 +58,7 @@ public void ParsesSingleWhitespaceStringAsString() @" Comment - ")); + "), null, false); AssertSingleStringResource(resxWithSingleString, "StringResource", " "); } @@ -71,7 +71,7 @@ public void ParsesSingleWhitespaceStringWithNoPreserveAsEmptyString() @" Comment - ")); + "), null, false); AssertSingleStringResource(resxWithSingleString, "StringResource", ""); } @@ -83,7 +83,7 @@ public void ParsesSingleStringWithPartialTypeName() ResXHelper.SurroundWithBoilerplate( @" StringValue - ")); + "), null, false); AssertSingleStringResource(resxWithSingleString, "StringResource", "StringValue"); } @@ -100,7 +100,7 @@ public void LoadsMultipleStringsPreservingOrder() 2StringValue2 - ")); + "), null, false); resxWithTwoStrings.Count.ShouldBe(2); @@ -121,7 +121,7 @@ public void ResXNullRefProducesNullLiveObject() @" - ")); + "), null, false); resxWithNullRef.ShouldHaveSingleItem(); @@ -143,7 +143,7 @@ public void LoadsStringFromFileRefAsString(string stringType) $@" ResourceHandling\TextFile1.txt;{stringType};utf-8 - ")); + "), null, false); AssertSingleStringResource(resxWithLinkedString, "TextFile1", "Contents of TextFile1"); } @@ -174,6 +174,8 @@ public void LoadsStringFromFileRefAsStringWithShiftJISEncoding() ResourceHandling\TextFileInShiftJIS.txt;System.String, mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089;shift_jis "), + null, + false, Path.Combine(baseDir.Path, nameof(LoadsStringFromFileRefAsStringWithShiftJISEncoding) + ".resx"), useRelativePath: true); @@ -210,7 +212,7 @@ public void PassesThroughBitmapInResx() b7eblRw4yy8Ta2GCpaZp1sIzz2LfCMS+EYh9401iw/gG1gYfvzjQIXcAAAAASUVORK5CYII= -")); +"), null, false); resxWithEmbeddedBitmap.ShouldHaveSingleItem(); resxWithEmbeddedBitmap[0].ShouldBeOfType(typeof(TypeConverterByteArrayResource)); @@ -228,7 +230,7 @@ public void TypeConverterStringWellFormatted() Blue -")); +"), null, false); resxWithEmbeddedBitmap.ShouldHaveSingleItem(); resxWithEmbeddedBitmap[0].ShouldBeOfType(typeof(TypeConverterStringResource)); @@ -252,7 +254,7 @@ public void TypeConverterStringDirectValue() ResXHelper.SurroundWithBoilerplate( @" Blue -")); +"), null, false); resxWithEmbeddedBitmap.ShouldHaveSingleItem(); resxWithEmbeddedBitmap[0].ShouldBeOfType(typeof(TypeConverterStringResource)); @@ -272,7 +274,7 @@ public void ResXFileRefToBitmap() $@" {bitmapPath};System.Drawing.Bitmap, System.Drawing, Version=2.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a -")); +"), null, false); resxWithLinkedBitmap.ShouldHaveSingleItem(); resxWithLinkedBitmap[0].ShouldBeOfType(typeof(FileStreamResource)); @@ -301,7 +303,7 @@ public void ResXFileRefToMemoryStream(string typeNameInResx) $@" {linkedTextFile.Path};{typeNameInResx} -")); +"), null, false); var resource = resources.ShouldHaveSingleItem() .ShouldBeOfType(); @@ -321,7 +323,7 @@ public void AssemblyElementWithNoAliasInfersSimpleName() ResXHelper.SurroundWithBoilerplate( @" Blue -")); +"), null, false); resxWithEmbeddedBitmap.ShouldHaveSingleItem(); resxWithEmbeddedBitmap[0].ShouldBeOfType(typeof(TypeConverterStringResource)); diff --git a/src/Tasks.UnitTests/ResourceHandling/ResGenDependencies_Tests.cs b/src/Tasks.UnitTests/ResourceHandling/ResGenDependencies_Tests.cs index d8462b8017b..6538fef25df 100644 --- a/src/Tasks.UnitTests/ResourceHandling/ResGenDependencies_Tests.cs +++ b/src/Tasks.UnitTests/ResourceHandling/ResGenDependencies_Tests.cs @@ -40,7 +40,7 @@ public void DirtyCleanScenario(bool useMSBuildResXReader) cache.IsDirty.ShouldBeFalse(); // Getting a file that wasn't in the cache is a write operation. - cache.GetResXFileInfo(resx, useMSBuildResXReader); + cache.GetResXFileInfo(resx, useMSBuildResXReader, null, false); cache.IsDirty.ShouldBeTrue(); // Add linkedFiles to further test serialization and deserialization. @@ -72,7 +72,7 @@ public void DirtyCleanScenario(bool useMSBuildResXReader) resX2.linkedFiles[1].ShouldBe(resX.linkedFiles[1]); // Asking for a file that's in the cache should not dirty the cache. - cache2.GetResXFileInfo(resx, useMSBuildResXReader); + cache2.GetResXFileInfo(resx, useMSBuildResXReader, null, false); cache2.IsDirty.ShouldBeFalse(); // Changing UseSourcePath to false should dirty the cache. diff --git a/src/Tasks/GenerateResource.cs b/src/Tasks/GenerateResource.cs index fe22b568293..ac63dfc4ed9 100644 --- a/src/Tasks/GenerateResource.cs +++ b/src/Tasks/GenerateResource.cs @@ -274,6 +274,12 @@ public string StronglyTypedLanguage } } + // Indicates whether any BinaryFormatter use should lead to a warning. + public bool WarnOnBinaryFormatterUse + { + get; set; + } + /// /// Specifies the namespace to use for the generated class source for the /// strongly typed resource. If left blank, no namespace is used. @@ -808,7 +814,8 @@ public override bool Execute() StronglyTypedClassName, PublicClass, ExtractResWFiles, - OutputDirectory); + OutputDirectory, + WarnOnBinaryFormatterUse); this.StronglyTypedClassName = process.StronglyTypedClassName; // in case a default was chosen this.StronglyTypedFileName = process.StronglyTypedFilename; // in case a default was chosen @@ -1510,7 +1517,7 @@ private bool ShouldRebuildResgenOutputFile(string sourceFilePath, string outputF ResGenDependencies.ResXFile resxFileInfo; try { - resxFileInfo = _cache.GetResXFileInfo(sourceFilePath, UsePreserializedResources); + resxFileInfo = _cache.GetResXFileInfo(sourceFilePath, UsePreserializedResources, Log, WarnOnBinaryFormatterUse); } catch (Exception e) when (!ExceptionHandling.NotExpectedIoOrXmlException(e) || e is MSBuildResXException) { @@ -1971,7 +1978,7 @@ private bool DetermineWhetherSerializedObjectLoads(string data) { byte[] serializedData = ByteArrayFromBase64WrappedString(data); - BinaryFormatter binaryFormatter = new BinaryFormatter(); + BinaryFormatter binaryFormatter = new(); using (MemoryStream memoryStream = new MemoryStream(serializedData)) { @@ -2337,6 +2344,8 @@ internal bool StronglyTypedResourceSuccessfullyCreated /// private bool _useSourcePath = false; + private bool _logWarningForBinaryFormatter = false; + #endregion /// @@ -2357,7 +2366,8 @@ internal void Run( string classname, bool publicClass, bool extractingResWFiles, - string resWOutputDirectory) + string resWOutputDirectory, + bool logWarningForBinaryFormatter) { _logger = log; _assemblyFiles = assemblyFilesList; @@ -2376,6 +2386,7 @@ internal void Run( _resWOutputDirectory = resWOutputDirectory; _portableLibraryCacheInfo = new List(); _usePreserializedResources = usePreserializedResources; + _logWarningForBinaryFormatter = logWarningForBinaryFormatter; #if !FEATURE_ASSEMBLYLOADCONTEXT // If references were passed in, we will have to give the ResxResourceReader an object @@ -2980,7 +2991,7 @@ private void ReadResources(String filename, bool shouldUseSourcePath, String out } else { - foreach (IResource resource in MSBuildResXReader.GetResourcesFromFile(filename, shouldUseSourcePath)) + foreach (IResource resource in MSBuildResXReader.GetResourcesFromFile(filename, shouldUseSourcePath, _logger, _logWarningForBinaryFormatter)) { AddResource(reader, resource, filename, 0, 0); } diff --git a/src/Tasks/Microsoft.Common.CurrentVersion.targets b/src/Tasks/Microsoft.Common.CurrentVersion.targets index 69b7c661777..f9d0e509c2a 100644 --- a/src/Tasks/Microsoft.Common.CurrentVersion.targets +++ b/src/Tasks/Microsoft.Common.CurrentVersion.targets @@ -3328,8 +3328,10 @@ Copyright (C) Microsoft Corporation. All rights reserved. SdkToolsPath="$(ResgenToolPath)" ExecuteAsTool="$(ResGenExecuteAsTool)" EnvironmentVariables="$(ResGenEnvironment)" + WarnOnBinaryFormatterUse="$(GenerateResourceWarnOnBinaryFormatterUse)" MSBuildRuntime="$(GenerateResourceMSBuildRuntime)" - MSBuildArchitecture="$(GenerateResourceMSBuildArchitecture)"> + MSBuildArchitecture="$(GenerateResourceMSBuildArchitecture)" + > diff --git a/src/Tasks/ResGenDependencies.cs b/src/Tasks/ResGenDependencies.cs index ea0be01f3fa..2a3c042cff0 100644 --- a/src/Tasks/ResGenDependencies.cs +++ b/src/Tasks/ResGenDependencies.cs @@ -125,13 +125,13 @@ public override void Translate(ITranslator translator) translator.Translate(ref baseLinkedFileDirectory); } - internal ResXFile GetResXFileInfo(string resxFile, bool useMSBuildResXReader) + internal ResXFile GetResXFileInfo(string resxFile, bool useMSBuildResXReader, TaskLoggingHelper log, bool logWarningForBinaryFormatter) { // First, try to retrieve the resx information from our hashtable. if (!resXFiles.TryGetValue(resxFile, out ResXFile retVal)) { // Ok, the file wasn't there. Add it to our cache and return it to the caller. - retVal = AddResxFile(resxFile, useMSBuildResXReader); + retVal = AddResxFile(resxFile, useMSBuildResXReader, log, logWarningForBinaryFormatter); } else { @@ -141,19 +141,19 @@ internal ResXFile GetResXFileInfo(string resxFile, bool useMSBuildResXReader) { resXFiles.Remove(resxFile); _isDirty = true; - retVal = AddResxFile(resxFile, useMSBuildResXReader); + retVal = AddResxFile(resxFile, useMSBuildResXReader, log, logWarningForBinaryFormatter); } } return retVal; } - private ResXFile AddResxFile(string file, bool useMSBuildResXReader) + private ResXFile AddResxFile(string file, bool useMSBuildResXReader, TaskLoggingHelper log, bool logWarningForBinaryFormatter) { // This method adds a .resx file "file" to our .resx cache. The method causes the file // to be cracked for contained files. - var resxFile = new ResXFile(file, BaseLinkedFileDirectory, useMSBuildResXReader); + var resxFile = new ResXFile(file, BaseLinkedFileDirectory, useMSBuildResXReader, log, logWarningForBinaryFormatter); resXFiles.Add(file, resxFile); _isDirty = true; return resxFile; @@ -230,7 +230,7 @@ internal sealed class ResXFile : DependencyFile, ITranslatable internal string[] LinkedFiles => linkedFiles; - internal ResXFile(string filename, string baseLinkedFileDirectory, bool useMSBuildResXReader) : base(filename) + internal ResXFile(string filename, string baseLinkedFileDirectory, bool useMSBuildResXReader, TaskLoggingHelper log, bool logWarningForBinaryFormatter) : base(filename) { // Creates a new ResXFile object and populates the class member variables // by computing a list of linked files within the .resx that was passed in. @@ -239,7 +239,7 @@ internal ResXFile(string filename, string baseLinkedFileDirectory, bool useMSBui if (FileSystems.Default.FileExists(FileName)) { - linkedFiles = GetLinkedFiles(filename, baseLinkedFileDirectory, useMSBuildResXReader); + linkedFiles = GetLinkedFiles(filename, baseLinkedFileDirectory, useMSBuildResXReader, log, logWarningForBinaryFormatter); } } @@ -260,7 +260,7 @@ public void Translate(ITranslator translator) /// /// May be thrown if Resx is invalid. May contain XmlException. /// May be thrown if Resx is invalid - private static string[] GetLinkedFiles(string filename, string baseLinkedFileDirectory, bool useMSBuildResXReader) + private static string[] GetLinkedFiles(string filename, string baseLinkedFileDirectory, bool useMSBuildResXReader, TaskLoggingHelper log, bool logWarningForBinaryFormatter) { // This method finds all linked .resx files for the .resx file that is passed in. // filename is the filename of the .resx file that is to be examined. @@ -270,7 +270,7 @@ private static string[] GetLinkedFiles(string filename, string baseLinkedFileDir if (useMSBuildResXReader) { - foreach (IResource resource in MSBuildResXReader.GetResourcesFromFile(filename, pathsRelativeToBasePath: baseLinkedFileDirectory == null)) + foreach (IResource resource in MSBuildResXReader.GetResourcesFromFile(filename, pathsRelativeToBasePath: baseLinkedFileDirectory == null, log, logWarningForBinaryFormatter)) { if (resource is FileStreamResource linkedResource) { diff --git a/src/Tasks/ResourceHandling/MSBuildResXReader.cs b/src/Tasks/ResourceHandling/MSBuildResXReader.cs index 765a023a102..664927523f3 100644 --- a/src/Tasks/ResourceHandling/MSBuildResXReader.cs +++ b/src/Tasks/ResourceHandling/MSBuildResXReader.cs @@ -9,6 +9,7 @@ using System.Xml; using System.Xml.Linq; using Microsoft.Build.Shared; +using Microsoft.Build.Utilities; #nullable disable @@ -16,7 +17,7 @@ namespace Microsoft.Build.Tasks.ResourceHandling { internal class MSBuildResXReader { - public static IReadOnlyList ReadResources(Stream s, string filename, bool pathsRelativeToBasePath) + public static IReadOnlyList ReadResources(Stream s, string filename, bool pathsRelativeToBasePath, TaskLoggingHelper log, bool logWarningForBinaryFormatter) { var resources = new List(); var aliases = new Dictionary(); @@ -38,7 +39,7 @@ public static IReadOnlyList ReadResources(Stream s, string filename, case "resheader": break; case "data": - ParseData(filename, pathsRelativeToBasePath, resources, aliases, elem); + ParseData(filename, pathsRelativeToBasePath, resources, aliases, elem, log, logWarningForBinaryFormatter); break; } } @@ -101,7 +102,14 @@ private static string GetFullTypeNameFromAlias(string aliasedTypeName, Dictionar return aliasedTypeName; } - private static void ParseData(string resxFilename, bool pathsRelativeToBasePath, List resources, Dictionary aliases, XElement elem) + private static void ParseData( + string resxFilename, + bool pathsRelativeToBasePath, + List resources, + Dictionary aliases, + XElement elem, + TaskLoggingHelper log, + bool logWarningForBinaryFormatter) { string name = elem.Attribute("name").Value; string value; @@ -186,6 +194,12 @@ private static void ParseData(string resxFilename, bool pathsRelativeToBasePath, case BinSerializedObjectMimeType: case Beta2CompatSerializedObjectMimeType: case CompatBinSerializedObjectMimeType: + // Warn of BinaryFormatter exposure (SDK should turn this on by default in .NET 8+) + if (logWarningForBinaryFormatter) + { + log?.LogWarningWithCodeFromResources(null, resxFilename, ((IXmlLineInfo)elem).LineNumber, ((IXmlLineInfo)elem).LinePosition, 0, 0, "GenerateResource.BinaryFormatterUse", name, typename); + } + // BinaryFormatter from byte array byte[] binaryFormatterBytes = Convert.FromBase64String(value); @@ -284,19 +298,19 @@ internal static bool IsMemoryStream(string fileRefType) /// /// Extract s from a given file on disk. /// - public static IReadOnlyList GetResourcesFromFile(string filename, bool pathsRelativeToBasePath) + public static IReadOnlyList GetResourcesFromFile(string filename, bool pathsRelativeToBasePath, TaskLoggingHelper log, bool logWarningForBinaryFormatter) { using (var x = new FileStream(filename, FileMode.Open, FileAccess.Read, FileShare.Read)) { - return ReadResources(x, filename, pathsRelativeToBasePath); + return ReadResources(x, filename, pathsRelativeToBasePath, log, logWarningForBinaryFormatter); } } - public static IReadOnlyList GetResourcesFromString(string resxContent, string basePath = null, bool? useRelativePath = null) + public static IReadOnlyList GetResourcesFromString(string resxContent, TaskLoggingHelper log, bool logWarningForBinaryFormatter, string basePath = null, bool? useRelativePath = null) { using (var x = new MemoryStream(Encoding.UTF8.GetBytes(resxContent))) { - return ReadResources(x, basePath, useRelativePath.GetValueOrDefault(basePath != null)); + return ReadResources(x, basePath, useRelativePath.GetValueOrDefault(basePath != null), log, logWarningForBinaryFormatter); } } diff --git a/src/Tasks/Resources/Strings.resx b/src/Tasks/Resources/Strings.resx index 51638cae838..7ed24d01ee2 100644 --- a/src/Tasks/Resources/Strings.resx +++ b/src/Tasks/Resources/Strings.resx @@ -1161,6 +1161,11 @@ MSB3824: In order to build with .NET Core, resource inputs must be in .txt or .resx format. {StrBegin="MSB3824: "} + + MSB3825: Resource "{0}" of type "{1}" is deserialized via BinaryFormatter at runtime. BinaryFormatter is deprecated due to possible security risks and will be removed with .NET 9. If you wish to continue using it, set property "GenerateResourceWarnOnBinaryFormatterUse" to false. + More information: https://learn.microsoft.com/dotnet/standard/serialization/binaryformatter-security-guide + {StrBegin="MSB3825: "} +