Skip to content

Commit 038f9ba

Browse files
author
Nathan Mytelka
committed
Merged PR 413718: Prep work for 17.2
On Linux, the default /tmp folder is shared across all users and accessible by them. There are some cases in which we put sensitive information in temp and assume it's fine because on Windows, it is. This doesn't actually fix that assumption, since we're currently waiting for a new API that will be introduced in .NET 7 that will make a folder with appropriate permissions. However, this PR changes all the issues Eric Erhardt identified such that they go through a single code path, so to fix the security issue afterwards just requires changing the one place in our code. It did occur to me that we may not be able to use that API, in which case I can just write something to make a folder with a random name under temp then tweak its permissions.
2 parents d5f5823 + 6b118f4 commit 038f9ba

31 files changed

+179
-82
lines changed

‎eng/Versions.props‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
<!-- Copyright (c) .NET Foundation and contributors. All rights reserved. Licensed under the MIT license. See License.txt in the project root for full license information. -->
33
<Project>
44
<PropertyGroup>
5-
<VersionPrefix>17.2.1</VersionPrefix><DotNetFinalVersionKind>release</DotNetFinalVersionKind>
5+
<VersionPrefix>17.2.2</VersionPrefix><DotNetFinalVersionKind>release</DotNetFinalVersionKind>
66
<AssemblyVersion>15.1.0.0</AssemblyVersion>
77
<PreReleaseVersionLabel>preview</PreReleaseVersionLabel>
88
<DotNetUseShippingVersions>true</DotNetUseShippingVersions>

‎src/Build.OM.UnitTests/Definition/Project_Tests.cs‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -654,9 +654,7 @@ public void TransformsUseCorrectDirectory_Basic()
654654
project.ReevaluateIfNecessary();
655655

