Security fixes: SSRF, staff role escalation, SPA-only gating, throttling, encrypted keys
Findings from the audit (50-check harness across guest/user/uploader/staff/ admin) and their fixes: - SSRF: 'Download to Library' and the staged-upload resolve path fetched any http(s) URL. services.validate_remote_url now enforces the e621 media allowlist and open_remote re-validates every redirect hop; the download-task create endpoint and the guest proxy use them, so internal addresses (127.0.0.1, LAN, metadata) are rejected with 400. - Privilege escalation: staff could promote users to staff and demote other staff. Role changes across the staff boundary now require an admin, matching the account-deletion rules; the Users page hides what the backend would refuse. - SPA-only gating: /api/storage/ and /api/duplicates/* were readable by any authenticated account (absolute paths, duplicate groups) while the SPA only shows them to uploaders. They now require CanUpload. - Throttling (REST_FRAMEWORK, env-overridable, counted in Redis): anon 120/min, user 600/min, login 5/min, register 20/hour, guest e621 proxy 60/hour. Login now goes through a throttled view. - e621 API keys are encrypted at rest with a Fernet key derived from SECRET_KEY (apps/accounts/crypto.py); a data migration encrypts existing rows and the column widens first. Reads decrypt transparently, legacy plaintext still works, and a changed SECRET_KEY reads as 'not configured' instead of leaking. Rotating SECRET_KEY now invalidates stored keys as well as signed media URLs. - Hardening: the server refuses to start with DEBUG=False while SECRET_KEY is still the development default. Verified: corrected harness 50/50 (guest visibility, IDOR, signed-URL tamper/expiry, staged-upload/similarity privacy, role matrix, SSRF), login throttles at the 6th attempt with 429, anon polling unaffected, the guest proxy still reaches allowlisted hosts, live e621 auth works with the decrypted key, and DB rows hold only ciphertext.
This commit is contained in:
@@ -59,7 +59,11 @@ def get(user, path, params=None, timeout=30, require_auth=True):
|
||||
response = requests.get(
|
||||
f"{base}{path}",
|
||||
params=params,
|
||||
auth=(user.e621_username, user.e621_api_key) if configured else None,
|
||||
auth=(
|
||||
(user.e621_username, user.e621_api_key_plain)
|
||||
if configured
|
||||
else None
|
||||
),
|
||||
headers={"User-Agent": settings.USER_AGENT},
|
||||
timeout=timeout,
|
||||
)
|
||||
|
||||
@@ -255,6 +255,51 @@ class DownloadCancelled(Exception):
|
||||
"""Raised when a streamed download is cancelled by the user."""
|
||||
|
||||
|
||||
class RemoteUrlError(ValueError):
|
||||
"""The URL is not an allowed e621 media URL."""
|
||||
|
||||
|
||||
def validate_remote_url(url):
|
||||
"""Only http(s) URLs on the known e621 media hosts may be fetched.
|
||||
|
||||
Without this the download paths are an SSRF hole: any uploader could make
|
||||
the server fetch internal addresses (127.0.0.1, LAN services, cloud
|
||||
metadata) and read the response back through the library.
|
||||
"""
|
||||
from urllib.parse import urlparse
|
||||
|
||||
parsed = urlparse(str(url or "").strip())
|
||||
if parsed.scheme not in {"http", "https"} or not parsed.hostname:
|
||||
raise RemoteUrlError("Only http(s) URLs can be fetched.")
|
||||
if parsed.hostname not in settings.E621_MEDIA_HOSTS:
|
||||
raise RemoteUrlError("That host is not an allowed e621 media host.")
|
||||
return url
|
||||
|
||||
|
||||
def open_remote(url, *, max_redirects=3, **kwargs):
|
||||
"""GET an allowlisted URL, re-validating every redirect hop.
|
||||
|
||||
Returns a streaming ``requests`` response. Redirects are followed
|
||||
manually so a hop cannot jump to an internal host.
|
||||
"""
|
||||
import requests
|
||||
from urllib.parse import urljoin
|
||||
|
||||
current = url
|
||||
for _ in range(max_redirects + 1):
|
||||
validate_remote_url(current)
|
||||
response = requests.get(current, allow_redirects=False, **kwargs)
|
||||
if response.is_redirect or response.is_permanent_redirect:
|
||||
location = response.headers.get("Location")
|
||||
response.close()
|
||||
if not location:
|
||||
raise RemoteUrlError("The remote server redirected without a target.")
|
||||
current = urljoin(current, location)
|
||||
continue
|
||||
return response
|
||||
raise RemoteUrlError("Too many redirects from the remote server.")
|
||||
|
||||
|
||||
def download_file(
|
||||
url,
|
||||
destination,
|
||||
@@ -268,10 +313,8 @@ def download_file(
|
||||
worker: without it a hung socket would keep a job "downloading" forever
|
||||
and the cancel flag could never be observed.
|
||||
"""
|
||||
import requests
|
||||
|
||||
headers = {"User-Agent": settings.USER_AGENT}
|
||||
with requests.get(
|
||||
with open_remote(
|
||||
url, headers=headers, stream=True, timeout=(10, read_timeout)
|
||||
) as response:
|
||||
response.raise_for_status()
|
||||
|
||||
@@ -8,7 +8,7 @@ from django.conf import settings
|
||||
from django.core.cache import cache
|
||||
from django.db.models import Count, Q
|
||||
from rest_framework import status
|
||||
from rest_framework.permissions import AllowAny, IsAuthenticated
|
||||
from rest_framework.permissions import AllowAny
|
||||
from rest_framework.response import Response
|
||||
from rest_framework.views import APIView
|
||||
|
||||
@@ -120,7 +120,7 @@ def resolve_item(data):
|
||||
class ExactDuplicatesView(APIView):
|
||||
"""Items whose content exists at more than one path."""
|
||||
|
||||
permission_classes = [IsAuthenticated]
|
||||
permission_classes = [CanUpload]
|
||||
|
||||
def get(self, request):
|
||||
items = (
|
||||
@@ -143,7 +143,7 @@ class ExactDuplicatesView(APIView):
|
||||
class VisualMatchesView(APIView):
|
||||
"""Items visually similar to one library item."""
|
||||
|
||||
permission_classes = [IsAuthenticated]
|
||||
permission_classes = [CanUpload]
|
||||
|
||||
def post(self, request):
|
||||
threshold = parse_threshold(request.data.get("threshold"))
|
||||
@@ -185,7 +185,7 @@ class VisualGroupsView(APIView):
|
||||
a personal library. Revisit with a bucketed index if libraries grow huge.
|
||||
"""
|
||||
|
||||
permission_classes = [IsAuthenticated]
|
||||
permission_classes = [CanUpload]
|
||||
|
||||
def post(self, request):
|
||||
threshold = parse_threshold(request.data.get("threshold"))
|
||||
@@ -450,7 +450,7 @@ def storage_info():
|
||||
class StorageView(APIView):
|
||||
"""Disk usage for the watched folder, media root and temp uploads."""
|
||||
|
||||
permission_classes = [IsAuthenticated]
|
||||
permission_classes = [CanUpload]
|
||||
|
||||
def get(self, request):
|
||||
return Response(storage_info())
|
||||
|
||||
@@ -15,6 +15,7 @@ from rest_framework import mixins, status, viewsets
|
||||
from rest_framework.decorators import action
|
||||
from rest_framework.permissions import AllowAny, IsAuthenticatedOrReadOnly
|
||||
from rest_framework.response import Response
|
||||
from rest_framework.throttling import ScopedRateThrottle
|
||||
from rest_framework.views import APIView
|
||||
|
||||
from . import e621, matching, services
|
||||
@@ -428,9 +429,11 @@ class DownloadTaskViewSet(
|
||||
url = str(request.data.get("url") or "").strip()
|
||||
post_id = request.data.get("post_id")
|
||||
filename = str(request.data.get("filename") or "").strip()
|
||||
if not url.startswith(("http://", "https://")):
|
||||
try:
|
||||
services.validate_remote_url(url)
|
||||
except services.RemoteUrlError as exc:
|
||||
return Response(
|
||||
{"detail": "A valid file URL is required."},
|
||||
{"detail": str(exc)},
|
||||
status=status.HTTP_400_BAD_REQUEST,
|
||||
)
|
||||
trimmed = services.trim_e621_post(request.data.get("post"))
|
||||
@@ -521,15 +524,15 @@ class ClientDownloadView(APIView):
|
||||
"""Stream an e621 file straight to the browser (no library write)."""
|
||||
|
||||
permission_classes = [AllowAny]
|
||||
throttle_classes = [ScopedRateThrottle]
|
||||
throttle_scope = "e621_proxy"
|
||||
|
||||
def get(self, request):
|
||||
url = str(request.query_params.get("url") or "").strip()
|
||||
filename = str(request.query_params.get("filename") or "").strip()
|
||||
parsed = urlparse(url)
|
||||
if (
|
||||
parsed.scheme not in {"http", "https"}
|
||||
or parsed.hostname not in settings.E621_MEDIA_HOSTS
|
||||
):
|
||||
try:
|
||||
services.validate_remote_url(url)
|
||||
except services.RemoteUrlError:
|
||||
return Response(
|
||||
{"detail": "URL not allowed."},
|
||||
status=status.HTTP_400_BAD_REQUEST,
|
||||
@@ -538,13 +541,17 @@ class ClientDownloadView(APIView):
|
||||
import requests
|
||||
|
||||
try:
|
||||
upstream = requests.get(
|
||||
upstream = services.open_remote(
|
||||
url,
|
||||
headers={"User-Agent": settings.USER_AGENT},
|
||||
stream=True,
|
||||
timeout=60,
|
||||
)
|
||||
upstream.raise_for_status()
|
||||
except services.RemoteUrlError as exc:
|
||||
return Response(
|
||||
{"detail": str(exc)}, status=status.HTTP_400_BAD_REQUEST
|
||||
)
|
||||
except requests.RequestException as exc:
|
||||
return Response(
|
||||
{"detail": f"Could not fetch the file: {exc}"},
|
||||
|
||||
Reference in New Issue
Block a user