Fixes security issue: potential remote Path Traversal via File Upload
This commit is contained in:
+160
-101
@@ -23,14 +23,13 @@ from app.auth import (
|
||||
)
|
||||
from app.config import settings
|
||||
from app.database import async_session, get_session
|
||||
from app.models import MobileBuildArtifact, MobileBuildRequest, MobileBuildUploadedFile, StationConfig
|
||||
from app.models import MobileBuildArtifact, MobileBuildRequest, StationConfig
|
||||
from app.schemas import (
|
||||
MobileBuildArtifactResponse,
|
||||
MobileBuildCreate,
|
||||
MobileBuildDefaultsResponse,
|
||||
MobileBuildPreflightResponse,
|
||||
MobileBuildRequestResponse,
|
||||
MobileBuildUploadedFileResponse,
|
||||
)
|
||||
from app.user_models import User
|
||||
|
||||
@@ -41,6 +40,25 @@ _ALLOWED_KINDS = {
|
||||
"ios": {"certificate", "profile"},
|
||||
}
|
||||
|
||||
_FILE_ROLE_DISPLAY = {
|
||||
"android_keystore": "Android Keystore",
|
||||
"android_descriptor": "Android Descriptor JSON",
|
||||
"ios_certificate": "iOS Certificate (.p12)",
|
||||
"ios_profile": "iOS Provisioning Profile",
|
||||
}
|
||||
|
||||
# Accepted extensions per file kind (platform → {kind → {extensions}})
|
||||
_ALLOWED_EXTENSIONS = {
|
||||
"android": {
|
||||
"keystore": {".jks", ".keystore", ".p12"},
|
||||
"descriptor": {".json"},
|
||||
},
|
||||
"ios": {
|
||||
"certificate": {".p12"},
|
||||
"profile": {".mobileprovision"},
|
||||
},
|
||||
}
|
||||
|
||||
_RUNNING_TASKS: set[int] = set()
|
||||
|
||||
|
||||
@@ -50,6 +68,26 @@ def _utc_iso(value: datetime | None) -> str | None:
|
||||
return value.replace(microsecond=0).isoformat() + "Z"
|
||||
|
||||
|
||||
def _sanitize_filename(filename: str) -> str:
|
||||
"""Remove directory traversal components."""
|
||||
return Path(filename).name
|
||||
|
||||
|
||||
def _find_uploaded_file(request_id: int, platform: str, kind: str) -> Path | None:
|
||||
"""Find an uploaded signing file in the request's upload directory.
|
||||
|
||||
Scans for a file matching {platform}_{kind}_{uuid}{suffix}.
|
||||
Returns the first matching Path, or None if not found.
|
||||
"""
|
||||
upload_dir, _ = _ensure_dirs()
|
||||
req_dir = upload_dir / f"request_{request_id}"
|
||||
if not req_dir.is_dir():
|
||||
return None
|
||||
pattern = f"{platform}_{kind}_*"
|
||||
matches = list(req_dir.glob(pattern))
|
||||
return matches[0] if matches else None
|
||||
|
||||
|
||||
def _hash_file(path: Path) -> str:
|
||||
h = hashlib.sha256()
|
||||
with path.open("rb") as f:
|
||||
@@ -66,6 +104,39 @@ def _ensure_dirs() -> tuple[Path, Path]:
|
||||
return upload_dir, artifact_dir
|
||||
|
||||
|
||||
def _validate_upload(platform: str, file_kind: str, filename: str | None, data: bytes) -> None:
|
||||
"""Validate uploaded file extension and basic content integrity."""
|
||||
# Extension check
|
||||
allowed_exts = _ALLOWED_EXTENSIONS[platform][file_kind]
|
||||
suffix = Path(filename or "").suffix.lower()
|
||||
if suffix not in allowed_exts:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_400_BAD_REQUEST,
|
||||
detail=f"File must have one of: {', '.join(sorted(allowed_exts))}",
|
||||
)
|
||||
|
||||
# Content checks
|
||||
if file_kind == "descriptor":
|
||||
try:
|
||||
import json
|
||||
json.loads(data)
|
||||
except (ValueError, UnicodeDecodeError):
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_400_BAD_REQUEST,
|
||||
detail="Android descriptor must be valid JSON.",
|
||||
)
|
||||
|
||||
if file_kind == "profile":
|
||||
# .mobileprovision is a DER-encoded PKCS#7 (signed Apple certificate).
|
||||
# Quick heuristic: it should contain readable markers like "Provision" or "plist".
|
||||
text = data.decode("latin-1", errors="ignore").lower()
|
||||
if "provision" not in text and "plist" not in text:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_400_BAD_REQUEST,
|
||||
detail="File does not appear to be a valid iOS provisioning profile.",
|
||||
)
|
||||
|
||||
|
||||
def _check_mobileprovision_for_carplay(path: Path) -> bool:
|
||||
try:
|
||||
raw = path.read_bytes()
|
||||
@@ -149,14 +220,7 @@ async def _append_log(session: AsyncSession, request: MobileBuildRequest, line:
|
||||
await session.commit()
|
||||
|
||||
|
||||
async def _load_files(session: AsyncSession, request_id: int) -> list[MobileBuildUploadedFile]:
|
||||
result = await session.execute(
|
||||
select(MobileBuildUploadedFile).where(MobileBuildUploadedFile.request_id == request_id)
|
||||
)
|
||||
return list(result.scalars().all())
|
||||
|
||||
|
||||
def _preflight_issues(request: MobileBuildRequest, files: list[MobileBuildUploadedFile]) -> list[str]:
|
||||
def _preflight_issues(request: MobileBuildRequest, request_id: int) -> list[str]:
|
||||
issues: list[str] = []
|
||||
|
||||
if not request.platform_android and not request.platform_ios:
|
||||
@@ -180,53 +244,29 @@ def _preflight_issues(request: MobileBuildRequest, files: list[MobileBuildUpload
|
||||
if request.platform_ios and not settings.MOBILE_BUILD_IOS_HOST:
|
||||
issues.append("iOS build host is not configured.")
|
||||
|
||||
grouped: dict[str, dict[str, MobileBuildUploadedFile]] = {"android": {}, "ios": {}}
|
||||
for f in files:
|
||||
grouped.setdefault(f.platform, {})[f.file_kind] = f
|
||||
|
||||
# Check for expected files on disk
|
||||
if request.platform_android:
|
||||
present = set(grouped.get("android", {}).keys())
|
||||
expected = _ALLOWED_KINDS["android"]
|
||||
if present != expected:
|
||||
missing = sorted(expected - present)
|
||||
extra = sorted(present - expected)
|
||||
if missing:
|
||||
issues.append(f"Android requires files for kinds: {', '.join(missing)}")
|
||||
if extra:
|
||||
issues.append(f"Android has unsupported file kinds: {', '.join(extra)}")
|
||||
for kind in _ALLOWED_KINDS["android"]:
|
||||
role_key = f"android_{kind}"
|
||||
display = _FILE_ROLE_DISPLAY[role_key]
|
||||
if not _find_uploaded_file(request_id, "android", kind):
|
||||
issues.append(f"Missing: {display}")
|
||||
|
||||
if request.platform_ios:
|
||||
present = set(grouped.get("ios", {}).keys())
|
||||
expected = _ALLOWED_KINDS["ios"]
|
||||
if present != expected:
|
||||
missing = sorted(expected - present)
|
||||
extra = sorted(present - expected)
|
||||
if missing:
|
||||
issues.append(f"iOS requires files for kinds: {', '.join(missing)}")
|
||||
if extra:
|
||||
issues.append(f"iOS has unsupported file kinds: {', '.join(extra)}")
|
||||
for kind in _ALLOWED_KINDS["ios"]:
|
||||
role_key = f"ios_{kind}"
|
||||
display = _FILE_ROLE_DISPLAY[role_key]
|
||||
if not _find_uploaded_file(request_id, "ios", kind):
|
||||
issues.append(f"Missing: {display}")
|
||||
|
||||
profile = grouped.get("ios", {}).get("profile")
|
||||
if profile:
|
||||
profile_path = Path(profile.stored_path)
|
||||
profile_path = _find_uploaded_file(request_id, "ios", "profile")
|
||||
if profile_path:
|
||||
if not _check_mobileprovision_for_carplay(profile_path):
|
||||
issues.append("iOS provisioning profile does not appear to include CarPlay entitlement text.")
|
||||
|
||||
return issues
|
||||
|
||||
|
||||
def _to_file_response(row: MobileBuildUploadedFile) -> MobileBuildUploadedFileResponse:
|
||||
return MobileBuildUploadedFileResponse(
|
||||
id=row.id,
|
||||
request_id=row.request_id,
|
||||
platform=row.platform,
|
||||
file_kind=row.file_kind,
|
||||
original_name=row.original_name,
|
||||
size=row.size,
|
||||
uploaded_at=_utc_iso(row.uploaded_at) or "",
|
||||
)
|
||||
|
||||
|
||||
def _to_artifact_response(row: MobileBuildArtifact) -> MobileBuildArtifactResponse:
|
||||
return MobileBuildArtifactResponse(
|
||||
id=row.id,
|
||||
@@ -242,14 +282,9 @@ def _to_artifact_response(row: MobileBuildArtifact) -> MobileBuildArtifactRespon
|
||||
|
||||
|
||||
async def _hydrate_request(session: AsyncSession, request: MobileBuildRequest) -> MobileBuildRequestResponse:
|
||||
files_result = await session.execute(
|
||||
select(MobileBuildUploadedFile).where(MobileBuildUploadedFile.request_id == request.id)
|
||||
)
|
||||
artifact_result = await session.execute(
|
||||
select(MobileBuildArtifact).where(MobileBuildArtifact.request_id == request.id)
|
||||
)
|
||||
|
||||
files = list(files_result.scalars().all())
|
||||
artifacts = list(artifact_result.scalars().all())
|
||||
|
||||
return MobileBuildRequestResponse(
|
||||
@@ -271,7 +306,6 @@ async def _hydrate_request(session: AsyncSession, request: MobileBuildRequest) -
|
||||
updated_at=_utc_iso(request.updated_at) or "",
|
||||
started_at=_utc_iso(request.started_at),
|
||||
completed_at=_utc_iso(request.completed_at),
|
||||
files=[_to_file_response(f) for f in files],
|
||||
artifacts=[_to_artifact_response(a) for a in artifacts],
|
||||
)
|
||||
|
||||
@@ -323,19 +357,21 @@ async def _copy_worker_artifact(
|
||||
|
||||
|
||||
async def _upload_files_to_worker(
|
||||
files: list[MobileBuildUploadedFile], host: str
|
||||
request_id: int, platform: str, host: str
|
||||
) -> None:
|
||||
scp_prefix = _build_scp_prefix(host)
|
||||
remote_target = f"{settings.MOBILE_BUILD_SSH_USER}@{host}:{settings.MOBILE_BUILD_REMOTE_APP_DIR}/"
|
||||
|
||||
for row in files:
|
||||
local_path = Path(row.stored_path)
|
||||
if not local_path.exists():
|
||||
raise RuntimeError(f"Uploaded file missing: {row.original_name}")
|
||||
for kind in _ALLOWED_KINDS[platform]:
|
||||
role_key = f"{platform}_{kind}"
|
||||
display = _FILE_ROLE_DISPLAY[role_key]
|
||||
local_path = _find_uploaded_file(request_id, platform, kind)
|
||||
if not local_path or not local_path.exists():
|
||||
raise RuntimeError(f"Uploaded file missing: {display}")
|
||||
cmd = scp_prefix + [str(local_path), remote_target]
|
||||
code, out = await _run_command(cmd)
|
||||
if code != 0:
|
||||
raise RuntimeError(f"Failed to upload file {row.original_name}: {out}")
|
||||
raise RuntimeError(f"Failed to upload {display}: {out}")
|
||||
|
||||
|
||||
async def _upload_theme_to_worker(
|
||||
@@ -364,10 +400,8 @@ async def _run_platform_build(
|
||||
host: str,
|
||||
script_name: str,
|
||||
) -> None:
|
||||
files = await _load_files(session, request.id)
|
||||
platform_files = [f for f in files if f.platform == platform]
|
||||
await _append_log(session, request, f"[{platform}] Uploading {len(platform_files)} file(s) to worker")
|
||||
await _upload_files_to_worker(platform_files, host)
|
||||
await _append_log(session, request, f"[{platform}] Uploading {len(_ALLOWED_KINDS[platform])} file(s) to worker")
|
||||
await _upload_files_to_worker(request.id, platform, host)
|
||||
|
||||
# Upload theme.json to worker
|
||||
await _upload_theme_to_worker(session, request, host)
|
||||
@@ -519,21 +553,20 @@ async def _run_flutter_container_build(
|
||||
await _append_log(session, request, "[android] theme.json not found, skipping")
|
||||
|
||||
# Step 5: Copy signing files into the container
|
||||
files = await _load_files(session, request.id)
|
||||
android_files = [f for f in files if f.platform == "android"]
|
||||
|
||||
for row in android_files:
|
||||
local_path = Path(row.stored_path)
|
||||
if not local_path.exists():
|
||||
raise RuntimeError(f"Signing file missing: {row.original_name}")
|
||||
for kind in _ALLOWED_KINDS["android"]:
|
||||
role_key = f"android_{kind}"
|
||||
display = _FILE_ROLE_DISPLAY[role_key]
|
||||
local_path = _find_uploaded_file(request.id, "android", kind)
|
||||
if not local_path or not local_path.exists():
|
||||
raise RuntimeError(f"Signing file missing: {display}")
|
||||
|
||||
# SCP directly to build container
|
||||
remote_path = f"{project_dir}/{row.original_name}"
|
||||
remote_path = f"{project_dir}/{_sanitize_filename(local_path.name)}"
|
||||
code, out = await _scp_to_container(host, local_path, remote_path)
|
||||
if code != 0:
|
||||
raise RuntimeError(f"Failed to SCP {row.original_name} to build container: {out}")
|
||||
raise RuntimeError(f"Failed to SCP {display} to build container: {out}")
|
||||
|
||||
await _append_log(session, request, f"[android] Signing file {row.original_name} copied to build container")
|
||||
await _append_log(session, request, f"[android] {display} copied to build container")
|
||||
|
||||
# Step 6: Run build_whitelabel.py
|
||||
await _append_log(session, request, "[android] Running build_whitelabel.py...")
|
||||
@@ -705,7 +738,42 @@ async def get_mobile_build_request(
|
||||
return await _hydrate_request(session, request)
|
||||
|
||||
|
||||
@router.post("/requests/{request_id}/files", response_model=MobileBuildUploadedFileResponse)
|
||||
@router.delete("/requests/{request_id}", status_code=status.HTTP_204_NO_CONTENT)
|
||||
async def delete_mobile_build_request(
|
||||
request_id: int,
|
||||
session: AsyncSession = Depends(get_session),
|
||||
_: User = Depends(get_current_admin_user),
|
||||
):
|
||||
"""Delete a draft build request and its uploaded files."""
|
||||
result = await session.execute(
|
||||
select(MobileBuildRequest).where(MobileBuildRequest.id == request_id)
|
||||
)
|
||||
request = result.scalar_one_or_none()
|
||||
if request is None:
|
||||
raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Build request not found")
|
||||
|
||||
if request.status in {"queued", "building"}:
|
||||
raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail="Cannot delete a request that is in progress")
|
||||
|
||||
# Clean up uploaded files on disk
|
||||
upload_dir, _ = _ensure_dirs()
|
||||
req_dir = upload_dir / f"request_{request_id}"
|
||||
if req_dir.is_dir():
|
||||
import shutil
|
||||
shutil.rmtree(req_dir, ignore_errors=True)
|
||||
|
||||
# Delete artifacts first (FK cascade may not cover async)
|
||||
await session.execute(
|
||||
delete(MobileBuildArtifact).where(MobileBuildArtifact.request_id == request_id)
|
||||
)
|
||||
# Then the request itself
|
||||
await session.execute(
|
||||
delete(MobileBuildRequest).where(MobileBuildRequest.id == request_id)
|
||||
)
|
||||
await session.commit()
|
||||
|
||||
|
||||
@router.post("/requests/{request_id}/files")
|
||||
async def upload_mobile_build_file(
|
||||
request_id: int,
|
||||
platform: Literal["android", "ios"] = Query(...),
|
||||
@@ -714,6 +782,10 @@ async def upload_mobile_build_file(
|
||||
session: AsyncSession = Depends(get_session),
|
||||
_: User = Depends(get_current_admin_user),
|
||||
):
|
||||
"""Upload a signing file to the filesystem for a build request.
|
||||
|
||||
Files are stored on disk, not in the database.
|
||||
"""
|
||||
if file_kind not in _ALLOWED_KINDS[platform]:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_400_BAD_REQUEST,
|
||||
@@ -731,43 +803,32 @@ async def upload_mobile_build_file(
|
||||
req_dir = upload_dir / f"request_{request_id}"
|
||||
req_dir.mkdir(parents=True, exist_ok=True)
|
||||
|
||||
payload = await file.read()
|
||||
|
||||
# Validate extension and content
|
||||
_validate_upload(platform, file_kind, file.filename, payload)
|
||||
|
||||
suffix = Path(file.filename or "upload.bin").suffix
|
||||
local_name = f"{platform}_{file_kind}_{uuid.uuid4().hex}{suffix}"
|
||||
local_path = req_dir / local_name
|
||||
|
||||
payload = await file.read()
|
||||
local_path.write_bytes(payload)
|
||||
|
||||
sha = hashlib.sha256(payload).hexdigest()
|
||||
size = len(payload)
|
||||
|
||||
await session.execute(
|
||||
delete(MobileBuildUploadedFile).where(
|
||||
MobileBuildUploadedFile.request_id == request_id,
|
||||
MobileBuildUploadedFile.platform == platform,
|
||||
MobileBuildUploadedFile.file_kind == file_kind,
|
||||
)
|
||||
)
|
||||
|
||||
row = MobileBuildUploadedFile(
|
||||
request_id=request_id,
|
||||
platform=platform,
|
||||
file_kind=file_kind,
|
||||
original_name=file.filename or local_name,
|
||||
stored_path=str(local_path),
|
||||
sha256=sha,
|
||||
size=size,
|
||||
)
|
||||
session.add(row)
|
||||
|
||||
request.preflight_passed = False
|
||||
request.preflight_report = ""
|
||||
request.updated_at = datetime.utcnow()
|
||||
|
||||
await session.commit()
|
||||
await session.refresh(row)
|
||||
|
||||
return _to_file_response(row)
|
||||
return {
|
||||
"platform": platform,
|
||||
"file_kind": file_kind,
|
||||
"display_name": _FILE_ROLE_DISPLAY[f"{platform}_{file_kind}"],
|
||||
"sha256": sha,
|
||||
"size": size,
|
||||
}
|
||||
|
||||
|
||||
@router.post("/requests/{request_id}/preflight", response_model=MobileBuildPreflightResponse)
|
||||
@@ -783,8 +844,7 @@ async def preflight_mobile_build(
|
||||
if request is None:
|
||||
raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Build request not found")
|
||||
|
||||
files = await _load_files(session, request_id)
|
||||
issues = _preflight_issues(request, files)
|
||||
issues = _preflight_issues(request, request_id)
|
||||
|
||||
request.preflight_passed = len(issues) == 0
|
||||
request.preflight_report = "\n".join(issues)
|
||||
@@ -807,8 +867,7 @@ async def trigger_mobile_build(
|
||||
if request is None:
|
||||
raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Build request not found")
|
||||
|
||||
files = await _load_files(session, request_id)
|
||||
issues = _preflight_issues(request, files)
|
||||
issues = _preflight_issues(request, request_id)
|
||||
if issues:
|
||||
request.preflight_passed = False
|
||||
request.preflight_report = "\n".join(issues)
|
||||
|
||||
Reference in New Issue
Block a user