fix(security): address SSL fallback, ZIP slip, and unbounded download vulnerabilities

astrbot/core/dashboard_assets.py

@@ -211,7 +211,7 @@ async def _download_package(
     proxy: str | None = None,
     progress_callback=None,
     extract: bool = True,
-    allow_insecure_ssl_fallback: bool = True,
+    allow_insecure_ssl_fallback: bool = False,
 ) -> None:
     """Download a Dashboard package pinned to one Core version.
 
@@ -223,7 +223,8 @@ async def _download_package(
         progress_callback: Internal download progress callback.
         extract: Whether to extract the downloaded package.
         allow_insecure_ssl_fallback: Whether certificate failures may retry with
-            TLS verification disabled.
+            TLS verification disabled. Defaults to ``False`` to avoid silent
+            man-in-the-middle attacks against the dashboard asset download.
 
     Raises:
         RuntimeError: If neither source provides a valid ZIP package.

astrbot/core/platform/sources/misskey/misskey_api.py

@@ -706,24 +706,11 @@ async def upload_and_find_file(
             import os
             import tempfile
 
-            # SSL 验证下载，失败则重试不验证 SSL
-            tmp_bytes = None
-            try:
-                tmp_bytes = await self._download_with_existing_session(
-                    url,
-                    ssl_verify=True,
-                ) or await self._download_with_temp_session(url, ssl_verify=True)
-            except Exception as ssl_error:
-                logger.debug(
-                    f"[Misskey API] SSL 验证下载失败: {ssl_error}，重试不验证 SSL",
-                )
-                try:
-                    tmp_bytes = await self._download_with_existing_session(
-                        url,
-                        ssl_verify=False,
-                    ) or await self._download_with_temp_session(url, ssl_verify=False)
-                except Exception:
-                    pass
+            # 下载文件时强制进行 TLS 校验，避免 MITM 攻击注入恶意内容。
+            tmp_bytes = await self._download_with_existing_session(
+                url,
+                ssl_verify=True,
+            ) or await self._download_with_temp_session(url, ssl_verify=True)
 
             if tmp_bytes:
                 with tempfile.NamedTemporaryFile(delete=False) as tmpf:

astrbot/core/star/updater.py

@@ -20,7 +20,7 @@
 from astrbot.core.utils.io import ensure_dir, remove_dir
 
 from ..star.star import StarMetadata
-from ..zip_updater import _RepoZipUpdater
+from ..zip_updater import _RepoZipUpdater, _zip_slip_filter
 
 PLUGIN_METADATA_FILENAMES = ("metadata.yaml", "metadata.yml")
 PLUGIN_METADATA_REQUIRED_FIELDS = ("name", "desc", "version", "author")
@@ -459,6 +459,6 @@ def _extract_plugin_archive(self, zip_path: str, target_dir: str) -> None:
         logger.info(f"Extracting archive: {zip_path}")
         with zipfile.ZipFile(zip_path, "r") as z:
             update_dir = self._resolve_archive_root_dir(z.namelist())
-            z.extractall(target_dir)
+            z.extractall(target_dir, filter=_zip_slip_filter)
 
         self._finalize_extracted_archive(zip_path, target_dir, update_dir)

astrbot/core/utils/io.py

@@ -20,6 +20,14 @@
 
 logger = logging.getLogger("astrbot")
 
+# 2 GiB safety cap for streamed HTTP downloads. Content-Length headers above this
+# threshold cause the download to abort before any bytes are written to disk.
+_DOWNLOAD_MAX_BYTES = 2 * 1024 * 1024 * 1024
+
+
+class DownloadTooLargeError(RuntimeError):
+    """Raised when an HTTP download exceeds the configured size cap."""
+
 
 def _safe_url_for_log(url: str) -> str:
     """Return a URL summary that omits query strings and fragments.
@@ -117,57 +125,47 @@ async def download_image_by_url(
     path: str | None = None,
 ) -> str:
     """下载图片, 返回 path"""
