From 1cf82abac2c5157031f17c80ec93586b9379dc2c Mon Sep 17 00:00:00 2001 From: DuoChen-317 Date: Mon, 1 Dec 2025 16:26:50 -0500 Subject: [PATCH] 1 --- .../BooleanConceptChangeSet.java | 21 ++++++++--- .../module/web/ModuleResourcesServlet.java | 12 +++++-- .../org/openmrs/module/web/WebModuleUtil.java | 36 ++++++++++++++++--- .../initialization/InitializationFilter.java | 8 ++++- .../initialization/TestInstallUtil.java | 13 ++++++- .../web/filter/update/UpdateFilter.java | 4 ++- 6 files changed, 80 insertions(+), 14 deletions(-) diff --git a/api/src/main/java/org/openmrs/util/databasechange/BooleanConceptChangeSet.java b/api/src/main/java/org/openmrs/util/databasechange/BooleanConceptChangeSet.java index f4cd7e6d97ae..45ef4774c01a 100644 --- a/api/src/main/java/org/openmrs/util/databasechange/BooleanConceptChangeSet.java +++ b/api/src/main/java/org/openmrs/util/databasechange/BooleanConceptChangeSet.java @@ -103,14 +103,25 @@ public void execute(Database database) throws CustomChangeException { * @return a concept id. * @throws CustomChangeException */ - private Integer findConceptByName(JdbcConnection connection, Map names) throws CustomChangeException { + private Integer findConceptByName(JdbcConnection connection, Map names) + throws CustomChangeException { for (Map.Entry e : names.entrySet()) { String locale = e.getKey(); + for (String name : e.getValue()) { - Integer ret = getInt(connection, "select concept_id from concept_name where name = '" + name - + "' and locale like '" + locale + "%'"); - if (ret != null) { - return ret; + String sql = "select concept_id from concept_name where name = ? and locale like ?"; + + try (PreparedStatement ps = connection.prepareStatement(sql)) { + ps.setString(1, name); + ps.setString(2, locale + "%"); + + try (ResultSet rs = ps.executeQuery()) { + if (rs.next()) { + return rs.getInt(1); + } + } + } catch (Exception ex) { + throw new CustomChangeException("Error querying concept by name", ex); } } } diff --git a/web/src/main/java/org/openmrs/module/web/ModuleResourcesServlet.java b/web/src/main/java/org/openmrs/module/web/ModuleResourcesServlet.java index afe9c3907f0f..a574254eeb5a 100644 --- a/web/src/main/java/org/openmrs/module/web/ModuleResourcesServlet.java +++ b/web/src/main/java/org/openmrs/module/web/ModuleResourcesServlet.java @@ -100,8 +100,16 @@ protected File getFile(HttpServletRequest request) { } realPath = realPath.replace("/", File.separator); - - File f = new File(realPath); + // fix + String safePath = realPath; + if (safePath.contains("..")) { + throw new UnsupportedOperationException( + "Attempted to access file '" + safePath + + "' but this is rejected because it contains '..' which may indicate " + + "a path traversal or zip-slip style attack." + ); + } + File f = new File(safePath); if (!f.exists()) { log.warn("No file with path '" + realPath + "' exists for module '" + module.getModuleId() + "'"); return null; diff --git a/web/src/main/java/org/openmrs/module/web/WebModuleUtil.java b/web/src/main/java/org/openmrs/module/web/WebModuleUtil.java index cb1e356d68fe..d17aaf51b12d 100644 --- a/web/src/main/java/org/openmrs/module/web/WebModuleUtil.java +++ b/web/src/main/java/org/openmrs/module/web/WebModuleUtil.java @@ -185,7 +185,13 @@ public static boolean startModule(Module mod, ServletContext servletContext, boo log.debug("Moving file from: {} to {}", name, absPath); // get the output file - File outFile = new File(absPath.toString().replace("/", File.separator)); + String safePath = absPath.toString(); + + // basic path traversal protection + if (safePath.contains("..") || safePath.contains("\\") || safePath.contains("//")) { + throw new IllegalArgumentException("Unsafe file path detected: " + safePath); + } + File outFile = new File(safePath.replace("/", File.separator)); if (entry.isDirectory()) { if (!outFile.exists()) { outFile.mkdirs(); @@ -782,7 +788,15 @@ public static void shutdownModules(ServletContext servletContext) { // clear the module messages String messagesPath = realPath + "/WEB-INF/"; - File folder = new File(messagesPath.replace("/", File.separator)); + // Normalize separators + String safePath = messagesPath.replace("/", File.separator); + + // Detect basic path traversal patterns + if (safePath.contains("..")) { + throw new IllegalArgumentException("Invalid path: potential path traversal detected: " + safePath); + } + + File folder = new File(safePath); File[] files = folder.listFiles(); if (folder.exists() && files != null) { @@ -833,8 +847,22 @@ public static void stopModule(Module mod, ServletContext servletContext, boolean String realPath = getRealPath(servletContext); // delete the web files from the webapp - String absPath = realPath + "/WEB-INF/view/module/" + moduleId; - File moduleWebFolder = new File(absPath.replace("/", File.separator)); + String absPath = realPath + "/WEB-INF/web/module/" + moduleId; + + // normalize path separator + String safePath = absPath.replace("/", File.separator); + + // Basic path traversal protection + if (safePath.contains("..")) { + throw new UnsupportedOperationException( + "Attempted to access '" + safePath + + "' but this was rejected because it contains '..', which may indicate " + + "a path traversal or zip-slip style attack." + ); + } + + File moduleWebFolder = new File(safePath); + if (moduleWebFolder.exists()) { try { OpenmrsUtil.deleteDirectory(moduleWebFolder); diff --git a/web/src/main/java/org/openmrs/web/filter/initialization/InitializationFilter.java b/web/src/main/java/org/openmrs/web/filter/initialization/InitializationFilter.java index 225929ffd602..d95c5e4b306c 100644 --- a/web/src/main/java/org/openmrs/web/filter/initialization/InitializationFilter.java +++ b/web/src/main/java/org/openmrs/web/filter/initialization/InitializationFilter.java @@ -260,7 +260,9 @@ protected void doGet(HttpServletRequest httpRequest, HttpServletResponse httpRes } PrintWriter writer = httpResponse.getWriter(); - writer.write(toJSONString(result)); + String jsonText = toJSONString(result); + jsonText = org.apache.commons.text.StringEscapeUtils.escapeHtml4(jsonText); + writer.write(jsonText); writer.close(); } else if (InitializationWizardModel.INSTALL_METHOD_AUTO.equals(wizardModel.installMethod) || httpRequest.getServletPath().equals("/" + AUTO_RUN_OPENMRS)) { @@ -1039,6 +1041,10 @@ private File getRuntimePropertiesFile() { String pathName = OpenmrsUtil.getRuntimePropertiesFilePathName(WebConstants.WEBAPP_NAME); if (pathName != null) { + // Block path traversal attempts + if (pathName.contains("..") || pathName.contains("/") || pathName.contains("\\")) { + throw new IllegalArgumentException("Unsafe path detected: " + pathName); + } file = new File(pathName); } else { file = new File(OpenmrsUtil.getApplicationDataDirectory(), getRuntimePropertiesFileName()); diff --git a/web/src/main/java/org/openmrs/web/filter/initialization/TestInstallUtil.java b/web/src/main/java/org/openmrs/web/filter/initialization/TestInstallUtil.java index 7d7547f12f04..0a1703eceab5 100644 --- a/web/src/main/java/org/openmrs/web/filter/initialization/TestInstallUtil.java +++ b/web/src/main/java/org/openmrs/web/filter/initialization/TestInstallUtil.java @@ -159,7 +159,11 @@ protected static boolean addZippedTestModules(InputStream in) { //Convert the names of .omod files located in nested directories so that they get //created under the module repo directory when being copied if (fileName.contains(System.getProperty("file.separator"))) { - fileName = new File(entry.getName()).getName(); + String entryName = entry.getName(); + if (entryName.contains("..") || entryName.contains("/") || entryName.contains("\\")) { + throw new IOException("Blocked unsafe zip entry name: " + entryName); + } + fileName = new File(entryName).getName(); } log.debug("Extracting module file: {}", fileName); @@ -186,6 +190,13 @@ protected static boolean addZippedTestModules(InputStream in) { FileUtils.cleanDirectory(moduleRepository); final File zipEntryFile = new File(moduleRepository, fileName); + + String canonicalRepoPath = moduleRepository.getCanonicalPath(); + String canonicalTargetPath = zipEntryFile.getCanonicalPath(); + + if (!canonicalTargetPath.startsWith(canonicalRepoPath)) { + throw new IOException("Blocked unsafe file path: " + fileName); + } if (!zipEntryFile.toPath().normalize().startsWith(moduleRepository.toPath().normalize())) { throw new IOException("Bad zip entry"); diff --git a/web/src/main/java/org/openmrs/web/filter/update/UpdateFilter.java b/web/src/main/java/org/openmrs/web/filter/update/UpdateFilter.java index 08c61759bbcb..6d4ba06eb84c 100644 --- a/web/src/main/java/org/openmrs/web/filter/update/UpdateFilter.java +++ b/web/src/main/java/org/openmrs/web/filter/update/UpdateFilter.java @@ -274,8 +274,10 @@ else if (REVIEW_CHANGES.equals(page)) { addLogLinesToResponse(result); } - + String jsonText = toJSONString(result); + httpResponse.setContentType("application/json;charset=UTF-8"); + jsonText = org.apache.commons.text.StringEscapeUtils.escapeHtml4(jsonText); httpResponse.getWriter().write(jsonText); } }