[5.0] Misc fix backport

src/WebAppDIRAC/WebApp/handler/ConfigurationManagerHandler.py

@@ -1,4 +1,5 @@
 import json
+import tornado.escape
 import datetime
 
 from diraccfg import CFG
@@ -517,7 +518,8 @@ def __generateHTMLDiff(self, diffGen):
                 diffList.append(("", diffLine[1:], diffLine[1:], lineNumber))
                 lineNumber = lineNumber + 1
 
-        return {"diff": diffList, "lines": linesDiffList, "totalLines": lineNumber}
+        escList = [(k, tornado.escape.xhtml_escape(a), tornado.escape.xhtml_escape(b), n) for (k, a, b, n) in diffList]
+        return {"diff": escList, "lines": linesDiffList, "totalLines": lineNumber}
 
     def __showCurrentDiff(self):
         if not self.__authorizeAction():

src/WebAppDIRAC/WebApp/handler/FileCatalogHandler.py

@@ -1,6 +1,5 @@
 import os
-import time
-import random
+import tempfile
 import shutil
 import zipfile
 import datetime
@@ -19,6 +18,9 @@ class FileCatalogHandler(WebHandler):
     # Supported operands
     __operands = ["in", "nin", "=", "!=", ">=", "<=", ">", "<"]
 
+    # Store temp directory path for validating archivePath in second pass
+    __tempDir = None
+
     def initializeRequest(self):
         """Called at every request, may be overwritten in your handler."""
         self.user = self.getUserName()
@@ -39,10 +41,10 @@ def web_getSelectedFiles(self, archivePath=None, path=None):
 
         # First pass: download files and check for the success
         if not archivePath:
-            tmpdir = "/tmp/" + str(time.time()) + str(random.random())
+            # Use tempfile.mkdtemp() for secure temporary directory creation
+            tmpdir = tempfile.mkdtemp(prefix="DIRAC_FC_")
+            FileCatalogHandler.__tempDir = tmpdir
             dataMgr = DataManager(vo=self.vo)
-            if not os.path.isdir(tmpdir):
-                os.makedirs(tmpdir)
             os.chdir(tmpdir)
             for lfn in path.split(","):
                 gLogger.always(f"Data manager get file {lfn}")
@@ -54,17 +56,19 @@ def web_getSelectedFiles(self, archivePath=None, path=None):
                 pathInZip = lfn[pos_relative:last_slash]
                 tmpPathInZip = tmpdir + pathInZip
                 gLogger.always(f"path in zip {tmpPathInZip}")
-                if not os.path.isdir(tmpPathInZip):
-                    os.makedirs(tmpPathInZip)
+                if ".." in pathInZip.split(os.sep):
+                    shutil.rmtree(tmpdir, ignore_errors=True)
+                    return {"success": "false", "error": "Invalid path", "lfn": lfn}
+                os.makedirs(tmpPathInZip, exist_ok=True)
                 result = dataMgr.getFile(str(lfn), destinationDir=str(tmpPathInZip))
                 if not result["OK"]:
                     gLogger.error("Error getting while getting files", result["Message"])
-                    shutil.rmtree(tmpdir)
+                    shutil.rmtree(tmpdir, ignore_errors=True)
                     return {"success": "false", "error": result["Message"], "lfn": lfn}
 
             # make zip file
             zipname = tmpdir.split("/")[-1] + ".zip"
-            archivePath = "/tmp/" + zipname
+            archivePath = os.path.join(tempfile.gettempdir(), zipname)
             zFile = zipfile.ZipFile(archivePath, "w")
             gLogger.always("zip file", archivePath)
             gLogger.always("start walk in tmpdir", tmpdir)
@@ -82,16 +86,32 @@ def web_getSelectedFiles(self, archivePath=None, path=None):
                     gLogger.always(f"relativePath {relativePath}, file {filename}")
                     zFile.write(os.path.join(absolutePath, filename))
             zFile.close()
-            shutil.rmtree(tmpdir)
+            shutil.rmtree(tmpdir, ignore_errors=True)
             return {"success": "true", "archivePath": archivePath}
 
         # Second pass: deliver the requested archive
+        # Validate archivePath to prevent path traversal attacks
+        realArchivePath = os.path.realpath(archivePath)
+        tempDir = tempfile.gettempdir()
+        base = os.path.basename(realArchivePath)
+        if realArchivePath != os.path.join(tempDir, base) or not (
+            base.startswith("DIRAC_FC_") and base.endswith(".zip")
+        ):
+            gLogger.error(f"Path traversal attempt detected: {archivePath}")
+            return {"success": "false", "error": "Invalid archive path"}
+
+        if not os.path.isfile(realArchivePath):
+            return {"success": "false", "error": "Archive file not found"}
+
         # read zip file
