Better exploit in test, throw when found

HebaruSan · Aug 17, 2024 · c6cd0f5 · c6cd0f5
1 parent dc4f2c6
commit c6cd0f5
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 35 deletions.
diff --git a/Core/Types/ModuleInstallDescriptor.cs b/Core/Types/ModuleInstallDescriptor.cs
@@ -534,10 +534,15 @@ internal string TransformOutputName(IGame game, string outputName, string instal
                 }
             }
 
+            if (outputName.Contains("/../") || outputName.EndsWith("/.."))
+            {
+                throw new BadInstallLocationKraken(
+                    string.Format(Properties.Resources.ModuleInstallDescriptorInvalidInstallPath,
+                                  outputName));
+            }
+
             // Return our snipped, normalised, and ready to go output filename!
-            return CKANPathUtils.NormalizePath(
-                Path.Combine(installDir, outputName)
-            );
+            return CKANPathUtils.NormalizePath(Path.Combine(installDir, outputName));
         }
 
         private string ShortestMatchingPrefix(string fullPath)

diff --git a/Tests/Core/ModuleInstallerTests.cs b/Tests/Core/ModuleInstallerTests.cs
@@ -227,21 +227,14 @@ public void No_Installable_Files()
 
             CkanModule bugged_mod = TestData.DogeCoinFlag_101_bugged_module();
 
-            Assert.Throws<BadMetadataKraken>(delegate
+            var exc = Assert.Throws<BadMetadataKraken>(delegate
                 {
                     ModuleInstaller.FindInstallableFiles(bugged_mod, TestData.DogeCoinFlagZip(), ksp.KSP);
                 });
 
-            try
-            {
-                ModuleInstaller.FindInstallableFiles(bugged_mod, TestData.DogeCoinFlagZip(), ksp.KSP);
-            }
-            catch (BadMetadataKraken ex)
-            {
-                // Make sure our module information is attached.
-                Assert.IsNotNull(ex.module);
-                Assert.AreEqual(bugged_mod.identifier, ex.module.identifier);
-            }
+            // Make sure our module information is attached.
+            Assert.IsNotNull(exc.module);
+            Assert.AreEqual(bugged_mod.identifier, exc.module.identifier);
         }
 
 #pragma warning disable 0414
@@ -824,39 +817,34 @@ public void AllowInstallsToScenarios()
         }
 
         [Test]
-        public void DetectsZipSlipVulnerability()
+        public void FindInstallableFiles_ZipSlip_Throws()
         {
+            // Arrange
             // Create a ZIP file with an entry that tries to exploit Zip Slip
             var zip = ZipFile.Create(new MemoryStream());
             zip.BeginUpdate();
-            zip.AddDirectory("Ships");
-            zip.Add(new ZipEntry("Ships/../../outside.txt") { Size = 0, CompressedSize = 0 });
+            zip.AddDirectory("AwesomeMod");
+            zip.Add(new ZipEntry("AwesomeMod/../../../outside.txt") { Size = 0, CompressedSize = 0 });
             zip.CommitUpdate();
-
+            // Create a mod that would install the top folder of that path
             var mod = CkanModule.FromJson(@"
                 {
                     ""spec_version"": 1,
                     ""identifier"": ""AwesomeMod"",
                     ""version"": ""1.0.0"",
-                    ""download"": ""https://awesomemod.example/AwesomeMod.zip"",
-                    ""install"": [
-                        {
-                            ""file"": ""Ships/../../outside.txt"",
-                            ""install_to"": ""Ships""
-                        }
-                    ]
+                    ""download"": ""https://awesomemod.example/AwesomeMod.zip""
                 }");
 
-            // Act
-            List<InstallableFile> results;
-            using (var ksp = new DisposableKSP())
-            {
-                results = mod.install.First().FindInstallableFiles(zip, ksp.KSP);
-            }
-
-            // Assert
-            // The result should be empty or should have an appropriate exception if the Zip Slip is detected
-            Assert.That(results, Is.Empty, "The ZIP file should not contain any installable files due to Zip Slip vulnerability.");
+            // Act / Assert
+            Assert.Throws<BadInstallLocationKraken>(
+                delegate
+                {
+                    using (var ksp = new DisposableKSP())
+                    {
+                        var contents = ModuleInstaller.FindInstallableFiles(mod, zip, ksp.KSP);
+                    }
+                },
+                "Kraken should be thrown if ZIP file attempts to exploit Zip Slip vulnerability");
         }
 
         [Test]