Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 46 additions & 8 deletions acestep/api_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -894,13 +894,33 @@ def bool(self, name: str, default: bool = False) -> bool:
def _validate_audio_path(path: Optional[str]) -> Optional[str]:
"""Validate a user-supplied audio file path to prevent path traversal attacks.
Rejects absolute paths and paths containing '..' traversal sequences.
Returns the validated path or None if the input is None/empty.
Accepts absolute paths strictly only if they are within the system temporary directory.
Otherwise, rejects absolute paths and paths containing '..' traversal sequences.
Returns the validated, normalized path or None if the input is None/empty.
Raises HTTPException 400 if the path is unsafe.
"""
if not path:
return None
# Reject absolute paths (Unix and Windows)

# Resolve requested path and system temp path to normalized absolute forms
import tempfile
system_temp = os.path.realpath(tempfile.gettempdir())
requested_path = os.path.realpath(path)

# SECURE CHECK: Use os.path.commonpath to verify directory boundary integrity.
# This prevents prefix bypasses (e.g., /tmp_evil when /tmp is allowed).
try:
is_in_temp = os.path.commonpath([system_temp, requested_path]) == system_temp
except ValueError:
# Occurs on Windows if paths are on different drives
is_in_temp = False

if is_in_temp:
# Accept server-generated files in temp
return requested_path
Comment on lines +905 to +921
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# Check module-level imports and the import at line 907
head -50 acestep/api_server.py | grep -n "import"

Repository: ace-step/ACE-Step-1.5

Length of output: 1025


🏁 Script executed:

# Check around line 26 for tempfile import and line 907
sed -n '20,30p' acestep/api_server.py
sed -n '900,925p' acestep/api_server.py

Repository: ace-step/ACE-Step-1.5

Length of output: 1322


🏁 Script executed:

# Search for how temp files are created and named in the codebase
rg -n "mkstemp|mkdtemp|NamedTemporaryFile|tempfile\." --type py -B2 -A2

Repository: ace-step/ACE-Step-1.5

Length of output: 2437


Security concern: User-controlled absolute paths in temp directory bypass per-session validation.

The updated logic now accepts any absolute path within the system temp directory, including paths that users supply directly. While tempfile.mkstemp() generates cryptographically random file names (making enumeration impractical), the function does not verify that a user-provided path actually corresponds to a legitimate server-generated file for the current request/session. This allows potential cross-request file access if an attacker successfully predicts or enumerates temp file names.

Recommended mitigations:

  1. Maintain a whitelist of temp files created for the current request/session and validate against it before accepting user-provided temp paths
  2. Only accept absolute temp paths that were generated by the server, not user-supplied strings
  3. Document the security boundary clearly if accepting all temp paths is intentional

Additionally, remove the redundant import tempfile at line 907 (tempfile is already imported at module level on line 26).

🔧 Proposed fix for redundant import
    if not path:
        return None
-    
-    # Resolve requested path and system temp path to normalized absolute forms
-    import tempfile
+
+    # Resolve requested path and system temp path to normalized absolute forms
     system_temp = os.path.realpath(tempfile.gettempdir())
🤖 Prompt for AI Agents
In `@acestep/api_server.py` around lines 905 - 921, The current temp-path
acceptance logic in the code around the path resolution (use of system_temp and
requested_path and is_in_temp) accepts any path under the system temp dir and
thereby allows user-supplied absolute paths; change it to only accept temp paths
that the server actually created for the current request/session by maintaining
and checking a per-request/per-session whitelist (e.g., store filenames/paths
returned by tempfile.mkstemp() when creating files and validate requested_path
against that whitelist) and reject any absolute temp paths not present in that
whitelist; also remove the redundant local import of tempfile (it’s already
imported at module level) and keep the os.path.commonpath check only as a
secondary guard after whitelist validation.


# Reject manual absolute paths outside of temp
if os.path.isabs(path):
raise HTTPException(status_code=400, detail="absolute audio file paths are not allowed")
# Reject path traversal via '..' components
Expand Down Expand Up @@ -2220,8 +2240,8 @@ def _build_request(p: RequestParser, **kwargs) -> GenerateMusicRequest:
repainting_end=p.float("repainting_end"),
instruction=p.str("instruction", DEFAULT_DIT_INSTRUCTION),
audio_cover_strength=p.float("audio_cover_strength", 1.0),
reference_audio_path=_validate_audio_path(ref_audio),
src_audio_path=_validate_audio_path(src_audio),
reference_audio_path=ref_audio,
src_audio_path=src_audio,
task_type=p.str("task_type", "text2music"),
use_adg=p.bool("use_adg"),
cfg_interval_start=p.float("cfg_interval_start", 0.0),
Expand Down Expand Up @@ -2252,14 +2272,27 @@ def _build_request(p: RequestParser, **kwargs) -> GenerateMusicRequest:
if not isinstance(body, dict):
raise HTTPException(status_code=400, detail="JSON payload must be an object")
verify_token_from_request(body, authorization)
req = _build_request(RequestParser(body))

# Explicitly validate manual string paths from JSON input
p = RequestParser(body)
req = _build_request(
p,
reference_audio_path=_validate_audio_path(p.str("reference_audio_path") or None),
src_audio_path=_validate_audio_path(p.str("src_audio_path") or None)
)

elif content_type.endswith("+json"):
body = await request.json()
if not isinstance(body, dict):
raise HTTPException(status_code=400, detail="JSON payload must be an object")
verify_token_from_request(body, authorization)
req = _build_request(RequestParser(body))

p = RequestParser(body)
req = _build_request(
p,
reference_audio_path=_validate_audio_path(p.str("reference_audio_path") or None),
src_audio_path=_validate_audio_path(p.str("src_audio_path") or None)
)

elif content_type.startswith("multipart/form-data"):
form = await request.form()
Expand Down Expand Up @@ -2312,7 +2345,12 @@ def _build_request(p: RequestParser, **kwargs) -> GenerateMusicRequest:
body = json.loads(raw.decode("utf-8"))
if isinstance(body, dict):
verify_token_from_request(body, authorization)
req = _build_request(RequestParser(body))
p = RequestParser(body)
req = _build_request(
p,
reference_audio_path=_validate_audio_path(p.str("reference_audio_path") or None),
src_audio_path=_validate_audio_path(p.str("src_audio_path") or None)
)
else:
raise HTTPException(status_code=400, detail="JSON payload must be an object")
except HTTPException:
Expand Down