From 6db903ad5e2eb7f7ca7b212700a1e36104a2e74c Mon Sep 17 00:00:00 2001 From: Mayssa Rouissi Date: Tue, 21 Apr 2026 11:36:16 +0200 Subject: [PATCH] updates with the eskemm proposition --- .../creatis/vip/api/business/ApiBusiness.java | 4 +- .../vip/api/business/ApiUserBusiness.java | 6 +- .../vip/api/business/ExecutionBusiness.java | 4 +- .../controller/AuthenticationController.java | 4 +- .../server/business/BoutiquesBusiness.java | 70 ++++++++++++++----- .../parser/AbstractWorkflowParser.java | 24 +++++-- .../server/dao/h2/SimulationData.java | 35 ++++++++-- .../server/business/VipSessionBusiness.java | 2 +- 8 files changed, 111 insertions(+), 38 deletions(-) diff --git a/vip-api/src/main/java/fr/insalyon/creatis/vip/api/business/ApiBusiness.java b/vip-api/src/main/java/fr/insalyon/creatis/vip/api/business/ApiBusiness.java index c9b61b3a0..07ccc1813 100644 --- a/vip-api/src/main/java/fr/insalyon/creatis/vip/api/business/ApiBusiness.java +++ b/vip-api/src/main/java/fr/insalyon/creatis/vip/api/business/ApiBusiness.java @@ -61,7 +61,7 @@ private User signin(String username, String password) throws VipException { try { // we do not care about the session, we're not in browser action User user = authenticationBusiness.signin(username, password); - logger.info("Credentials OK for " + username); + logger.info("Credentials OK for " + username.replaceAll("[\\n\\r]", "_")); return user; } catch (VipException e) { if (e.getMessage().startsWith("Authentication failed")) { @@ -75,7 +75,7 @@ private String getAnApikeyForUser(String email) throws VipException { boolean generateNewApiKey = server.getCarminApikeyGenerateNewEachTime(); if (generateNewApiKey) { - logger.info("generating a new apikey for " + email); + logger.info("generating a new apikey for " + email.replaceAll("[\\n\\r]", "_")); return userBusiness.generateNewUserApikey(email); } else { logger.debug("keeping the current api key for " + email); diff --git a/vip-api/src/main/java/fr/insalyon/creatis/vip/api/business/ApiUserBusiness.java b/vip-api/src/main/java/fr/insalyon/creatis/vip/api/business/ApiUserBusiness.java index e9321e892..58ed7e123 100644 --- a/vip-api/src/main/java/fr/insalyon/creatis/vip/api/business/ApiUserBusiness.java +++ b/vip-api/src/main/java/fr/insalyon/creatis/vip/api/business/ApiUserBusiness.java @@ -36,16 +36,16 @@ public void signup(User user, String comments) throws VipException { false, true, new HashSet<>()); - logger.info("Signing up with the " + user.getEmail()); + logger.info("Signing up with the " + user.getEmail().replaceAll("[\\n\\r]", "_")); } public void sendResetCode(String email) throws VipException { emailBusiness.sendResetCode(email); - logger.info("Sending reset code for user with email: " + email); + logger.info("Sending reset code for user with email: " + email.replaceAll("[\\n\\r]", "_")); } public void resetPassword(String email, String code, String password) throws VipException { passwordBusiness.reset(email, code, password); - logger.info("Resetting password for user with email: " + email); + logger.info("Resetting password for user with email: " + email.replaceAll("[\\n\\r]", "_")); } } diff --git a/vip-api/src/main/java/fr/insalyon/creatis/vip/api/business/ExecutionBusiness.java b/vip-api/src/main/java/fr/insalyon/creatis/vip/api/business/ExecutionBusiness.java index 94d0655b3..b6b9d8741 100644 --- a/vip-api/src/main/java/fr/insalyon/creatis/vip/api/business/ExecutionBusiness.java +++ b/vip-api/src/main/java/fr/insalyon/creatis/vip/api/business/ExecutionBusiness.java @@ -310,8 +310,8 @@ public void updateExecution(Execution execution) throws VipException { throw new VipException("Update of execution timeout is not supported."); } checkInputExecNameIsValid(execution.getName()); - logger.info("updating execution " + execution.getIdentifier() - + " name to " + execution.getName()); + logger.info("updating execution " + execution.getIdentifier().replaceAll("[\\n\\r]", "_") + + " name to " + execution.getName().replaceAll("[\\n\\r]", "_")); workflowBusiness.updateDescription(execution.getIdentifier(), execution.getName()); } diff --git a/vip-api/src/main/java/fr/insalyon/creatis/vip/api/controller/AuthenticationController.java b/vip-api/src/main/java/fr/insalyon/creatis/vip/api/controller/AuthenticationController.java index 907c17b35..69edff22c 100644 --- a/vip-api/src/main/java/fr/insalyon/creatis/vip/api/controller/AuthenticationController.java +++ b/vip-api/src/main/java/fr/insalyon/creatis/vip/api/controller/AuthenticationController.java @@ -51,10 +51,10 @@ public void sendResetPassword(@RequestBody @Valid ResetPasswordDTO resetPassword logMethodInvocation(logger, "resetPassword", resetPassword.getEmail()); if (resetPassword.getActivationCode() == null) { apiUserBusiness.sendResetCode(resetPassword.getEmail()); - logger.info("reset code of " + resetPassword.getEmail()); + logger.info("reset code of " + resetPassword.getEmail().replaceAll("[\\n\\r]", "_")); } else { apiUserBusiness.resetPassword(resetPassword.getEmail(), resetPassword.getActivationCode(), resetPassword.getNewPassword()); - logger.info("reset password with activation code: " + resetPassword.getActivationCode()); + logger.info("reset password with activation code: " + resetPassword.getActivationCode().replaceAll("[\\n\\r]", "_")); } } diff --git a/vip-application/src/main/java/fr/insalyon/creatis/vip/application/server/business/BoutiquesBusiness.java b/vip-application/src/main/java/fr/insalyon/creatis/vip/application/server/business/BoutiquesBusiness.java index 2c73381b2..181df0f28 100644 --- a/vip-application/src/main/java/fr/insalyon/creatis/vip/application/server/business/BoutiquesBusiness.java +++ b/vip-application/src/main/java/fr/insalyon/creatis/vip/application/server/business/BoutiquesBusiness.java @@ -10,6 +10,7 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.util.ArrayList; +import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -80,9 +81,9 @@ public String publishVersion(User user, String applicationName, String version) String doi; try { // call publish command - String command = "FILE=" + tempFile + "; " + server.getPublicationCommandLine(); - List output = runCommandAndFailOnError(command); - + List commandArgs = Arrays.asList("bosh", "publish", server.getPublicationCommandLine()); + List output = runCommandAndFailOnError(commandArgs, tempFile.toString()); + // get the doi // There should be only one line with the DOI doi = getDoiFromPublishOutput(output); @@ -111,7 +112,7 @@ public void validateBoutiquesFile(String localPath) throws VipException { throw new VipException("Can't get boutiques file size", e); } // call validate command - String command = "bosh validate " + localPath; + List command = Arrays.asList("bosh", "validate", localPath); try { // if no exception : the command was successful runCommand(command); @@ -188,22 +189,22 @@ public List getCout() { } } - private List runCommandAndFailOnError(String command) throws VipException { + private List runCommandAndFailOnError(List commandArgs, String tempFilePath) throws VipException { try { - return runCommand(command); + return runCommandWithEnv(commandArgs, tempFilePath); } catch (CommandErrorException e) { - throw new VipException("Command {" + command + "} failed : " + String.join("\n", e.getCout())); + throw new VipException("Command failed : " + String.join("\n", e.getCout())); } } - private List runCommand(String command) throws CommandErrorException, VipException { - ProcessBuilder builder = new ProcessBuilder("bash", "-c", command); + private List runCommand(List commandArgs) throws CommandErrorException, VipException { + ProcessBuilder builder = new ProcessBuilder(commandArgs); builder.redirectErrorStream(true); Process process = null; List cout = new ArrayList<>(); try { - logger.info("Executing command : " + command); + logger.info("Executing command : " + String.join(" ", commandArgs)); process = builder.start(); BufferedReader r = new BufferedReader( new InputStreamReader(process.getInputStream())); @@ -229,16 +230,51 @@ private List runCommand(String command) throws CommandErrorException, Vi process = null; return cout; } + private List runCommandWithEnv(List commandArgs, String tempFilePath)throws CommandErrorException, VipException { + + ProcessBuilder builder = new ProcessBuilder(commandArgs); + builder.redirectErrorStream(true); + + if (tempFilePath != null) { + builder.environment().put("FILE", tempFilePath); + } + + Process process = null; + List cout = new ArrayList<>(); - private void closeProcess(Process process) { - if (process == null) - return; - close(process.getOutputStream()); - close(process.getInputStream()); - close(process.getErrorStream()); - process.destroy(); + try { + logger.info("Executing command with FILE=" + tempFilePath + " : " + String.join(" ", commandArgs)); + process = builder.start(); + + try (BufferedReader r = new BufferedReader(new InputStreamReader(process.getInputStream()))) { + String s; + while ((s = r.readLine()) != null) { + cout.add(s); + } + } + process.waitFor(); + } catch (IOException | InterruptedException e) { + logger.error("Unexpected error in a boutiques command", e); + throw new VipException("Unexpected error in a boutiques command", e); + } finally { + closeProcess(process); + } + + if (process.exitValue() != 0) { + throw new CommandErrorException(cout); + } + return cout; } + private void closeProcess(Process process) { + if (process == null) + return; + close(process.getOutputStream()); + close(process.getInputStream()); + close(process.getErrorStream()); + process.destroy(); + } + private void close(Closeable c) { if (c != null) { diff --git a/vip-application/src/main/java/fr/insalyon/creatis/vip/application/server/business/simulation/parser/AbstractWorkflowParser.java b/vip-application/src/main/java/fr/insalyon/creatis/vip/application/server/business/simulation/parser/AbstractWorkflowParser.java index 9b2c31f79..a0cb60af6 100644 --- a/vip-application/src/main/java/fr/insalyon/creatis/vip/application/server/business/simulation/parser/AbstractWorkflowParser.java +++ b/vip-application/src/main/java/fr/insalyon/creatis/vip/application/server/business/simulation/parser/AbstractWorkflowParser.java @@ -12,9 +12,12 @@ import org.xml.sax.InputSource; import org.xml.sax.SAXException; +import org.xml.sax.SAXNotRecognizedException; +import org.xml.sax.SAXNotSupportedException; import org.xml.sax.XMLReader; -import org.xml.sax.helpers.DefaultHandler; - +import org.xml.sax.helpers.DefaultHandler; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import fr.insalyon.creatis.vip.application.models.Descriptor; import fr.insalyon.creatis.vip.application.models.Source; @@ -24,6 +27,7 @@ public abstract class AbstractWorkflowParser extends DefaultHandler { protected XMLReader reader; protected List sources; protected String description = "No description found for this application. Please contact the developer to know what it is about."; + private static final Logger logger = LoggerFactory.getLogger(AbstractWorkflowParser.class); protected AbstractWorkflowParser() { sources = new ArrayList(); @@ -37,13 +41,25 @@ public Descriptor parseString(String workflowString) throws IOException, SAXExce return parse(new StringReader(workflowString)); } - private Descriptor parse(Reader workflowReader) throws IOException, SAXException, ParserConfigurationException { + private Descriptor parse(Reader workflowReader) throws IOException, SAXException, ParserConfigurationException { SAXParserFactory parserFactory = SAXParserFactory.newInstance(); parserFactory.setNamespaceAware(true); + + try { + // Disable external general entities to prevent local file disclosure + parserFactory.setFeature("http://xml.org/sax/features/external-general-entities", false); + // Disable external parameter entities to prevent attacks via external DTD files + parserFactory.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + // Prevent the parser from loading external DTDs (Document Type Definitions) + parserFactory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); + } catch (SAXNotRecognizedException | SAXNotSupportedException e) { + logger.warn("The SAX parser does not support some XXE security features", e); + } + reader = parserFactory.newSAXParser().getXMLReader(); reader.setContentHandler(this); reader.parse(new InputSource(workflowReader)); - return new Descriptor(sources,description); + return new Descriptor(sources, description); } } diff --git a/vip-application/src/main/java/fr/insalyon/creatis/vip/application/server/dao/h2/SimulationData.java b/vip-application/src/main/java/fr/insalyon/creatis/vip/application/server/dao/h2/SimulationData.java index e243b1b06..a32f19b32 100644 --- a/vip-application/src/main/java/fr/insalyon/creatis/vip/application/server/dao/h2/SimulationData.java +++ b/vip-application/src/main/java/fr/insalyon/creatis/vip/application/server/dao/h2/SimulationData.java @@ -9,6 +9,7 @@ import java.sql.SQLException; import java.sql.Statement; import java.util.ArrayList; +import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -448,21 +449,41 @@ public Map getNodeCountriesMap() throws DAOException { } private List getHistogramTimes(int binSize, String startField, String endField) - throws DAOException { + throws DAOException { + + //the method :Whitelist;to prevent SQL injection + // accepts only the allowed fields, and throws an exception if the fields are not in the allowed list + List allowedFields = Arrays.asList("running", "upload", "download", "end_e"); + + // if the fields are not in the allowed list, we log an error and throw an exception + if (!allowedFields.contains(startField) || !allowedFields.contains(endField)) { + logger.error("Tentative d'injection SQL détectée avec les champs : " + startField + " / " + endField); + throw new DAOException("Accès non autorisé aux colonnes spécifiées."); + } try { List list = new ArrayList(); - PreparedStatement ps = connection.prepareStatement("SELECT " - + "TIMESTAMPDIFF('SECOND', " + startField + ", " + endField + ")/" + binSize + "*" + binSize + " AS execut, " + + // prepare the query with placeholders for the binsize and the status, and for the fields which are validated by the whitelist + String query = "SELECT " + + "TIMESTAMPDIFF('SECOND', " + startField + ", " + endField + ")/?*? AS execut, " + "COUNT(id) AS num, " + "MIN(TIMESTAMPDIFF('SECOND', " + startField + ", " + endField + ")) AS mini, " + "MAX(TIMESTAMPDIFF('SECOND', " + startField + ", " + endField + ")) AS maxi, " + "SUM(TIMESTAMPDIFF('SECOND', " + startField + ", " + endField + ")) AS som " + "FROM JOBS " + "WHERE STATUS = ? AND TIMESTAMPDIFF('SECOND', " + startField + ", " + endField + ") >= 0 " - + "GROUP BY TIMESTAMPDIFF('SECOND', " + startField + ", " + endField + ")/" + binSize + "*" + binSize + " " - + "ORDER BY EXECUT"); - ps.setString(1, TaskStatus.COMPLETED.name()); + + "GROUP BY TIMESTAMPDIFF('SECOND', " + startField + ", " + endField + ")/?*? " + + "ORDER BY EXECUT"; + + PreparedStatement ps = connection.prepareStatement(query); + + // Binding the parameters for the binsize and the status, and for the fields which are validated by the whitelist + ps.setInt(1, binSize); + ps.setInt(2, binSize); + ps.setString(3, TaskStatus.COMPLETED.name()); + ps.setInt(4, binSize); + ps.setInt(5, binSize); ResultSet rs = ps.executeQuery(); @@ -483,7 +504,7 @@ private List getHistogramTimes(int binSize, String startField, String en close(logger); } } - + private int parseMinorStatus(String minorStatus) { if (minorStatus == null || minorStatus.isEmpty()) { diff --git a/vip-core/src/main/java/fr/insalyon/creatis/vip/core/server/business/VipSessionBusiness.java b/vip-core/src/main/java/fr/insalyon/creatis/vip/core/server/business/VipSessionBusiness.java index 340fad9e2..225754f6e 100644 --- a/vip-core/src/main/java/fr/insalyon/creatis/vip/core/server/business/VipSessionBusiness.java +++ b/vip-core/src/main/java/fr/insalyon/creatis/vip/core/server/business/VipSessionBusiness.java @@ -109,7 +109,7 @@ public User resetSessionFromCookie(HttpServletRequest request) String email = cookies.get(CoreConstants.COOKIES_USER); String sessionId = cookies.get(CoreConstants.COOKIES_SESSION); // the cookies are there, verify them - logger.info("Using cookies to reload session for {} ", email); + logger.info("Using cookies to reload session for {} ", email.replaceAll("[\\n\\r]", "_")); if (sessionBusiness.validateSession(email, sessionId)) { return setUserInSession(email, request.getSession());