656656
project.GetItems("BuiltProjectOutputGroupKeyOutput").First().EvaluatedInclude.ShouldBe(
657-
NativeMethodsShared.IsWindows
658-
? Path.Combine(Path.GetTempPath(), @"obj\i386\foo.dll")
659-
: Path.Combine(Path.GetTempPath(), @"obj/i386/foo.dll"));
657+
Path.Combine(FileUtilities.TempFileDirectory, "obj", "i386", "foo.dll"));
660658
}
661659
finally
662660
{
@@ -721,8 +719,8 @@ public void TransformsUseCorrectDirectory_DirectoryTransform()
721719
Project project = new Project(xml);
722720
ProjectInstance projectInstance = new ProjectInstance(xml);
723721

724-
project.GetItems("BuiltProjectOutputGroupKeyOutput").First().EvaluatedInclude.ShouldBe(Path.Combine(Path.GetTempPath(), "obj", "i386").Substring(RootPrefixLength) + Path.DirectorySeparatorChar);
725-
projectInstance.GetItems("BuiltProjectOutputGroupKeyOutput").First().EvaluatedInclude.ShouldBe(Path.Combine(Path.GetTempPath(), "obj", "i386").Substring(RootPrefixLength) + Path.DirectorySeparatorChar);
722+
project.GetItems("BuiltProjectOutputGroupKeyOutput").First().EvaluatedInclude.ShouldBe(Path.Combine(FileUtilities.TempFileDirectory, "obj", "i386").Substring(RootPrefixLength) + Path.DirectorySeparatorChar);
723+
projectInstance.GetItems("BuiltProjectOutputGroupKeyOutput").First().EvaluatedInclude.ShouldBe(Path.Combine(FileUtilities.TempFileDirectory, "obj", "i386").Substring(RootPrefixLength) + Path.DirectorySeparatorChar);
726724
}
727725
finally
728726
{
@@ -756,8 +754,8 @@ public void TransformsUseCorrectDirectory_DirectoryItemFunction()
756754
Project project = new Project(xml);
757755
ProjectInstance projectInstance = new ProjectInstance(xml);
758756

759-
project.GetItems("BuiltProjectOutputGroupKeyOutput").First().EvaluatedInclude.ShouldBe(Path.Combine(Path.GetTempPath(), "obj", "i386").Substring(RootPrefixLength) + Path.DirectorySeparatorChar);
760-
projectInstance.GetItems("BuiltProjectOutputGroupKeyOutput").First().EvaluatedInclude.ShouldBe(Path.Combine(Path.GetTempPath(), "obj", "i386").Substring(RootPrefixLength) + Path.DirectorySeparatorChar);
757+
project.GetItems("BuiltProjectOutputGroupKeyOutput").First().EvaluatedInclude.ShouldBe(Path.Combine(FileUtilities.TempFileDirectory, "obj", "i386").Substring(RootPrefixLength) + Path.DirectorySeparatorChar);
758+
projectInstance.GetItems("BuiltProjectOutputGroupKeyOutput").First().EvaluatedInclude.ShouldBe(Path.Combine(FileUtilities.TempFileDirectory, "obj", "i386").Substring(RootPrefixLength) + Path.DirectorySeparatorChar);
761759
}
762760
finally
763761
{
@@ -794,8 +792,8 @@ public void TransformsUseCorrectDirectory_DirectoryNameItemFunction()
794792
ProjectInstance projectInstance = new ProjectInstance(xml);
795793

796794
// Should be the full path to the directory
797-
project.GetItems("BuiltProjectOutputGroupKeyOutput").First().EvaluatedInclude.ShouldBe(Path.Combine(Path.GetTempPath() /* remove c:\ */, "obj" + Path.DirectorySeparatorChar + "i386"));
798-
projectInstance.GetItems("BuiltProjectOutputGroupKeyOutput").First().EvaluatedInclude.ShouldBe(Path.Combine(Path.GetTempPath() /* remove c:\ */, "obj" + Path.DirectorySeparatorChar + "i386"));
795+
project.GetItems("BuiltProjectOutputGroupKeyOutput").First().EvaluatedInclude.ShouldBe(Path.Combine(FileUtilities.TempFileDirectory /* remove c:\ */, "obj" + Path.DirectorySeparatorChar + "i386"));
796+
projectInstance.GetItems("BuiltProjectOutputGroupKeyOutput").First().EvaluatedInclude.ShouldBe(Path.Combine(FileUtilities.TempFileDirectory /* remove c:\ */, "obj" + Path.DirectorySeparatorChar + "i386"));
799797
}
800798
finally
801799
{

‎src/Build.UnitTests/BackEnd/BuildEventArgTransportSink_Tests.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ void TransportDelegate(INodePacket packet)
129129
[SkipOnTargetFramework(TargetFrameworkMonikers.Netcoreapp, "https://github.com/dotnet/msbuild/issues/282")]
130130
public void TestShutDown()
131131
{
132-
SendDataDelegate transportDelegate = PacketProcessor;
132+
SendDataDelegate transportDelegate = new(PacketProcessor);
133133
var weakTransportDelegateReference = new WeakReference(transportDelegate);
134134
var transportSink = new BuildEventArgTransportSink(transportDelegate);
135135

‎src/Build.UnitTests/BackEnd/BuildRequestConfiguration_Tests.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -476,6 +476,7 @@ public void TestCache2()
476476
Environment.SetEnvironmentVariable("TEMP", problematicTmpPath);
477477

478478
FileUtilities.ClearCacheDirectoryPath();
479+
FileUtilities.ClearTempFileDirectory();
479480
string cacheFilePath = configuration.GetCacheFile();
480481
Assert.StartsWith(problematicTmpPath, cacheFilePath);
481482
}
@@ -484,6 +485,7 @@ public void TestCache2()
484485
Environment.SetEnvironmentVariable("TMP", originalTmp);
485486
Environment.SetEnvironmentVariable("TEMP", originalTemp);
486487
FileUtilities.ClearCacheDirectoryPath();
488+
FileUtilities.ClearTempFileDirectory();
487489
}
488490
}
489491

