diff --git a/modules/swagger-codegen/src/main/java/io/swagger/codegen/AbstractGenerator.java b/modules/swagger-codegen/src/main/java/io/swagger/codegen/AbstractGenerator.java index 234cdaeef45..5d2cf9c5a59 100644 --- a/modules/swagger-codegen/src/main/java/io/swagger/codegen/AbstractGenerator.java +++ b/modules/swagger-codegen/src/main/java/io/swagger/codegen/AbstractGenerator.java @@ -13,6 +13,7 @@ import java.util.Scanner; import java.util.regex.Pattern; +import io.swagger.codegen.utils.SecureFileUtils; import org.apache.commons.lang3.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -21,11 +22,13 @@ public abstract class AbstractGenerator { private static final Logger LOGGER = LoggerFactory.getLogger(AbstractGenerator.class); @SuppressWarnings("static-method") - public File writeToFile(String filename, String contents) throws IOException { + public File writeToFile(String filename, String contents) throws IOException, SecurityException { LOGGER.info("writing file " + filename); + SecureFileUtils.validatePath(filename); File output = new File(filename); if (output.getParent() != null && !new File(output.getParent()).exists()) { + SecureFileUtils.validatePath(output.getParent()); File parent = new File(output.getParent()); parent.mkdirs(); } diff --git a/modules/swagger-codegen/src/main/java/io/swagger/codegen/DefaultCodegen.java b/modules/swagger-codegen/src/main/java/io/swagger/codegen/DefaultCodegen.java index b4fd03124ae..c43f252f925 100644 --- a/modules/swagger-codegen/src/main/java/io/swagger/codegen/DefaultCodegen.java +++ b/modules/swagger-codegen/src/main/java/io/swagger/codegen/DefaultCodegen.java @@ -14,6 +14,7 @@ import com.samskivert.mustache.Mustache; import com.samskivert.mustache.Template; +import io.swagger.codegen.utils.SecureFileUtils; import io.swagger.models.properties.UntypedProperty; import org.apache.commons.lang3.ObjectUtils; import org.apache.commons.lang3.StringEscapeUtils; @@ -3562,6 +3563,11 @@ public String apiTestFilename(String templateName, String tag) { } public boolean shouldOverwrite(String filename) { + try { + SecureFileUtils.validatePath(filename); + } catch (SecurityException e) { + return false; + } return !(skipOverwrite && new File(filename).exists()); } @@ -3825,10 +3831,13 @@ public void writeOptional(String outputFolder, SupportingFile supportingFile) { else { folder = supportingFile.destinationFilename; } - if(!new File(folder).exists()) { + try { + SecureFileUtils.validatePath(folder); + if (!new File(folder).exists()) { + LOGGER.info("Skipped overwriting " + supportingFile.destinationFilename + " as the file already exists in " + folder); + } + } catch (Exception e) { supportingFiles.add(supportingFile); - } else { - LOGGER.info("Skipped overwriting " + supportingFile.destinationFilename + " as the file already exists in " + folder); } } diff --git a/modules/swagger-codegen/src/main/java/io/swagger/codegen/DefaultGenerator.java b/modules/swagger-codegen/src/main/java/io/swagger/codegen/DefaultGenerator.java index 13435e74d65..0c5f1a9a282 100644 --- a/modules/swagger-codegen/src/main/java/io/swagger/codegen/DefaultGenerator.java +++ b/modules/swagger-codegen/src/main/java/io/swagger/codegen/DefaultGenerator.java @@ -3,8 +3,8 @@ import com.samskivert.mustache.Mustache; import com.samskivert.mustache.Template; import io.swagger.codegen.ignore.CodegenIgnoreProcessor; -import io.swagger.codegen.languages.AbstractJavaCodegen; import io.swagger.codegen.utils.ImplementationVersion; +import io.swagger.codegen.utils.SecureFileUtils; import io.swagger.models.*; import io.swagger.models.auth.OAuth2Definition; import io.swagger.models.auth.SecuritySchemeDefinition; @@ -597,6 +597,7 @@ protected void generateSupportingFiles(List files, Map bun if (StringUtils.isNotEmpty(support.folder)) { outputFolder += File.separator + support.folder; } + SecureFileUtils.validatePath(outputFolder); File of = new File(outputFolder); if (!of.isDirectory()) { of.mkdirs(); @@ -671,6 +672,7 @@ public Reader getTemplate(String name) { // Output .swagger-codegen-ignore if it doesn't exist and wasn't explicitly created by a generator final String swaggerCodegenIgnore = ".swagger-codegen-ignore"; String ignoreFileNameTarget = config.outputFolder() + File.separator + swaggerCodegenIgnore; + SecureFileUtils.validatePath(ignoreFileNameTarget); File ignoreFile = new File(ignoreFileNameTarget); if (isGenerateSwaggerMetadata && !ignoreFile.exists()) { String ignoreFileNameSource = File.separator + config.getCommonTemplateDir() + File.separator + swaggerCodegenIgnore; @@ -785,6 +787,7 @@ public List generate() { protected File processTemplateToFile(Map templateData, String templateName, String outputFilename) throws IOException { String adjustedOutputFilename = outputFilename.replaceAll("//", "/").replace('/', File.separatorChar); + SecureFileUtils.validatePath(adjustedOutputFilename); if (ignoreProcessor.allowsFile(new File(adjustedOutputFilename))) { String templateFile = getFullTemplateFile(config, templateName); String template = readTemplate(templateFile); @@ -801,6 +804,7 @@ public Reader getTemplate(String name) { .compile(template); writeToFile(adjustedOutputFilename, tmpl.execute(templateData)); + SecureFileUtils.validatePath(adjustedOutputFilename); return new File(adjustedOutputFilename); } diff --git a/modules/swagger-codegen/src/main/java/io/swagger/codegen/ignore/CodegenIgnoreProcessor.java b/modules/swagger-codegen/src/main/java/io/swagger/codegen/ignore/CodegenIgnoreProcessor.java index 76179951d96..12014ba46e2 100644 --- a/modules/swagger-codegen/src/main/java/io/swagger/codegen/ignore/CodegenIgnoreProcessor.java +++ b/modules/swagger-codegen/src/main/java/io/swagger/codegen/ignore/CodegenIgnoreProcessor.java @@ -3,6 +3,7 @@ import com.google.common.collect.ImmutableList; import io.swagger.codegen.ignore.rules.DirectoryRule; import io.swagger.codegen.ignore.rules.Rule; +import io.swagger.codegen.utils.SecureFileUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -40,12 +41,19 @@ public CodegenIgnoreProcessor(final String baseDirectory) { */ @SuppressWarnings("WeakerAccess") public CodegenIgnoreProcessor(final String baseDirectory, final String ignoreFile) { - final File directory = new File(baseDirectory); - final File targetIgnoreFile = new File(directory, ignoreFile); - if (directory.exists() && directory.isDirectory()) { - loadFromFile(targetIgnoreFile); - } else { - LOGGER.warn("Output directory does not exist, or is inaccessible. No file (.swagger-codegen-ignore) will be evaluated."); + try { + SecureFileUtils.validatePath(baseDirectory); + final File directory = new File(baseDirectory); + SecureFileUtils.validatePath(directory); + SecureFileUtils.validatePath(ignoreFile); + final File targetIgnoreFile = new File(directory, ignoreFile); + if (directory.exists() && directory.isDirectory()) { + loadFromFile(targetIgnoreFile); + } else { + LOGGER.warn("Output directory does not exist, or is inaccessible. No file (.swagger-codegen-ignore) will be evaluated."); + } + } catch (SecurityException e) { + LOGGER.error("Security violation: attempted to access unsafe ignore file path ", e); } } @@ -55,7 +63,12 @@ public CodegenIgnoreProcessor(final String baseDirectory, final String ignoreFil * @param targetIgnoreFile The ignore file location. */ public CodegenIgnoreProcessor(final File targetIgnoreFile) { - loadFromFile(targetIgnoreFile); + try { + SecureFileUtils.validatePath(targetIgnoreFile); + loadFromFile(targetIgnoreFile); + } catch (SecurityException e) { + LOGGER.error("Security violation: attempted to access unsafe ignore file path: " + targetIgnoreFile.getAbsolutePath(), e); + } } private void loadFromFile(File targetIgnoreFile) { diff --git a/modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/JavaJAXRSSpecServerCodegen.java b/modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/JavaJAXRSSpecServerCodegen.java index ff5b9be3f0b..d6cecb27dbb 100644 --- a/modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/JavaJAXRSSpecServerCodegen.java +++ b/modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/JavaJAXRSSpecServerCodegen.java @@ -7,6 +7,7 @@ import io.swagger.codegen.CodegenParameter; import io.swagger.codegen.CodegenProperty; import io.swagger.codegen.SupportingFile; +import io.swagger.codegen.utils.SecureFileUtils; import io.swagger.models.Operation; import io.swagger.models.Swagger; import io.swagger.models.parameters.Parameter; @@ -197,7 +198,11 @@ public void preprocessSwagger(Swagger swagger) { //copy input swagger to output folder try { String swaggerJson = Json.pretty(swagger); - FileUtils.writeStringToFile(new File(outputFolder + File.separator + "swagger.json"), swaggerJson, StandardCharsets.UTF_8); + SecureFileUtils.validatePath(outputFolder); + File outputFile = new File(outputFolder + File.separator + "swagger.json"); + FileUtils.writeStringToFile(outputFile, swaggerJson, StandardCharsets.UTF_8); + } catch (SecurityException e) { + throw new RuntimeException("Security violation: attempted to write to unsafe file path: " + outputFolder + File.separator + "swagger.json", e); } catch (IOException e) { throw new RuntimeException(e.getMessage(), e.getCause()); } diff --git a/modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/RubyClientCodegen.java b/modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/RubyClientCodegen.java index 0f2a44285bc..446acb0a205 100644 --- a/modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/RubyClientCodegen.java +++ b/modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/RubyClientCodegen.java @@ -9,6 +9,7 @@ import io.swagger.codegen.CodegenType; import io.swagger.codegen.DefaultCodegen; import io.swagger.codegen.SupportingFile; +import io.swagger.codegen.utils.SecureFileUtils; import io.swagger.models.Model; import io.swagger.models.Operation; import io.swagger.models.Swagger; @@ -741,7 +742,13 @@ public void setGemAuthorEmail(String gemAuthorEmail) { @Override public boolean shouldOverwrite(String filename) { // skip spec file as the file might have been updated with new test cases - return !(skipOverwrite && new File(filename).exists()); + try { + SecureFileUtils.validatePath(filename); + return !(skipOverwrite && new File(filename).exists()); + } catch (SecurityException e) { + LOGGER.warn("Security violation: attempted to check unsafe file path: " + filename, e); + return false; + } // //return super.shouldOverwrite(filename) && !filename.endsWith("_spec.rb"); } diff --git a/modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/SwaggerGenerator.java b/modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/SwaggerGenerator.java index 83b7c0443c1..25e89a42f1d 100644 --- a/modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/SwaggerGenerator.java +++ b/modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/SwaggerGenerator.java @@ -6,9 +6,8 @@ import java.util.Map; import io.swagger.codegen.CliOption; -import io.swagger.codegen.CodegenConstants; +import io.swagger.codegen.utils.SecureFileUtils; import io.swagger.models.Model; -import io.swagger.models.properties.Property; import org.apache.commons.io.FileUtils; import org.apache.commons.lang3.StringUtils; import org.slf4j.Logger; @@ -63,6 +62,7 @@ public void processSwagger(Swagger swagger) { try { String outputFile = outputFolder + File.separator + this.outputFile; + SecureFileUtils.validatePath(outputFile); FileUtils.writeStringToFile(new File(outputFile), swaggerString, StandardCharsets.UTF_8); LOGGER.debug("wrote file to " + outputFile); } catch (Exception e) { diff --git a/modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/SwaggerYamlGenerator.java b/modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/SwaggerYamlGenerator.java index 420119df6a9..bc494ef7f1f 100644 --- a/modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/SwaggerYamlGenerator.java +++ b/modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/SwaggerYamlGenerator.java @@ -8,6 +8,7 @@ import com.fasterxml.jackson.dataformat.yaml.YAMLFactory; import com.fasterxml.jackson.dataformat.yaml.YAMLGenerator; import io.swagger.codegen.*; +import io.swagger.codegen.utils.SecureFileUtils; import io.swagger.jackson.mixin.OperationResponseMixin; import io.swagger.jackson.mixin.ResponseSchemaMixin; import io.swagger.models.Model; @@ -104,6 +105,7 @@ public void processSwagger(Swagger swagger) { configureMapper(mapper); String swaggerString = mapper.writeValueAsString(swagger); String outputFile = outputFolder + File.separator + this.outputFile; + SecureFileUtils.validatePath(outputFile); FileUtils.writeStringToFile(new File(outputFile), swaggerString, StandardCharsets.UTF_8); LOGGER.debug("wrote file to " + outputFile); } catch (Exception e) { diff --git a/modules/swagger-codegen/src/main/java/io/swagger/codegen/utils/SecureFileUtils.java b/modules/swagger-codegen/src/main/java/io/swagger/codegen/utils/SecureFileUtils.java new file mode 100644 index 00000000000..cdc71330133 --- /dev/null +++ b/modules/swagger-codegen/src/main/java/io/swagger/codegen/utils/SecureFileUtils.java @@ -0,0 +1,46 @@ +package io.swagger.codegen.utils; + +import java.io.File; +import java.io.IOException; + + +/** + * Utility class for secure file operations that prevent path traversal attacks. + * Uses a simplified approach focusing on canonical path validation and allowlist-based security. + */ +public class SecureFileUtils { + + public static void validatePath(File file) throws SecurityException { + if (file == null) { + throw new IllegalArgumentException("File cannot be null"); + } + + try { + String absolutePath = file.getAbsolutePath(); + String canonicalPath = file.getCanonicalPath(); + + if (absolutePath.contains("..") || absolutePath.contains("\0")) { + throw new SecurityException("Path contains suspicious characters: " + absolutePath); + } + + if (canonicalPath.contains("..") || canonicalPath.contains("\0")) { + throw new SecurityException("Path contains suspicious characters: " + canonicalPath); + } + + } catch (IOException e) { + throw new SecurityException("Unable to resolve canonical path for: " + file.getAbsolutePath(), e); + } + } + + public static void validatePath(String path) throws SecurityException { + if (path == null || path.trim().isEmpty()) { + throw new IllegalArgumentException("Path cannot be null or empty"); + } + + if (path.contains("..") || path.contains("\0")) { + throw new SecurityException("Path contains suspicious characters: " + path); + } + + validatePath(new File(path)); + } +} diff --git a/modules/swagger-codegen/src/test/java/io/swagger/codegen/AbstractGeneratorTest.java b/modules/swagger-codegen/src/test/java/io/swagger/codegen/AbstractGeneratorTest.java new file mode 100644 index 00000000000..c2cb49a2ab0 --- /dev/null +++ b/modules/swagger-codegen/src/test/java/io/swagger/codegen/AbstractGeneratorTest.java @@ -0,0 +1,18 @@ +package io.swagger.codegen; + +import org.testng.annotations.Test; + +import java.io.IOException; + +public class AbstractGeneratorTest { + + + private static class TestableAbstractGenerator extends AbstractGenerator { + } + + @Test(expectedExceptions = SecurityException.class) + public void testWriteToFileWithPathTraversal() throws IOException { + TestableAbstractGenerator generator = new TestableAbstractGenerator(); + generator.writeToFile("../../../etc/passwd", "malicious content"); + } +} diff --git a/modules/swagger-codegen/src/test/java/io/swagger/codegen/DefaultCodegenTest.java b/modules/swagger-codegen/src/test/java/io/swagger/codegen/DefaultCodegenTest.java index be20a4d6649..4653677e663 100644 --- a/modules/swagger-codegen/src/test/java/io/swagger/codegen/DefaultCodegenTest.java +++ b/modules/swagger-codegen/src/test/java/io/swagger/codegen/DefaultCodegenTest.java @@ -33,4 +33,12 @@ public void testAdditionalPropertiesPutForConfigValues() throws Exception { Assert.assertEquals(codegen.additionalProperties().get(CodegenConstants.HIDE_GENERATION_TIMESTAMP), Boolean.FALSE); Assert.assertEquals(codegen.isHideGenerationTimestamp(), false); } + + @Test + public void testShouldOverwriteWithPathTraversal() { + DefaultCodegen codegen = new DefaultCodegen(); + boolean result = codegen.shouldOverwrite("../../../etc/passwd"); + + Assert.assertFalse(result, "shouldOverwrite should return false when SecurityException is thrown"); + } } diff --git a/modules/swagger-codegen/src/test/java/io/swagger/codegen/DefaultGeneratorTest.java b/modules/swagger-codegen/src/test/java/io/swagger/codegen/DefaultGeneratorTest.java index bd5b9b54b4e..822f5afe396 100644 --- a/modules/swagger-codegen/src/test/java/io/swagger/codegen/DefaultGeneratorTest.java +++ b/modules/swagger-codegen/src/test/java/io/swagger/codegen/DefaultGeneratorTest.java @@ -12,7 +12,6 @@ import org.apache.commons.io.FileUtils; import org.apache.commons.lang3.StringUtils; import org.junit.rules.TemporaryFolder; -import org.testng.Assert; import org.testng.annotations.AfterMethod; import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; @@ -45,6 +44,8 @@ public class DefaultGeneratorTest { public TemporaryFolder folder = new TemporaryFolder(); + private final DefaultGenerator generator = new DefaultGenerator();; + @BeforeMethod public void setUp() throws Exception { folder.create(); @@ -1134,4 +1135,9 @@ private static CodegenOperation findCodegenOperationByOperationId(Map(), "template.mustache", "../../../etc/passwd"); + } } diff --git a/modules/swagger-codegen/src/test/java/io/swagger/codegen/ignore/CodegenIgnoreProcessorSecurityTest.java b/modules/swagger-codegen/src/test/java/io/swagger/codegen/ignore/CodegenIgnoreProcessorSecurityTest.java new file mode 100644 index 00000000000..6169f7e8a08 --- /dev/null +++ b/modules/swagger-codegen/src/test/java/io/swagger/codegen/ignore/CodegenIgnoreProcessorSecurityTest.java @@ -0,0 +1,28 @@ +package io.swagger.codegen.ignore; + +import org.testng.Assert; +import org.testng.annotations.Test; + +import java.io.File; + +public class CodegenIgnoreProcessorSecurityTest { + + @Test + public void testConstructorWithPathTraversal() { + CodegenIgnoreProcessor processor = new CodegenIgnoreProcessor("../../../etc"); + + Assert.assertNotNull(processor, "Processor should be created despite security violation"); + Assert.assertTrue(processor.getExclusionRules().isEmpty(), "No exclusion rules should be loaded"); + Assert.assertTrue(processor.getInclusionRules().isEmpty(), "No inclusion rules should be loaded"); + } + + @Test + public void testFileConstructorWithPathTraversal() { + File maliciousFile = new File("../../../etc/passwd"); + CodegenIgnoreProcessor processor = new CodegenIgnoreProcessor(maliciousFile); + + Assert.assertNotNull(processor, "Processor should be created despite security violation"); + Assert.assertTrue(processor.getExclusionRules().isEmpty(), "No exclusion rules should be loaded"); + Assert.assertTrue(processor.getInclusionRules().isEmpty(), "No inclusion rules should be loaded"); + } +} diff --git a/modules/swagger-codegen/src/test/java/io/swagger/codegen/languages/RubyClientCodegenTest.java b/modules/swagger-codegen/src/test/java/io/swagger/codegen/languages/RubyClientCodegenTest.java new file mode 100644 index 00000000000..637ba0c6ca1 --- /dev/null +++ b/modules/swagger-codegen/src/test/java/io/swagger/codegen/languages/RubyClientCodegenTest.java @@ -0,0 +1,16 @@ +package io.swagger.codegen.languages; + +import org.testng.Assert; +import org.testng.annotations.Test; + +public class RubyClientCodegenTest { + + @Test + public void testShouldOverwriteWithPathTraversal() { + RubyClientCodegen codegen = new RubyClientCodegen(); + + boolean result = codegen.shouldOverwrite("../../../etc/passwd"); + + Assert.assertFalse(result, "shouldOverwrite should return false when SecurityException is thrown"); + } +} diff --git a/modules/swagger-codegen/src/test/java/io/swagger/codegen/languages/SwaggerGeneratorTest.java b/modules/swagger-codegen/src/test/java/io/swagger/codegen/languages/SwaggerGeneratorTest.java new file mode 100644 index 00000000000..a3f5be1e532 --- /dev/null +++ b/modules/swagger-codegen/src/test/java/io/swagger/codegen/languages/SwaggerGeneratorTest.java @@ -0,0 +1,19 @@ +package io.swagger.codegen.languages; + +import io.swagger.models.Swagger; +import io.swagger.models.Info; +import org.testng.annotations.Test; + +public class SwaggerGeneratorTest { + + @Test + public void testProcessSwaggerWithPathTraversal() { + SwaggerGenerator generator = new SwaggerGenerator(); + generator.setOutputFile("../../../etc/passwd"); + + Swagger swagger = new Swagger(); + swagger.setInfo(new Info().title("Test API").version("1.0.0")); + + generator.processSwagger(swagger); + } +} diff --git a/modules/swagger-codegen/src/test/java/io/swagger/codegen/languages/SwaggerYamlGeneratorTest.java b/modules/swagger-codegen/src/test/java/io/swagger/codegen/languages/SwaggerYamlGeneratorTest.java new file mode 100644 index 00000000000..7314151a1d0 --- /dev/null +++ b/modules/swagger-codegen/src/test/java/io/swagger/codegen/languages/SwaggerYamlGeneratorTest.java @@ -0,0 +1,25 @@ +package io.swagger.codegen.languages; + +import io.swagger.models.Swagger; +import io.swagger.models.Info; +import org.testng.Assert; +import org.testng.annotations.Test; + +public class SwaggerYamlGeneratorTest { + + @Test + public void testProcessSwaggerWithPathTraversal() { + SwaggerYamlGenerator generator = new SwaggerYamlGenerator(); + + generator.setOutputFile("../../../etc/passwd"); + + Swagger swagger = new Swagger(); + swagger.setInfo(new Info().title("Test API").version("1.0.0")); + + try { + generator.processSwagger(swagger); + } catch (Exception e) { + Assert.fail("processSwagger should handle SecurityException gracefully: " + e.getMessage()); + } + } +} diff --git a/modules/swagger-codegen/src/test/java/io/swagger/codegen/utils/SecureFileUtilsTest.java b/modules/swagger-codegen/src/test/java/io/swagger/codegen/utils/SecureFileUtilsTest.java new file mode 100644 index 00000000000..718beda8f61 --- /dev/null +++ b/modules/swagger-codegen/src/test/java/io/swagger/codegen/utils/SecureFileUtilsTest.java @@ -0,0 +1,166 @@ +package io.swagger.codegen.utils; + +import org.testng.Assert; +import org.testng.annotations.Test; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.AfterMethod; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; + +/** + * Unit tests for SecureFileUtils class. + * Tests path validation and security checks for preventing path traversal attacks. + */ +public class SecureFileUtilsTest { + + private Path tempDir; + private File validFile; + private File validDirectory; + + @BeforeMethod + public void setUp() throws IOException { + tempDir = Files.createTempDirectory("secure-file-utils-test"); + validFile = tempDir.resolve("validfile.txt").toFile(); + validDirectory = tempDir.resolve("validdir").toFile(); + + if (!validFile.createNewFile()) { + throw new IOException("Failed to create test file"); + } + if (!validDirectory.mkdir()) { + throw new IOException("Failed to create test directory"); + } + } + + @AfterMethod + public void tearDown() { + deleteDirectoryRecursively(tempDir.toFile()); + } + + @Test + public void testValidatePathWithValidFile() { + try { + SecureFileUtils.validatePath(validFile); + } catch (Exception e) { + Assert.fail("Valid file should not throw exception: " + e.getMessage()); + } + } + + @Test + public void testValidatePathWithValidDirectory() { + try { + SecureFileUtils.validatePath(validDirectory); + } catch (Exception e) { + Assert.fail("Valid directory should not throw exception: " + e.getMessage()); + } + } + + @Test(expectedExceptions = IllegalArgumentException.class) + public void testValidatePathWithNullFile() { + SecureFileUtils.validatePath((File) null); + } + + @Test(expectedExceptions = SecurityException.class) + public void testValidatePathWithFileContainingDotDotInAbsolutePath() throws IOException { + File parentDir = new File(tempDir.toFile(), "parent"); + File childDir = new File(parentDir, "child"); + File dotDotDir = new File(childDir, ".."); + File targetFile = new File(dotDotDir, "file.txt"); + + if (!parentDir.mkdirs()) { + throw new IOException("Failed to create parent directory"); + } + if (!childDir.mkdirs()) { + throw new IOException("Failed to create child directory"); + } + + SecureFileUtils.validatePath(targetFile); + } + + @Test + public void testValidatePathWithValidStringPath() { + String validPath = validFile.getAbsolutePath(); + + try { + SecureFileUtils.validatePath(validPath); + } catch (Exception e) { + Assert.fail("Valid string path should not throw exception: " + e.getMessage()); + } + } + + @Test(expectedExceptions = IllegalArgumentException.class) + public void testValidatePathWithNullString() { + SecureFileUtils.validatePath((String) null); + } + + @Test(expectedExceptions = IllegalArgumentException.class) + public void testValidatePathWithEmptyString() { + SecureFileUtils.validatePath(""); + } + + @Test(expectedExceptions = SecurityException.class) + public void testValidatePathWithParentTraversalString() { + SecureFileUtils.validatePath("../../../etc/passwd"); + } + + @Test(expectedExceptions = SecurityException.class) + public void testValidatePathWithBackwardSlashTraversal() { + SecureFileUtils.validatePath("..\\..\\windows\\system32"); + } + + @Test(expectedExceptions = SecurityException.class) + public void testValidatePathWithMixedTraversal() { + SecureFileUtils.validatePath("legitimate/path/../../../sensitive/file"); + } + + @Test(expectedExceptions = SecurityException.class) + public void testValidatePathWithNullByteInString() { + SecureFileUtils.validatePath("normalfile.txt\0hiddenfile.exe"); + } + + @Test(expectedExceptions = SecurityException.class) + public void testValidatePathWithMultipleNullBytes() { + SecureFileUtils.validatePath("file\0with\0null\0bytes.txt"); + } + + @Test + public void testValidatePathWithCurrentDirectoryReference() { + try { + SecureFileUtils.validatePath("./currentdir/file.txt"); + } catch (Exception e) { + Assert.fail("Current directory reference should not throw exception: " + e.getMessage()); + } + } + + @Test + public void testValidatePathWithFileNameContainingDotsOnly() { + try { + SecureFileUtils.validatePath("file.name.with.dots.txt"); + } catch (Exception e) { + Assert.fail("Filename with dots should not throw exception: " + e.getMessage()); + } + } + + private void deleteDirectoryRecursively(File dir) { + if (dir.exists()) { + File[] files = dir.listFiles(); + if (files != null) { + for (File file : files) { + if (file.isDirectory()) { + deleteDirectoryRecursively(file); + } else { + if (!file.delete()) { + System.err.println("Failed to delete file: " + file.getAbsolutePath()); + } + } + } + } + if (!dir.delete()) { + System.err.println("Failed to delete directory: " + dir.getAbsolutePath()); + } + } + } + +} diff --git a/modules/swagger-generator/src/main/java/io/swagger/generator/online/Generator.java b/modules/swagger-generator/src/main/java/io/swagger/generator/online/Generator.java index 3c1fe8c9e77..5c1c9fabc2f 100644 --- a/modules/swagger-generator/src/main/java/io/swagger/generator/online/Generator.java +++ b/modules/swagger-generator/src/main/java/io/swagger/generator/online/Generator.java @@ -2,6 +2,7 @@ import com.fasterxml.jackson.databind.JsonNode; import io.swagger.codegen.*; +import io.swagger.codegen.utils.SecureFileUtils; import io.swagger.generator.exception.ApiException; import io.swagger.generator.exception.BadRequestException; import io.swagger.generator.model.GeneratorInput; @@ -146,6 +147,7 @@ private static String generate(String language, GeneratorInput opts, Type type) if (files.size() > 0) { List filesToAdd = new ArrayList(); LOGGER.debug("adding to " + outputFolder); + SecureFileUtils.validatePath(outputFolder); filesToAdd.add(new File(outputFolder)); ZipUtil zip = new ZipUtil(); zip.compressFiles(filesToAdd, outputFilename); @@ -161,6 +163,7 @@ private static String generate(String language, GeneratorInput opts, Type type) } } try { + SecureFileUtils.validatePath(outputFilename); new File(outputFolder).delete(); } catch (Exception e) { LOGGER.error("unable to delete output folder " + outputFolder); diff --git a/modules/swagger-generator/src/main/java/io/swagger/generator/resource/SwaggerResource.java b/modules/swagger-generator/src/main/java/io/swagger/generator/resource/SwaggerResource.java index 373474cf1db..33dac7f8b84 100644 --- a/modules/swagger-generator/src/main/java/io/swagger/generator/resource/SwaggerResource.java +++ b/modules/swagger-generator/src/main/java/io/swagger/generator/resource/SwaggerResource.java @@ -7,6 +7,7 @@ import io.swagger.codegen.Codegen; import io.swagger.codegen.CodegenConfig; import io.swagger.codegen.CodegenType; +import io.swagger.codegen.utils.SecureFileUtils; import io.swagger.generator.exception.BadRequestException; import io.swagger.generator.model.Generated; import io.swagger.generator.model.GeneratorInput; @@ -59,10 +60,12 @@ public Response downloadFile(@PathParam("fileId") String fileId) throws Exceptio System.out.println("looking for fileId " + fileId); System.out.println("got filename " + g.getFilename()); if (g.getFilename() != null) { + SecureFileUtils.validatePath(g.getFilename()); File file = new java.io.File(g.getFilename()); byte[] bytes = org.apache.commons.io.FileUtils.readFileToByteArray(file); try { + SecureFileUtils.validatePath(file.getParentFile()); FileUtils.deleteDirectory(file.getParentFile()); } catch (Exception e) { System.out.println("failed to delete file " + file.getAbsolutePath()); diff --git a/modules/swagger-generator/src/main/java/io/swagger/generator/util/ZipUtil.java b/modules/swagger-generator/src/main/java/io/swagger/generator/util/ZipUtil.java index 80c1872b999..6d0759fcb63 100644 --- a/modules/swagger-generator/src/main/java/io/swagger/generator/util/ZipUtil.java +++ b/modules/swagger-generator/src/main/java/io/swagger/generator/util/ZipUtil.java @@ -14,6 +14,8 @@ package io.swagger.generator.util; +import io.swagger.codegen.utils.SecureFileUtils; + import java.io.BufferedInputStream; import java.io.File; import java.io.FileInputStream; @@ -44,10 +46,12 @@ public class ZipUtil { * @param destZipFile The path of the destination zip file * @throws FileNotFoundException if file not found * @throws IOException if IO exception occurs + * @throws SecurityException if path validation fails */ public void compressFiles(List listFiles, String destZipFile) - throws FileNotFoundException, IOException { + throws FileNotFoundException, IOException, SecurityException { + SecureFileUtils.validatePath(destZipFile); ZipOutputStream zos = new ZipOutputStream(new FileOutputStream(destZipFile)); for (File file : listFiles) { @@ -70,9 +74,10 @@ public void compressFiles(List listFiles, String destZipFile) * @param zos the current zip output stream * @throws FileNotFoundException if file not found * @throws IOException if IO exception occurs + * @throws SecurityException if path validation fails */ private void addFolderToZip(File folder, String parentFolder, ZipOutputStream zos) - throws FileNotFoundException, IOException { + throws FileNotFoundException, IOException, SecurityException { for (File file : folder.listFiles()) { if (file.isDirectory()) { addFolderToZip(file, parentFolder + "/" + file.getName(), zos); @@ -104,9 +109,11 @@ private void addFolderToZip(File folder, String parentFolder, ZipOutputStream zo * @param zos the current zip output stream * @throws FileNotFoundException if file not found * @throws IOException if IO exception occurs + * @throws SecurityException if path validation fails */ private static void addFileToZip(File file, ZipOutputStream zos) throws FileNotFoundException, - IOException { + IOException, SecurityException { + SecureFileUtils.validatePath(file.getName()); zos.putNextEntry(new ZipEntry(file.getName())); BufferedInputStream bis = new BufferedInputStream(new FileInputStream(file)); diff --git a/modules/swagger-generator/src/test/java/io/swagger/generator/online/GeneratorTest.java b/modules/swagger-generator/src/test/java/io/swagger/generator/online/GeneratorTest.java new file mode 100644 index 00000000000..e2c0db2d067 --- /dev/null +++ b/modules/swagger-generator/src/test/java/io/swagger/generator/online/GeneratorTest.java @@ -0,0 +1,34 @@ +package io.swagger.generator.online; + +import io.swagger.generator.exception.BadRequestException; +import org.testng.annotations.Test; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; + +import java.util.HashMap; +import java.util.Map; + +/** + * Unit test for Generator class to verify SecurityException handling + * when SecureFileUtils.validatePath throws exceptions. + */ +public class GeneratorTest { + + @Test(expectedExceptions = BadRequestException.class) + public void testGenerateWithPathTraversalInOutputFolder() throws Exception { + io.swagger.generator.model.GeneratorInput opts = new io.swagger.generator.model.GeneratorInput(); + + ObjectMapper mapper = new ObjectMapper(); + JsonNode spec = mapper.readTree( + "{\"swagger\":\"2.0\",\"info\":{\"title\":\"Test\",\"version\":\"1.0\"}," + + "\"paths\":{\"/test\":{\"get\":{\"responses\":{\"200\":{\"description\":\"OK\"}}}}}}" + ); + opts.setSpec(spec); + + Map options = new HashMap<>(); + options.put("outputFolder", "../../../etc/passwd"); + opts.setOptions(options); + + Generator.generateClient("java", opts); + } +} diff --git a/modules/swagger-generator/src/test/java/io/swagger/generator/resource/SwaggerResourceTest.java b/modules/swagger-generator/src/test/java/io/swagger/generator/resource/SwaggerResourceTest.java new file mode 100644 index 00000000000..590e6245302 --- /dev/null +++ b/modules/swagger-generator/src/test/java/io/swagger/generator/resource/SwaggerResourceTest.java @@ -0,0 +1,27 @@ +package io.swagger.generator.resource; + +import org.testng.annotations.Test; + +public class SwaggerResourceTest { + + @Test(expectedExceptions = SecurityException.class) + public void testDownloadFileWithPathTraversal() throws Exception { + SwaggerResource resource = new SwaggerResource(); + + io.swagger.generator.model.Generated generated = new io.swagger.generator.model.Generated(); + generated.setFilename("../../../etc/passwd"); + + java.lang.reflect.Field fileMapField = SwaggerResource.class.getDeclaredField("fileMap"); + fileMapField.setAccessible(true); + @SuppressWarnings("unchecked") + java.util.Map fileMap = + (java.util.Map) fileMapField.get(null); + fileMap.put("test-file-id", generated); + + try { + resource.downloadFile("test-file-id"); + } finally { + fileMap.remove("test-file-id"); + } + } +} diff --git a/modules/swagger-generator/src/test/java/io/swagger/generator/util/ZipUtilTest.java b/modules/swagger-generator/src/test/java/io/swagger/generator/util/ZipUtilTest.java new file mode 100644 index 00000000000..a6c21daea1d --- /dev/null +++ b/modules/swagger-generator/src/test/java/io/swagger/generator/util/ZipUtilTest.java @@ -0,0 +1,20 @@ +package io.swagger.generator.util; + +import org.testng.annotations.Test; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Collections; + +public class ZipUtilTest { + + @Test(expectedExceptions = SecurityException.class) + public void testCompressFilesWithPathTraversal() throws Exception { + ZipUtil zipUtil = new ZipUtil(); + + Path tempFile = Files.createTempFile("test", ".txt"); + Files.write(tempFile, "test content".getBytes()); + + zipUtil.compressFiles(Collections.singletonList(tempFile.toFile()), "../../../etc/passwd.zip"); + } +}