CVE-2023-49796: Insufficient Information (NVD-CWE-noinfo)
value: MindsDB connects artificial intelligence models to real time data. Versions prior to 23.11.4.1 contain a limited file write vulnerability in `file.py` Users should use MindsDB's `staging` branch or v23.11.4.1, which contain a fix for the issue.
CWE description: There is insufficient information about the issue to classify it; details are unkown or unspecified.
CVSS v3 base score: 5.3
Published: 2023-12-11T21:15Z
Repository: https://github.com/mindsdb/mindsdb
Language: Python
Files changed: mindsdb/api/http/namespaces/file.py, mindsdb/integrations/handlers/llama_index_handler/llama_index_handler.py, mindsdb/integrations/handlers/web_handler/web_handler.py, mindsdb/utilities/security.py
Commit date: 2023-11-29 16:02:18 +0300
Commit message: fixes:
- SSRF
- arbitrary file write
https://github.com/mindsdb/mindsdb_private/issues/341

Diff: diff --git a/mindsdb/api/http/namespaces/file.py b/mindsdb/api/http/namespaces/file.py
index 837202fe7..e7664a6ad 100644
--- a/mindsdb/api/http/namespaces/file.py
+++ b/mindsdb/api/http/namespaces/file.py
@@ -12,10 +12,11 @@ from flask_restx import Resource
 from mindsdb.api.http.namespaces.configs.files import ns_conf
 from mindsdb.api.http.utils import http_error, safe_extract
 from mindsdb.utilities.config import Config
 from mindsdb.utilities.context import context as ctx
 from mindsdb.utilities import log
+from mindsdb.utilities.security import is_private_url, clear_filename
 
 logger = log.getLogger(__name__)
 
 
 @ns_conf.route("/")
@@ -49,11 +50,11 @@ class File(Resource):
 
         file_object = None
 
         def on_file(file):
             nonlocal file_object
-            data["file"] = file.file_name.decode()
+            data["file"] = clear_filename(file.file_name.decode())
             file_object = file.file_object
 
         temp_dir_path = tempfile.mkdtemp(prefix="mindsdb_file_")
 
         if request.headers["Content-Type"].startswith("multipart/form-data"):
@@ -89,14 +90,19 @@ class File(Resource):
                 f"File with name '{data['file']}' already exists",
             )
 
         if data.get("source_type") == "url":
             url = data["source"]
-            data["file"] = data["name"]
+            data["file"] = clear_filename(data["name"])
 
             config = Config()
             is_cloud = config.get("cloud", False)
+            if is_cloud and is_private_url(url):
+                return http_error(
+                    400, f'URL is private: {url}'
+                )
+
             if is_cloud is True and ctx.user_class != 1:
                 info = requests.head(url)
                 file_size = info.headers.get("Content-Length")
                 try:
                     file_size = int(file_size)
@@ -121,11 +127,11 @@ class File(Resource):
                 file_path = os.path.join(temp_dir_path, data["file"])
                 with open(file_path, "wb") as f:
                     for chunk in r.iter_content(chunk_size=8192):
                         f.write(chunk)
 
-        original_file_name = data.get("original_file_name")
+        original_file_name = clear_filename(data.get("original_file_name"))
 
         file_path = os.path.join(temp_dir_path, data["file"])
         lp = file_path.lower()
         if lp.endswith((".zip", ".tar.gz")):
             if lp.endswith(".zip"):
diff --git a/mindsdb/integrations/handlers/llama_index_handler/llama_index_handler.py b/mindsdb/integrations/handlers/llama_index_handler/llama_index_handler.py
index c98888a3a..901a5d641 100644
--- a/mindsdb/integrations/handlers/llama_index_handler/llama_index_handler.py
+++ b/mindsdb/integrations/handlers/llama_index_handler/llama_index_handler.py
@@ -11,10 +11,11 @@ from llama_index import ServiceContext, StorageContext, load_index_from_storage
 from llama_index import LLMPredictor, OpenAIEmbedding
 from llama_index.indices.vector_store.base import VectorStore
 
 from mindsdb.integrations.libs.base import BaseMLEngine
 from mindsdb.utilities.config import Config