-        with open(archivePath, "rb") as archive:
+        with open(realArchivePath, "rb") as archive:
             data = archive.read()
         # cleanup
-        os.remove(archivePath)
-        return FileResponse(data, os.path.basename(archivePath))
+        try:
+            os.remove(realArchivePath)
+        except OSError:
+            pass
+        return FileResponse(data, os.path.basename(realArchivePath))
 
     def web_getMetadataFields(self):
         """Method to read all the available fields possible for defining a query

src/WebAppDIRAC/WebApp/handler/JobLaunchpadHandler.py

@@ -180,9 +180,12 @@ def web_jobSubmit(self):
             storePath = tempfile.mkdtemp(prefix="DIRAC_")
             try:
                 for fileObj in store:
-                    name = os.path.join(storePath, fileObj.filename.lstrip(os.sep))
+                    basename = os.path.basename(fileObj.filename)
+                    if not basename or basename in (".", ".."):
+                        raise ValueError(f"Invalid upload filename: {fileObj.filename!r}")
+                    name = os.path.join(storePath, basename)
 
-                    tFile = open(name, "w")
+                    tFile = open(name, "xb")
                     tFile.write(fileObj.body)
                     tFile.close()
 

src/WebAppDIRAC/WebApp/handler/JobMonitorHandler.py

@@ -1,5 +1,6 @@
 import json
 import datetime
+from urllib.parse import quote
 
 from DIRAC import gConfig, gLogger
 from DIRAC.Core.Utilities import DictCache
@@ -469,10 +470,13 @@ def web_getSandbox(self, jobID: int = None, sandbox: str = "Output", check=None)
         if check:
             return {"success": "true"}
 
+        if sandbox not in ("Input", "Output", "Check"):
+            return {"success": "false", "error": "Invalid sandbox type"}
+
         data = result["Value"]
         fname = f"{str(jobID)}_{sandbox}Sandbox.tar"
         self.set_header("Content-type", "application/x-tar")
-        self.set_header("Content-Disposition", f'attachment; filename="{fname}"')
+        self.set_header("Content-Disposition", f"attachment; filename*=UTF-8''{quote(fname)}")
         self.set_header("Content-Length", len(data))
         self.set_header("Cache-Control", "no-cache, no-store, must-revalidate, max-age=0")
         self.set_header("Pragma", "no-cache")

src/WebAppDIRAC/WebApp/handler/ProxyUploadHandler.py

@@ -105,12 +105,37 @@ def web_proxyUpload(self, pass_p12=None):
             tmp = "".join(random.choice(string.ascii_letters) for x in range(10))
             keyDict[j] = os.path.join(storePath, tmp)
 
-        cmdCert = f"openssl pkcs12 -clcerts -nokeys -in {name} -out {keyDict['pub']} -password file:{p12}"
-        cmdKey = f"openssl pkcs12 -nocerts -in {name} -out {keyDict['private']} -passout file:{keyDict['pem']} -password file:{p12}"
+        # Use subprocess directly with argument lists to prevent command injection
+        # shellCall interprets the command through a shell, which is dangerous
+        cmdCert = [
+            "openssl",
+            "pkcs12",
+            "-clcerts",
+            "-nokeys",
+            "-in",
+            name,
+            "-out",
+            keyDict["pub"],
+            "-passin",
+            f"file:{p12}",
+        ]
+        cmdKey = [
+            "openssl",
+            "pkcs12",
+            "-nocerts",
+            "-in",
+            name,
+            "-out",
+            keyDict["private"],
+            "-passout",
+            f"file:{keyDict['pem']}",
+            "-passin",
+            f"file:{p12}",
+        ]
 
         for cmd in cmdCert, cmdKey:
-            result = Subprocess.shellCall(900, cmd)
-            gLogger.debug(f"Command is: {cmd}")
+            result = Subprocess.systemCall(900, cmd)
+            gLogger.debug(f"Command is: {' '.join(cmd)}")
             gLogger.debug(f"Result is: {result}")
             if not result["OK"]:
                 shutil.rmtree(storePath)

src/WebAppDIRAC/WebApp/handler/RootHandler.py

@@ -2,7 +2,7 @@
 import pprint
 from urllib.parse import urlparse
 
-from tornado.escape import xhtml_escape
+from tornado.escape import xhtml_escape, json_encode
 from tornado import template
 
 from DIRAC import gLogger
@@ -39,6 +39,24 @@ def __change(self, setup: str = None, group: str = None) -> str:
         query = ((ref := self.request.headers.get("Referer")) and urlparse(ref).query) or ""
         return f"/{root}/s:{setup}/g:{group}/?{query}"
 
+    def __validateNextURL(self, url):
+        """Validate redirect URL to prevent open redirect and XSS attacks.
+
+        Only allows URLs that start with the configured root URL path.
+        """
+        rootURL = Conf.rootURL().strip("/")
+        parsed = urlparse(url)
+        # Reject URLs with schemes (javascript:, data:, etc.)
+        if parsed.scheme and parsed.scheme.lower() not in ("http", "https"):
+            return rootURL + "/"
+        # Reject URLs with different netloc (different host)
+        if parsed.netloc:
+            return rootURL + "/"
+        # Ensure path starts with root URL
+        if not parsed.path.startswith(rootURL):
+            return rootURL + "/"
+        return url
+
     def web_getConfigData(self, **kwargs) -> dict:
         """Get session data"""
         return self.getSessionData()
@@ -68,6 +86,9 @@ def web_login(self, provider, next="/DIRAC", **kwargs):
 
         :return: TornadoResponse()
         """