-    try:
-        ssl_context = ssl.create_default_context(
-            cafile=certifi.where(),
-        )  # 使用 certifi 提供的 CA 证书
-        connector = aiohttp.TCPConnector(ssl=ssl_context)  # 使用 certifi 的根证书
-        async with aiohttp.ClientSession(
-            trust_env=True,
-            connector=connector,
-        ) as session:
-            if post:
-                async with session.post(url, json=post_data) as resp:
-                    if not path:
-                        return save_temp_img(await resp.read())
-                    with open(path, "wb") as f:
-                        f.write(await resp.read())
-                    return path
-            else:
-                async with session.get(url) as resp:
-                    if not path:
-                        return save_temp_img(await resp.read())
-                    with open(path, "wb") as f:
-                        f.write(await resp.read())
-                    return path
-    except (aiohttp.ClientConnectorSSLError, aiohttp.ClientConnectorCertificateError):
-        # 关闭SSL验证（仅在证书验证失败时作为fallback）
-        logger.warning(
-            f"SSL certificate verification failed for {_safe_url_for_log(url)}. "
-            "Disabling SSL verification (CERT_NONE) as a fallback. "
-            "This is insecure and exposes the application to man-in-the-middle attacks. "
-            "Please investigate and resolve certificate issues."
-        )
-        ssl_context = ssl.create_default_context()
-        ssl_context.check_hostname = False
-        ssl_context.verify_mode = ssl.CERT_NONE
-        async with aiohttp.ClientSession() as session:
-            if post:
-                async with session.post(url, json=post_data, ssl=ssl_context) as resp:
-                    if not path:
-                        return save_temp_img(await resp.read())
-                    with open(path, "wb") as f:
-                        f.write(await resp.read())
-                    return path
-            else:
-                async with session.get(url, ssl=ssl_context) as resp:
-                    if not path:
-                        return save_temp_img(await resp.read())
-                    with open(path, "wb") as f:
-                        f.write(await resp.read())
-                    return path
-    except Exception as e:
-        raise e
+    ssl_context = ssl.create_default_context(
+        cafile=certifi.where(),
+    )  # 使用 certifi 提供的 CA 证书
+    connector = aiohttp.TCPConnector(ssl=ssl_context)  # 使用 certifi 的根证书
+    async with aiohttp.ClientSession(
+        trust_env=True,
+        connector=connector,
+    ) as session:
+        if post:
+            async with session.post(url, json=post_data) as resp:
+                resp.raise_for_status()
+                return await _download_image_to_dest(resp, path)
+        else:
+            async with session.get(url) as resp:
+                resp.raise_for_status()
+                return await _download_image_to_dest(resp, path)
+
+
+async def _download_image_to_dest(
+    resp: aiohttp.ClientResponse, path: str | None
+) -> str:
+    """Stream download an image with size cap, saving to path or temp file."""
+    total_read = 0
+    chunks: list[bytes] = []
+
+    async for chunk in resp.content.iter_chunked(64 * 1024):
+        if not chunk:
+            break
+        total_read += len(chunk)
+        if total_read > _DOWNLOAD_MAX_BYTES:
+            raise DownloadTooLargeError(
+                f"Image download exceeded size cap of {_DOWNLOAD_MAX_BYTES} bytes"
+            )
+        chunks.append(chunk)
+
+    data = b"".join(chunks)
+    if path:
+        with open(path, "wb") as f:
+            f.write(data)
+        return path
+    return save_temp_img(data)
 
 
 async def _emit_download_progress(progress_callback, payload: dict) -> None:
