From 6e626ed3bf92897f29042b1b9bd66d9dd6fa10ed Mon Sep 17 00:00:00 2001 From: Marvin Schenkel Date: Sat, 17 Jan 2026 17:04:21 +0100 Subject: [PATCH] Maintenance for security related fixes (#2983) --- .github/workflows/release.yml | 3 +++ .github/workflows/test.yml | 3 +++ music_assistant/controllers/media/playlists.py | 3 ++- music_assistant/controllers/metadata.py | 6 +++++- music_assistant/helpers/audio.py | 6 ++++++ music_assistant/helpers/images.py | 5 +++-- music_assistant/helpers/security.py | 16 ++++++++++++++++ 7 files changed, 38 insertions(+), 4 deletions(-) create mode 100644 music_assistant/helpers/security.py diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 530adbc1..31f69dd0 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -44,6 +44,9 @@ env: BASE_IMAGE_VERSION_BETA: "1.4.11" BASE_IMAGE_VERSION_NIGHTLY: "1.4.11" +permissions: + contents: read + jobs: determine-branch: name: Determine release branch diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 0a074fd1..11c8fc0c 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -20,6 +20,9 @@ on: required: false type: string +permissions: + contents: read + jobs: lint: runs-on: ubuntu-latest diff --git a/music_assistant/controllers/media/playlists.py b/music_assistant/controllers/media/playlists.py index 528fe032..ce34c0b1 100644 --- a/music_assistant/controllers/media/playlists.py +++ b/music_assistant/controllers/media/playlists.py @@ -18,6 +18,7 @@ from music_assistant.constants import DB_TABLE_PLAYLISTS from music_assistant.helpers.compare import create_safe_string from music_assistant.helpers.database import UNSET from music_assistant.helpers.json import serialize_to_json +from music_assistant.helpers.security import is_safe_name from music_assistant.helpers.uri import create_uri, parse_uri from music_assistant.helpers.util import guard_single_request from music_assistant.models.music_provider import MusicProvider @@ -116,7 +117,7 @@ class PlaylistController(MediaControllerBase[Playlist]): # grab all existing track ids in the playlist so we can check for duplicates provider = cast("MusicProvider", provider) - if "/" in name or "\\" in name or ".." in name: + if not is_safe_name(name): msg = f"{name} is not a valid Playlist name" raise InvalidDataError(msg) # create playlist on the provider diff --git a/music_assistant/controllers/metadata.py b/music_assistant/controllers/metadata.py index 89c863ba..e50b4db8 100644 --- a/music_assistant/controllers/metadata.py +++ b/music_assistant/controllers/metadata.py @@ -52,6 +52,7 @@ from music_assistant.constants import ( from music_assistant.helpers.api import api_command from music_assistant.helpers.compare import compare_strings from music_assistant.helpers.images import create_collage, get_image_thumb +from music_assistant.helpers.security import is_safe_path from music_assistant.helpers.throttle_retry import Throttler from music_assistant.models.core_controller import CoreController from music_assistant.models.music_provider import MusicProvider @@ -428,7 +429,10 @@ class MetaDataController(CoreController): image_format = "png" if path.lower().endswith(".png") else "jpg" if provider == "builtin" and path.startswith("/collage/"): # special case for collage images - path = os.path.join(self._collage_images_dir, path.split("/collage/")[-1]) + collage_rel = path.split("/collage/")[-1] + if not is_safe_path(collage_rel): + raise FileNotFoundError("Invalid collage path") + path = os.path.join(self._collage_images_dir, collage_rel) thumbnail_bytes = await get_image_thumb( self.mass, path, size=size, provider=provider, image_format=image_format ) diff --git a/music_assistant/helpers/audio.py b/music_assistant/helpers/audio.py index d9824ac0..d8879481 100644 --- a/music_assistant/helpers/audio.py +++ b/music_assistant/helpers/audio.py @@ -1202,6 +1202,12 @@ async def get_preview_stream( raise ProviderUnavailableError if TYPE_CHECKING: # avoid circular import assert isinstance(music_prov, MusicProvider) + + # Validate that item_id corresponds to a valid item in the provider for security + if not await music_prov.get_item(media_type, item_id): + msg = f"Item {item_id} not found in provider {provider_instance_id_or_domain}" + raise MediaNotFoundError(msg) + streamdetails = await music_prov.get_stream_details(item_id, media_type) pcm_format = AudioFormat( content_type=ContentType.from_bit_depth(streamdetails.audio_format.bit_depth), diff --git a/music_assistant/helpers/images.py b/music_assistant/helpers/images.py index 97639ffa..dca001a5 100644 --- a/music_assistant/helpers/images.py +++ b/music_assistant/helpers/images.py @@ -15,6 +15,7 @@ import aiofiles from aiohttp.client_exceptions import ClientError from PIL import Image, UnidentifiedImageError +from music_assistant.helpers.security import is_safe_path from music_assistant.helpers.tags import get_embedded_image from music_assistant.models.metadata_provider import MetadataProvider from music_assistant.models.music_provider import MusicProvider @@ -48,12 +49,12 @@ async def get_image_data(mass: MusicAssistant, path_or_url: str, provider: str) if path_or_url.startswith("data:image"): return b64decode(path_or_url.split(",")[-1]) # handle FILE location (of type image) - if path_or_url.endswith(("jpg", "JPG", "png", "PNG", "jpeg")): + if path_or_url.endswith(("jpg", "JPG", "png", "PNG", "jpeg")) and is_safe_path(path_or_url): if await asyncio.to_thread(os.path.isfile, path_or_url): async with aiofiles.open(path_or_url, "rb") as _file: return cast("bytes", await _file.read()) # use ffmpeg for embedded images - if img_data := await get_embedded_image(path_or_url): + if is_safe_path(path_or_url) and (img_data := await get_embedded_image(path_or_url)): return img_data msg = f"Image not found: {path_or_url}" raise FileNotFoundError(msg) diff --git a/music_assistant/helpers/security.py b/music_assistant/helpers/security.py new file mode 100644 index 00000000..cbfb0ff3 --- /dev/null +++ b/music_assistant/helpers/security.py @@ -0,0 +1,16 @@ +"""Security utilities for input validation.""" + +from __future__ import annotations + +import os + + +def is_safe_path(path: str) -> bool: + """Check if path is free from path traversal components.""" + norm_path = os.path.normpath(path) + return not (norm_path.startswith("..") or "/../" in norm_path or "\\..\\" in norm_path) + + +def is_safe_name(name: str) -> bool: + """Check if name is safe for use (no path separators or traversal components).""" + return not ("/" in name or "\\" in name or ".." in name) -- 2.34.1