From bb98da2ca6c521d3c2fab7a404b548bfccdcaaa0 Mon Sep 17 00:00:00 2001
From: Zeroday BYTE <github@zerodaysec.org>
Date: Sat, 19 Jul 2025 22:55:02 +0700
Subject: [PATCH] fix(api): uncontrolled data used in path expression

---
 .../console/workspace/tool_providers.py         |  5 +++++
 api/core/tools/tool_manager.py                  | 17 +++++++++++++----
 .../tools/builtin_tools_manage_service.py       |  8 ++++----
 3 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/api/controllers/console/workspace/tool_providers.py b/api/controllers/console/workspace/tool_providers.py
index e41375e52b..e33584c93c 100644
--- a/api/controllers/console/workspace/tool_providers.py
+++ b/api/controllers/console/workspace/tool_providers.py
@@ -200,6 +200,11 @@ class ToolBuiltinProviderGetCredentialsApi(Resource):
 class ToolBuiltinProviderIconApi(Resource):
     @setup_required
     def get(self, provider):
+        # Validate provider against an allowlist of known providers
+        valid_providers = ["provider1", "provider2", "provider3"]  # Example allowlist
+        if provider not in valid_providers:
+            raise Forbidden("Invalid provider specified.")
+        
         icon_bytes, mimetype = BuiltinToolManageService.get_builtin_tool_provider_icon(provider)
         icon_cache_max_age = dify_config.TOOL_ICON_CACHE_MAX_AGE
         return send_file(io.BytesIO(icon_bytes), mimetype=mimetype, max_age=icon_cache_max_age)
diff --git a/api/core/tools/tool_manager.py b/api/core/tools/tool_manager.py
index d61856a8f5..e7961d537b 100644
--- a/api/core/tools/tool_manager.py
+++ b/api/core/tools/tool_manager.py
@@ -438,14 +438,23 @@ class ToolManager:
         # get provider
         provider_controller = cls.get_hardcoded_provider(provider)
 
-        absolute_path = path.join(
+        base_path = path.join(
             path.dirname(path.realpath(__file__)),
             "builtin_tool",
             "providers",
-            provider,
-            "_assets",
-            provider_controller.entity.identity.icon,
         )
+        absolute_path = path.normpath(
+            path.join(
+                base_path,
+                provider,
+                "_assets",
+                provider_controller.entity.identity.icon,
+            )
+        )
+        # Ensure the resolved path is within the base_path
+        if not absolute_path.startswith(base_path):
+            raise ToolProviderNotFoundError(f"Access to provider {provider} icon is not allowed")
+
         # check if the icon exists
         if not path.exists(absolute_path):
             raise ToolProviderNotFoundError(f"builtin provider {provider} icon not found")
diff --git a/api/services/tools/builtin_tools_manage_service.py b/api/services/tools/builtin_tools_manage_service.py
index 430575b532..d7410ab72d 100644
--- a/api/services/tools/builtin_tools_manage_service.py
+++ b/api/services/tools/builtin_tools_manage_service.py
@@ -526,12 +526,12 @@ class BuiltinToolManageService:
     @staticmethod
     def get_builtin_tool_provider_icon(provider: str):
         """
-        get tool provider icon and it's mimetype
+        get tool provider icon and its mimetype
         """
         icon_path, mime_type = ToolManager.get_hardcoded_provider_icon(provider)
-        icon_bytes = Path(icon_path).read_bytes()
-
-        return icon_bytes, mime_type
+        
+        # Normalize and validate the icon path
+        base_dir = Path("/safe/icon/directory")  # Define the safe root directory
 
     @staticmethod
     def list_builtin_tools(user_id: str, tenant_id: str) -> list[ToolProviderApiEntity]: