codeQL issues (#4948)

Fix codeQL issues
dotnet · Sep 19, 2024 · aafed67 · aafed67
1 parent 551e68d
commit aafed67
Show file tree

Hide file tree

Showing 4 changed files with 13 additions and 5 deletions.
diff --git a/src/Microsoft.Diagnostics.TestHelpers/AcquireDotNetTestStep.cs b/src/Microsoft.Diagnostics.TestHelpers/AcquireDotNetTestStep.cs
@@ -168,7 +168,12 @@ private static async Task Unzip(string zipPath, string expandedDirPath, ITestOut
                 ZipArchive zip = new(zipStream);
                 foreach (ZipArchiveEntry entry in zip.Entries)
                 {
-                    string extractedFilePath = Path.Combine(expandedDirPath, entry.FullName);
+                    string extractedFilePath = Path.GetFullPath(Path.Combine(expandedDirPath, entry.FullName));
+                    string fullExtractedDirPath = Path.GetFullPath(expandedDirPath + Path.DirectorySeparatorChar);
+                    if (!extractedFilePath.StartsWith(fullExtractedDirPath))
+                    {
+                        throw new InvalidDataException("Entry is outside of the target dir: " + entry.FullName);
+                    }
                     Directory.CreateDirectory(Path.GetDirectoryName(extractedFilePath));
                     using (Stream zipFileStream = entry.Open())
                     {

diff --git a/src/Microsoft.SymbolStore/KeyGenerators/SourceFileKeyGenerator.cs b/src/Microsoft.SymbolStore/KeyGenerators/SourceFileKeyGenerator.cs
@@ -28,6 +28,7 @@ public override IEnumerable<SymbolStoreKey> GetKeys(KeyTypeFlags flags)
             if ((flags & KeyTypeFlags.IdentityKey) != 0)
             {
 #pragma warning disable CA5350 // Do Not Use Weak Cryptographic Algorithms
+                // CodeQL [SM02196] SSQP protocol requires the use of SHA1 and this doesn't constitute a security boundary.
                 byte[] hash = SHA1.Create().ComputeHash(_file.Stream);
 #pragma warning restore CA5350 // Do Not Use Weak Cryptographic Algorithms
                 yield return GetKey(_file.FileName, hash);

diff --git a/src/SOS/Strike/clrma/exception.cpp b/src/SOS/Strike/clrma/exception.cpp
@@ -324,7 +324,8 @@ ClrmaException::Frame(
     *pDisplacement = 0;
 
     UINT nCount = 0;
-    if (HRESULT hr = get_FrameCount(&nCount))
+    HRESULT hr;
+    if (FAILED(hr = get_FrameCount(&nCount)))
     {
         return hr;
     }
@@ -411,7 +412,7 @@ ClrmaException::InnerException(
 
     HRESULT hr;
     USHORT nCount = 0;
-    if (hr = get_InnerExceptionCount(&nCount))
+    if (FAILED(hr = get_InnerExceptionCount(&nCount)))
     {
         return hr;
     }
@@ -453,7 +454,7 @@ ClrmaException::GetStackFrames()
         TraceError("ClrmaException::GetStackFrames GetObjectData(%016llx) FAILED %08x\n", m_exceptionData.StackTrace, hr);
         return hr;
     }
-        
+
     if (arrayObjData.ObjectType != OBJ_ARRAY || arrayObjData.dwNumComponents == 0)
     {
         TraceError("ClrmaException::GetStackFrames StackTrace not array or empty\n");

diff --git a/src/SOS/Strike/clrma/thread.cpp b/src/SOS/Strike/clrma/thread.cpp
@@ -269,7 +269,8 @@ ClrmaThread::Frame(
     *pDisplacement = 0;
 
     UINT nCount = 0;
-    if (HRESULT hr = get_FrameCount(&nCount))
+    HRESULT hr;
+    if (FAILED(hr= get_FrameCount(&nCount)))
     {
         return hr;
     }