‎src/Build.UnitTests/BackEnd/DebugUtils_tests.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ public void DumpExceptionToFileShouldWriteInTempPathByDefault()
2424
try
2525
{
2626
ExceptionHandling.DumpExceptionToFile(new Exception("hello world"));
27-
exceptionFiles = Directory.GetFiles(Path.GetTempPath(), "MSBuild_*failure.txt");
27+
exceptionFiles = Directory.GetFiles(FileUtilities.TempFileDirectory, "MSBuild_*failure.txt");
2828
}
2929
finally
3030
{

‎src/Build.UnitTests/BackEnd/TargetUpToDateChecker_Tests.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -978,7 +978,7 @@ private void SimpleSymlinkInputCheck(DateTime symlinkWriteTime, DateTime targetW
978978
_testOutputHelper.WriteLine($"Created input file {inputTarget}");
979979
File.SetLastWriteTime(inputTarget, targetWriteTime);
980980

981-
inputSymlink = FileUtilities.GetTemporaryFile(null, ".linkin", createFile: false);
981+
inputSymlink = FileUtilities.GetTemporaryFile(null, null, ".linkin", createFile: false);
982982

983983
if (!CreateSymbolicLink(inputSymlink, inputTarget, 0))
984984
{

‎src/Build.UnitTests/Construction/SolutionFile_Tests.cs‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ public void ParseFirstProjectLine_InvalidProject()
141141
[Fact]
142142
public void ParseEtpProject()
143143
{
144-
string proj1Path = Path.Combine(Path.GetTempPath(), "someproj.etp");
144+
string proj1Path = Path.Combine(FileUtilities.TempFileDirectory, "someproj.etp");
145145
try
146146
{
147147
// Create the first .etp project file
@@ -192,8 +192,8 @@ public void ParseEtpProject()
192192
[Fact]
193193
public void CanBeMSBuildFile()
194194
{
195-
string proj1Path = Path.Combine(Path.GetTempPath(), "someproj.etp");
196-
string proj2Path = Path.Combine(Path.GetTempPath(), "someproja.proj");
195+
string proj1Path = Path.Combine(FileUtilities.TempFileDirectory, "someproj.etp");
196+
string proj2Path = Path.Combine(FileUtilities.TempFileDirectory, "someproja.proj");
197197
try
198198
{
199199
// Create the first .etp project file
@@ -317,8 +317,8 @@ public void CanBeMSBuildFileRejectsMSBuildLikeFiles()
317317
[Fact]
318318
public void ParseNestedEtpProjectSingleLevel()
319319
{
320-
string proj1Path = Path.Combine(Path.GetTempPath(), "someproj.etp");
321-
string proj2Path = Path.Combine(Path.GetTempPath(), "someproj2.etp");
320+
string proj1Path = Path.Combine(FileUtilities.TempFileDirectory, "someproj.etp");
321+
string proj2Path = Path.Combine(FileUtilities.TempFileDirectory, "someproj2.etp");
322322
try
323323
{
324324
// Create the first .etp project file
@@ -513,9 +513,9 @@ public void TestVSAndSolutionVersionParsing()
513513
[Trait("Category", "netcore-linux-failing")]
514514
public void ParseNestedEtpProjectMultipleLevel()
515515
{
516-
string proj1Path = Path.Combine(Path.GetTempPath(), "someproj.etp");
517-
string proj2Path = Path.Combine(Path.GetTempPath(), "someproj2.etp");
518-
string proj3Path = Path.Combine(Path.GetTempPath(), "ETPProjUpgradeTest", "someproj3.etp");
516+
string proj1Path = Path.Combine(FileUtilities.TempFileDirectory, "someproj.etp");
517+
string proj2Path = Path.Combine(FileUtilities.TempFileDirectory, "someproj2.etp");
518+
string proj3Path = Path.Combine(FileUtilities.TempFileDirectory, "ETPProjUpgradeTest", "someproj3.etp");
519519
try
520520
{
521521
// Create the first .etp project file
@@ -567,7 +567,7 @@ public void ParseNestedEtpProjectMultipleLevel()
567567
</GENERAL>
568568
</EFPROJECT>";
569569
// Create the directory for the third project
570-
Directory.CreateDirectory(Path.Combine(Path.GetTempPath(), "ETPProjUpgradeTest"));
570+
Directory.CreateDirectory(Path.Combine(FileUtilities.TempFileDirectory, "ETPProjUpgradeTest"));
571571
File.WriteAllText(proj3Path, etpProjContent);
572572

573573
// Create the SolutionFile object
@@ -602,7 +602,7 @@ public void ParseNestedEtpProjectMultipleLevel()
602602
[Fact]
603603
public void MalformedEtpProjFile()
604604
{
605-
string proj1Path = Path.Combine(Path.GetTempPath(), "someproj.etp");
605+
string proj1Path = Path.Combine(FileUtilities.TempFileDirectory, "someproj.etp");
606606
try
607607
{
608608
// Create the .etp project file

‎src/Build.UnitTests/Construction/SolutionProjectGenerator_Tests.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -695,7 +695,7 @@ public void SolutionConfigurationWithDependencies()
695695
<ProjectConfiguration Project=`{{786E302A-96CE-43DC-B640-D6B6CC9BF6C0}}` AbsolutePath=`##temp##{Path.Combine("Project1", "A.csproj")}` BuildProjectInSolution=`True`>Debug|AnyCPU</ProjectConfiguration>
696696
<ProjectConfiguration Project=`{{881C1674-4ECA-451D-85B6-D7C59B7F16FA}}` AbsolutePath=`##temp##{Path.Combine("Project2", "B.csproj")}` BuildProjectInSolution=`True`>Debug|AnyCPU<ProjectDependency Project=`{{4A727FF8-65F2-401E-95AD-7C8BBFBE3167}}` /></ProjectConfiguration>
697697
<ProjectConfiguration Project=`{{4A727FF8-65F2-401E-95AD-7C8BBFBE3167}}` AbsolutePath=`##temp##{Path.Combine("Project3", "C.csproj")}` BuildProjectInSolution=`True`>Debug|AnyCPU</ProjectConfiguration>
698-
</SolutionConfiguration>".Replace("`", "\"").Replace("##temp##", Path.GetTempPath());
698+
</SolutionConfiguration>".Replace("`", "\"").Replace("##temp##", FileUtilities.TempFileDirectory);
699699

700700
Helpers.VerifyAssertLineByLine(expected, solutionConfigurationContents);
701701
}
@@ -935,14 +935,14 @@ public void TestAddPropertyGroupForSolutionConfiguration()
935935
msbuildProject.ReevaluateIfNecessary();
936936

937937
string solutionConfigurationContents = msbuildProject.GetPropertyValue("CurrentSolutionConfigurationContents");
938-
string tempProjectPath = Path.Combine(Path.GetTempPath(), "ClassLibrary1", "ClassLibrary1.csproj");
938+
string tempProjectPath = Path.Combine(FileUtilities.TempFileDirectory, "ClassLibrary1", "ClassLibrary1.csproj");
939939

940940
Assert.Contains("{6185CC21-BE89-448A-B3C0-D1C27112E595}", solutionConfigurationContents);
941941
tempProjectPath = Path.GetFullPath(tempProjectPath);
942942
Assert.True(solutionConfigurationContents.IndexOf(tempProjectPath, StringComparison.OrdinalIgnoreCase) > 0);
943943
Assert.Contains("CSConfig1|AnyCPU", solutionConfigurationContents);
944944

945-
tempProjectPath = Path.Combine(Path.GetTempPath(), "MainApp", "MainApp.vcxproj");
945+
tempProjectPath = Path.Combine(FileUtilities.TempFileDirectory, "MainApp", "MainApp.vcxproj");
946946
tempProjectPath = Path.GetFullPath(tempProjectPath);
947947
Assert.Contains("{A6F99D27-47B9-4EA4-BFC9-25157CBDC281}", solutionConfigurationContents);
948948
Assert.True(solutionConfigurationContents.IndexOf(tempProjectPath, StringComparison.OrdinalIgnoreCase) > 0);

‎src/Build.UnitTests/Evaluation/Expander_Tests.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3040,7 +3040,7 @@ public void PropertyFunctionStaticMethodEnumArgument()
30403040
[Fact]
30413041
public void PropertyFunctionStaticMethodDirectoryNameOfFileAbove()
30423042
{
3043-
string tempPath = Path.GetTempPath();
3043+
string tempPath = FileUtilities.TempFileDirectory;
30443044
string tempFile = Path.GetFileName(FileUtilities.GetTemporaryFile());
30453045

30463046
try
@@ -3078,7 +3078,7 @@ public void PropertyFunctionStaticMethodGetPathOfFileAbove()
30783078
//
30793079
MockElementLocation mockElementLocation = new MockElementLocation(Path.Combine(ObjectModelHelpers.TempProjectDir, "one", "two", "three", "four", "five", Path.GetRandomFileName()));
30803080

3081-
string fileToFind = FileUtilities.GetTemporaryFile(ObjectModelHelpers.TempProjectDir, ".tmp");
3081+
string fileToFind = FileUtilities.GetTemporaryFile(ObjectModelHelpers.TempProjectDir, null, ".tmp");
30823082

30833083
try
30843084
{

‎src/Build/BackEnd/BuildManager/BuildManager.cs‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -398,6 +398,14 @@ public DeferredBuildMessage(string text, MessageImportance importance)
398398
/// <exception cref="InvalidOperationException">Thrown if a build is already in progress.</exception>
399399
public void BeginBuild(BuildParameters parameters, IEnumerable<DeferredBuildMessage> deferredBuildMessages)
400400
{
401+
// TEMP can be modified from the environment. Most of Traits is lasts for the duration of the process (with a manual reset for tests)
402+
// and environment variables we use as properties are stored in a dictionary at the beginning of the build, so they also cannot be
403+
// changed during a build. Some of our older stuff uses live environment variable checks. The TEMP directory previously used a live
404+
// environment variable check, but it now uses a cached value. Nevertheless, we should support changing it between builds, so reset
405+
// it here in case the user is using Visual Studio or the MSBuild server, as those each last for multiple builds without changing
406+
// BuildManager.
407+
FileUtilities.ClearTempFileDirectory();
408+
401409
// deferredBuildMessages cannot be an optional parameter on a single BeginBuild method because it would break binary compatibility.
402410
_deferredBuildMessages = deferredBuildMessages;
403411
BeginBuild(parameters);

0 commit comments

Comments
 (0)