+from mindsdb.utilities.security import is_private_url
 
 
 def _validate_prompt_template(prompt_template: str):
     if '{context_str}' not in prompt_template or '{query_str}' not in prompt_template:
         raise Exception(
@@ -67,11 +68,17 @@ class LlamaIndexHandler(BaseMLEngine):
 
         elif args['using']['reader'] == 'SimpleWebPageReader':
             if 'source_url_link' not in args['using']:
                 raise Exception("SimpleWebPageReader requires a `source_url_link` parameter. Refer to LlamaIndex documentation for more details.")  # noqa
 
-            reader = SimpleWebPageReader(html_to_text=True).load_data([args['using']['source_url_link']])
+            url = args['using']['source_url_link']
+            config = Config()
+            is_cloud = config.get("cloud", False)
+            if is_cloud and is_private_url(url):
+                raise Exception(f'URL is private: {url}')
+
+            reader = SimpleWebPageReader(html_to_text=True).load_data([url])
 
         else:
             raise Exception(f"Invalid operation mode. Please use one of {self.supported_reader}.")
 
         self.model_storage.json_set('args', args)
diff --git a/mindsdb/integrations/handlers/web_handler/web_handler.py b/mindsdb/integrations/handlers/web_handler/web_handler.py
index e152c8439..43faef723 100644
--- a/mindsdb/integrations/handlers/web_handler/web_handler.py
+++ b/mindsdb/integrations/handlers/web_handler/web_handler.py
@@ -8,10 +8,12 @@ from mindsdb.integrations.utilities.sql_utils import extract_comparison_conditio
 from mindsdb.integrations.libs.response import (
     HandlerStatusResponse as StatusResponse,
     HandlerResponse as Response,
     RESPONSE_TYPE
 )
+from mindsdb.utilities.security import is_private_url
+from mindsdb.utilities.config import Config
 
 from .urlcrawl_helpers import get_df_from_query_str, get_all_websites
 
 
 class CrawlerTable(APITable):
@@ -35,11 +37,13 @@ class CrawlerTable(APITable):
                         urls = [str(url)]
                     else:
                         urls = url
                 else:
                     raise NotImplementedError(
-                        f'url can be url = "someurl", you can also crawl multiple sites, as follows: url IN ("url1", "url2", ..)')
+                        f'url can be url = "someurl", you can also crawl multiple sites, as follows:'
+                        f' url IN ("url1", "url2", ..)'
+                    )
 
             else:
                 pass
 
         if len(urls) == 0:
@@ -50,11 +54,20 @@ class CrawlerTable(APITable):
             raise NotImplementedError(f'You must specify a LIMIT which defines the number of pages to crawl')
         limit = query.limit.value
 
         if limit < 0:
             limit = 0
-            
+
+        config = Config()
+        is_cloud = config.get("cloud", False)
+        if is_cloud:
+            urls = [
+                url
+                for url in urls
+                if not is_private_url(url)
+            ]
+
         result = get_all_websites(urls, limit, html=False)
         if len(result) > limit:
             result = result[:limit]
         # filter targets
         result = project_dataframe(result, query.targets, self.get_columns())
diff --git a/mindsdb/utilities/security.py b/mindsdb/utilities/security.py
new file mode 100644
index 000000000..527148084
--- /dev/null
+++ b/mindsdb/utilities/security.py
@@ -0,0 +1,33 @@
+from urllib.parse import urlparse
+import socket
+import ipaddress
+
+
+def is_private_url(url: str):
+    """
+    Raises exception if url is private
+
+    :param url: url to check
+    """
+
+    hostname = urlparse(url).hostname
+    if not hostname:
+        # Unable find hostname in url
+        return True
+    ip = socket.gethostbyname(hostname)
+    return ipaddress.ip_address(ip).is_private
+
+
+def clear_filename(filename: str):
+    """
+    Removes path symbols from filename which could be used for path injection
+    :param s:
+    :return:
+    """
+
+    if not filename:
+        return filename
+    badchars = '\\/:*?\"<>|'
+    for c in badchars:
+        filename = filename.replace(c, '')
+    return filename

Vulnerable code:             data["file"] = file.file_name.decode()
            data["file"] = data["name"]
        original_file_name = data.get("original_file_name")
            reader = SimpleWebPageReader(html_to_text=True).load_data([args['using']['source_url_link']])
                        f'url can be url = "someurl", you can also crawl multiple sites, as follows: url IN ("url1", "url2", ..)')
            
Fixed code: from mindsdb.utilities.security import is_private_url, clear_filename
            data["file"] = clear_filename(file.file_name.decode())
            data["file"] = clear_filename(data["name"])
            if is_cloud and is_private_url(url):
                return http_error(
                    400, f'URL is private: {url}'
                )

        original_file_name = clear_filename(data.get("original_file_name"))
from mindsdb.utilities.security import is_private_url
            url = args['using']['source_url_link']
            config = Config()
            is_cloud = config.get("cloud", False)
            if is_cloud and is_private_url(url):
                raise Exception(f'URL is private: {url}')

            reader = SimpleWebPageReader(html_to_text=True).load_data([url])
from mindsdb.utilities.security import is_private_url
from mindsdb.utilities.config import Config
                        f'url can be url = "someurl", you can also crawl multiple sites, as follows:'
                        f' url IN ("url1", "url2", ..)'
                    )

        config = Config()
        is_cloud = config.get("cloud", False)
        if is_cloud:
            urls = [
                url
                for url in urls
                if not is_private_url(url)
            ]

from urllib.parse import urlparse
import socket
import ipaddress


def is_private_url(url: str):
    """
    Raises exception if url is private

    :param url: url to check
    """

    hostname = urlparse(url).hostname
    if not hostname:
        # Unable find hostname in url
        return True
    ip = socket.gethostbyname(hostname)
    return ipaddress.ip_address(ip).is_private


def clear_filename(filename: str):
    """
    Removes path symbols from filename which could be used for path injection
    :param s:
    :return:
    """

    if not filename:
        return filename
    badchars = '\\/:*?\"<>|'
    for c in badchars:
        filename = filename.replace(c, '')
    return filename