@@ -217,6 +215,11 @@ async def _download_response_to_file(
     """
 
     total_size = int(resp.headers.get("content-length", 0))
+    if total_size and total_size > _DOWNLOAD_MAX_BYTES:
+        raise DownloadTooLargeError(
+            f"Refusing download from {_safe_url_for_log(url)}: advertised size "
+            f"{total_size} bytes exceeds {_DOWNLOAD_MAX_BYTES} byte cap.",
+        )
     downloaded_size = 0
     start_time = time.time()
     if show_progress:
@@ -241,8 +244,13 @@ async def _download_response_to_file(
         chunk = await resp.content.read(8192)
         if not chunk:
             break
-        file_obj.write(chunk)
         downloaded_size += len(chunk)
+        if downloaded_size > _DOWNLOAD_MAX_BYTES:
+            raise DownloadTooLargeError(
+                f"Aborting download from {_safe_url_for_log(url)}: response "
+                f"exceeded {_DOWNLOAD_MAX_BYTES} byte cap.",
+            )
+        file_obj.write(chunk)
         elapsed_time = time.time() - start_time if time.time() - start_time > 0 else 1
         speed = downloaded_size / 1024 / elapsed_time  # KB/s
         percent = downloaded_size / total_size if total_size > 0 else 0
@@ -278,7 +286,7 @@ async def download_file(
     path: str,
     show_progress: bool = False,
     progress_callback=None,
-    allow_insecure_ssl_fallback: bool = True,
+    allow_insecure_ssl_fallback: bool = False,
 ) -> None:
     """Download a remote file to a local path.
 
@@ -288,7 +296,9 @@ async def download_file(
         show_progress: Whether to print progress to stdout.
         progress_callback: Optional callback for progress payloads.
         allow_insecure_ssl_fallback: Whether certificate failures may retry with
-            TLS certificate verification disabled.
+            TLS certificate verification disabled. Defaults to ``False`` because
+            silently disabling certificate verification enables man-in-the-middle
+            attacks against users and remote plugin/dashboard downloads.
 
     Returns:
         None.
@@ -342,6 +352,14 @@ async def download_file(
                         progress_callback,
                         show_downloading_label=False,
                     )
+    except DownloadTooLargeError:
+        # Remove the partial file written before the size cap tripped so a
+        # truncated/empty artifact is not left on disk.
+        try:
+            os.remove(path)
+        except FileNotFoundError:
+            pass
+        raise
     if show_progress:
         print()
 

astrbot/core/zip_updater.py

@@ -17,6 +17,28 @@
 __all__ = ["ReleaseInfo"]
 
 
+def _zip_slip_filter(member: zipfile.ZipInfo, path: str | bytes) -> zipfile.ZipInfo:
+    """Reject zip members that would escape the destination directory (CWE-22).
+
+    Mirrors the behaviour of ``zipfile.ZipFile.extractall(filter="data")`` but is
+    written explicitly so the safety check stays consistent across Python 3.12
+    and newer minor releases and is easy to unit test.
+    """
+
+    base = os.path.abspath(path)
+    target = os.path.abspath(os.path.join(base, member.filename))
+    try:
+        common = os.path.commonpath([base, target])
+    except ValueError:
+        # Raised when paths live on different drives on Windows.
+        common = ""
+    if common != base:
+        raise ValueError(
+            f"Refusing zip member that escapes destination: {member.filename!r}"
+        )
+    return member
+
+
 class ReleaseInfo:
     """Describe a repository release exposed by an updater.
 
@@ -315,7 +337,7 @@ def _extract_archive(self, zip_path: str, target_dir: str) -> None:
         ensure_dir(target_dir)
         with zipfile.ZipFile(zip_path, "r") as z:
             update_dir = self._resolve_archive_root_dir(z.namelist())
-            z.extractall(target_dir)
+            z.extractall(target_dir, filter=_zip_slip_filter)
         logger.debug(f"Finished extracting archive: {zip_path}")
 
         self._finalize_extracted_archive(zip_path, target_dir, update_dir)

tests/test_updater_socks.py

@@ -192,7 +192,7 @@ def read(self, name: str) -> bytes:
             )
         return b""
 
-    def extractall(self, target_dir: str) -> None:  # noqa: ARG002
+    def extractall(self, target_dir: str, filter=None) -> None:  # noqa: ARG002
         return None