updates with the eskemm proposition

vip-api/src/main/java/fr/insalyon/creatis/vip/api/business/ApiBusiness.java

@@ -21,27 +21,27 @@
     private final Logger logger = LoggerFactory.getLogger(getClass());
 
     private final Server server;
     private final AuthenticationBusiness authenticationBusiness;
     private final UserBusiness userBusiness;
 
     @Autowired
     public ApiBusiness(final Server server, final AuthenticationBusiness authenticationBusiness, final UserBusiness userBusiness) {
         this.server = server;
         this.authenticationBusiness = authenticationBusiness;
         this.userBusiness = userBusiness;
     }
 
     public AuthenticationInfo authenticate(AuthenticationCredentials authCreds) throws VipException {
         return authenticate(authCreds, false);
     }
 
     public AuthenticationInfo authenticateSession(AuthenticationCredentials authCreds) throws VipException {
         return authenticate(authCreds, true);
     }
 
     public AuthenticationInfo authenticate(AuthenticationCredentials authCreds, boolean initSession)
             throws VipException {
         String username = authCreds.getUsername(), password = authCreds.getPassword();
         logger.debug("Verifying credential for " + username);
         User user = signin(username, password);
         logger.debug("Constructing authentication info for " + username);
@@ -61,7 +61,7 @@
         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 @@
         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);

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]", "_"));
     }
 }

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());
     }
 

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]", "_"));
         }
 
     }

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<String> output = runCommandAndFailOnError(command);
-
+            List<String> commandArgs = Arrays.asList("bosh", "publish", server.getPublicationCommandLine());
+            List<String> 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<String> command = Arrays.asList("bosh", "validate", localPath);
         try {
             // if no exception : the command was successful
             runCommand(command);
@@ -188,22 +189,22 @@ public List<String> getCout() {
         }
     }
 
-    private List<String> runCommandAndFailOnError(String command) throws VipException {
+    private List<String> runCommandAndFailOnError(List<String> 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<String> runCommand(String command) throws CommandErrorException, VipException {
-        ProcessBuilder builder = new ProcessBuilder("bash", "-c", command);
+    private List<String> runCommand(List<String> commandArgs) throws CommandErrorException, VipException {
+        ProcessBuilder builder = new ProcessBuilder(commandArgs);
         builder.redirectErrorStream(true);
         Process process = null;
         List<String> 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<String> runCommand(String command) throws CommandErrorException, Vi
         process = null;
         return cout;
     }
+    private List<String> runCommandWithEnv(List<String> 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<String> 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) {

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<Source> 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<Source>();
@@ -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);
     }
 }

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<String, Integer> getNodeCountriesMap() throws DAOException {
     }
 
     private List<String> 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<String> 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<String> list = new ArrayList<String>();
-            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<String> getHistogramTimes(int binSize, String startField, String en
             close(logger);
         }
     }
-
+    
     private int parseMinorStatus(String minorStatus) {
 
         if (minorStatus == null || minorStatus.isEmpty()) {

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());