+        # Validate next URL to prevent open redirect
+        next = self.__validateNextURL(next)
+
         if not (result := self._idps.getIdProvider("DIRACWeb"))["OK"]:
             raise WErr(500, result["Message"])
         cli = result["Value"]
@@ -84,10 +105,10 @@ def web_login(self, provider, next="/DIRAC", **kwargs):
 
         resp = TornadoResponse()
         # pylint: disable=no-member
-        resp.set_secure_cookie("webauth_session", json.dumps(session), secure=True, httponly=True)
+        resp.set_secure_cookie("webauth_session", json.dumps(session), secure=True, httponly=True, samesite="Lax")
 
         # Redirect to authorization server
-        resp.set_cookie("authGrant", "Visitor")  # pylint: disable=no-member
+        resp.set_cookie("authGrant", "Visitor", samesite="Lax")  # pylint: disable=no-member
         return resp.redirect(uri)  # pylint: disable=no-member
 
     def web_loginComplete(self, code: str, state: str, **kwargs):
@@ -103,10 +124,10 @@ def web_loginComplete(self, code: str, state: str, **kwargs):
         <meta charset="utf-8" />
       </head>
       <body>
-        {{message}}
+        {{ message }}
         <script>
-          sessionStorage.setItem("access_token", "{{access_token}}");
-          window.location = "{{next}}";
+          sessionStorage.setItem("access_token", {% raw json_encode(access_token) %});
+          window.location = {% raw json_encode(next) %};
         </script>
       </body>
     </html>"""
@@ -116,16 +137,23 @@ def web_loginComplete(self, code: str, state: str, **kwargs):
             return resp.redirect("/")  # pylint: disable=no-member
         authSession = json.loads(authSession)
 
+        # Validate next URL to prevent open redirect and XSS
+        nextURL = self.__validateNextURL(authSession.get("next", "/DIRAC"))
+
         if not (result := self._idps.getIdProvider("DIRACWeb"))["OK"]:
             # pylint: disable=no-member
-            resp.finish(t.generate(next=authSession["next"], access_token="", message=result["Message"]).decode())
+            resp.finish(
+                t.generate(next=nextURL, access_token="", message=result["Message"], json_encode=json_encode).decode()
+            )
             return resp
         cli = result["Value"]
 
         result = cli.fetchToken(authorization_response=self.request.uri, code_verifier=authSession.get("code_verifier"))
         if not result["OK"]:
             # pylint: disable=no-member
-            resp.finish(t.generate(next=authSession["next"], access_token="", message=result["Message"]).decode())
+            resp.finish(
+                t.generate(next=nextURL, access_token="", message=result["Message"], json_encode=json_encode).decode()
+            )
             return resp
         token = result["Value"]
 
@@ -135,27 +163,26 @@ def web_loginComplete(self, code: str, state: str, **kwargs):
         # Create session to work through portal
         self.log.debug("Tokens received:\n", pprint.pformat(token))
         # pylint: disable=no-member
-        resp.set_secure_cookie("session_id", json.dumps(dict(token)), secure=True, httponly=True)
-        resp.set_cookie("authGrant", "Session")  # pylint: disable=no-member
+        resp.set_secure_cookie("session_id", json.dumps(dict(token)), secure=True, httponly=True, samesite="Lax")
+        resp.set_cookie("authGrant", "Session", samesite="Lax")  # pylint: disable=no-member
 
         if not (result := cli.researchGroup())["OK"]:
             # pylint: disable=no-member
             return resp.finish(
-                t.generate(next=authSession["next"], access_token="", message=result["Message"]).decode()
+                t.generate(next=nextURL, access_token="", message=result["Message"], json_encode=json_encode).decode()
             )
-        self.request.headers["Referer"] = authSession["next"]
-        nextURL = self.__change(group=result["Value"].get("group"))
+        self.request.headers["Referer"] = nextURL
+        redirectURL = self.__change(group=result["Value"].get("group"))
 
         # Save token and go to main page
-        # with document('DIRAC authentication') as html:
-        #   dom.div('Authorization is done.',
-        #           style='display:flex;justify-content:center;align-items:center;padding:28px;font-size:28px;')
-        #   dom.script("sessionStorage.setItem('access_token','%s');window.location='%s'" % (access_token, nextURL),
-        #              type="text/javascript")
-        # return template.Template(html.render()).generate()
         # pylint: disable=no-member
         resp.finish(
-            t.generate(next=nextURL, access_token=token["access_token"], message="Authorization is done").decode()
+            t.generate(
+                next=redirectURL,
+                access_token=token["access_token"],
+                message="Authorization is done",
+                json_encode=json_encode,
+            ).decode()
         )
         return resp
 
@@ -175,7 +202,40 @@ def web_index(self, *, url_state="", theme="", open_app="", **kwargs):
         if welcomeFile:
             try:
                 with open(welcomeFile) as f:
-                    welcome = f.read().replace("\n", "")
+                    rawWelcome = f.read().replace("\n", "")
+                # Sanitize HTML to prevent XSS - template has autoescape disabled
+                try:
+                    import bleach
+
+                    welcome = bleach.clean(
+                        rawWelcome,
+                        tags=[
+                            "b",
+                            "i",
+                            "u",
+                            "em",
+                            "strong",
+                            "p",
+                            "br",
+                            "a",
+                            "ul",
+                            "ol",
+                            "li",
+                            "h1",
+                            "h2",
+                            "h3",
+                            "h4",
+                            "h5",
+                            "h6",
+                            "img",
+                            "div",
+                            "span",
+                        ],
+                        attributes={"a": ["href", "title"], "img": ["src", "alt", "width", "height"]},
+                    )
+                except ImportError:
+                    # Fallback: escape all HTML if bleach not available
+                    welcome = xhtml_escape(rawWelcome)
             except Exception:
                 gLogger.warn(f"Welcome page not found here: {welcomeFile}")
 

src/WebAppDIRAC/WebApp/handler/SystemAdministrationHandler.py

@@ -4,6 +4,8 @@
 import json
 
 from DIRAC import gConfig, gLogger, S_ERROR
+from DIRAC.Core.Security import Properties
+from DIRAC.Core.Tornado.Server.private.BaseRequestHandler import authorization
 from DIRAC.Core.Utilities.List import uniqueElements
 from DIRAC.FrameworkSystem.Client.NotificationClient import NotificationClient
 from DIRAC.FrameworkSystem.Client.SystemAdministratorClient import SystemAdministratorClient
@@ -122,9 +124,11 @@ def web_getHostLog(self, host=None, system=None, component=None):
 
         return {"success": "true", "result": log.replace("\n", "<br>")}
 
+    @authorization([Properties.CS_ADMINISTRATOR, Properties.SERVICE_ADMINISTRATOR])
     def web_hostAction(self, host=None, action=None, version=None):
         """
         Restart all DIRAC components on a given host
+        Requires CSAdministrator or ServiceAdministrator property
         """
 
         if not host:
@@ -166,10 +170,12 @@ def web_hostAction(self, host=None, action=None, version=None):
 
         return self.aftermath(actionSuccess, actionFailed, action, "Host")
 
+    @authorization([Properties.CS_ADMINISTRATOR, Properties.SERVICE_ADMINISTRATOR])
     def web_componentAction(self, action=None, **kwargs):
         """
         Actions which should be done on components. The only parameters is an action
         to perform.
+        Requires CSAdministrator or ServiceAdministrator property
         Returns standard JSON response structure with with service response
         or error messages
         """