From dbe3bdb6ed2de433b85947d554166a1e35640f57 Mon Sep 17 00:00:00 2001 From: Shane Israel Date: Sat, 8 Aug 2026 08:31:58 -0600 Subject: [PATCH] fix an nginx auth check on password protected videos that would allow auth on urls it was unable to parse --- app/client/package-lock.json | 4 +-- app/client/package.json | 4 +-- app/nginx/dev.template.conf | 4 +++ app/nginx/prod.conf | 10 ++++-- app/server/fireshare/api/video.py | 57 +++++++++++++++++++++++++------ 5 files changed, 62 insertions(+), 17 deletions(-) diff --git a/app/client/package-lock.json b/app/client/package-lock.json index 68bf2ef1..b6474b83 100644 --- a/app/client/package-lock.json +++ b/app/client/package-lock.json @@ -1,6 +1,6 @@ { "name": "fireshare", - "version": "1.7.5", + "version": "1.7.6", "lockfileVersion": 3, "requires": true, "packages": { @@ -4868,4 +4868,4 @@ } } } -} \ No newline at end of file +} diff --git a/app/client/package.json b/app/client/package.json index 138e7b4f..24647d0d 100644 --- a/app/client/package.json +++ b/app/client/package.json @@ -1,6 +1,6 @@ { "name": "fireshare", - "version": "1.7.5", + "version": "1.7.6", "private": true, "dependencies": { "@emotion/react": "^11.9.0", @@ -39,4 +39,4 @@ "build": "vite build", "preview": "vite preview" } -} \ No newline at end of file +} diff --git a/app/nginx/dev.template.conf b/app/nginx/dev.template.conf index 78a02cc8..159fe2ce 100644 --- a/app/nginx/dev.template.conf +++ b/app/nginx/dev.template.conf @@ -36,6 +36,10 @@ http { proxy_pass_request_body off; proxy_set_header Content-Length ""; proxy_set_header X-Original-URI $request_uri; + # Empty here (this location block captures no id, so the auth endpoint falls back + # to parsing the URI), but declaring it stops a client-supplied header of this + # name from being forwarded and trusted. + proxy_set_header X-Fireshare-Video-Id $video_id; proxy_set_header Cookie $http_cookie; } diff --git a/app/nginx/prod.conf b/app/nginx/prod.conf index 7f60c6fe..cd7a3741 100644 --- a/app/nginx/prod.conf +++ b/app/nginx/prod.conf @@ -96,6 +96,10 @@ http { proxy_pass_request_body off; proxy_set_header Content-Length ""; proxy_set_header X-Original-URI $request_uri; + # The id captured by the serving location, so the auth check gates the same + # video the try_files below actually serves. Declaring it here also stops a + # client-supplied header of this name from being forwarded. + proxy_set_header X-Fireshare-Video-Id $video_id; proxy_set_header Cookie $http_cookie; } @@ -122,6 +126,10 @@ http { } location ~ ^/_content/video/([\w-]+)(\.[^/]+)$ { + # Captured before auth_request runs so the gate sees the same id as try_files + set $video_id $1; + set $video_ext $2; + auth_request /internal/video-auth; sendfile off; @@ -139,8 +147,6 @@ http { limit_rate_after 5m; - set $video_id $1; - set $video_ext $2; root /processed/; try_files /derived/$video_id/$video_id-cropped.mp4 /video_links/$video_id$video_ext =404; } diff --git a/app/server/fireshare/api/video.py b/app/server/fireshare/api/video.py index 3614939b..cfb6d275 100644 --- a/app/server/fireshare/api/video.py +++ b/app/server/fireshare/api/video.py @@ -1,5 +1,6 @@ import logging import os +import posixpath import re import secrets import shutil @@ -9,6 +10,7 @@ import threading from datetime import datetime, timedelta from pathlib import Path +from urllib.parse import unquote from flask import current_app, jsonify, request, Response, send_file, session from flask_login import login_required, current_user @@ -795,20 +797,53 @@ def nginx_video_auth_admin(): return '', 403 +_VIDEO_ID_RE = re.compile(r'^[\w-]+$') +_CONTENT_VIDEO_RE = re.compile(r'^/_content/video/([\w-]+)\.[a-z0-9]+', re.IGNORECASE) +_CONTENT_DERIVED_RE = re.compile(r'^/_content/derived/([\w-]+)/', re.IGNORECASE) + + +def _video_id_from_original_uri(original_uri): + """ + Resolve the video id from nginx's X-Original-URI. + + nginx matches its location blocks against the decoded, normalized URI but forwards the + raw one in $request_uri, so the URI seen here has to be decoded and normalized the same + way before it is parsed — otherwise a request like /_content/vide%6F/.mp4 routes to + the file on the nginx side while failing to resolve an id on this side. + """ + path = unquote(original_uri.split('?', 1)[0]) + if not path.startswith('/'): + return None + path = posixpath.normpath(path) + # normpath drops a meaningful trailing slash, which the derived pattern relies on + if original_uri.split('?', 1)[0].endswith('/') and not path.endswith('/'): + path += '/' + for pattern in (_CONTENT_VIDEO_RE, _CONTENT_DERIVED_RE): + m = pattern.match(path) + if m: + return m.group(1) + return None + + @api.route('/api/video/nginx-auth') def nginx_video_auth(): - """Internal endpoint called by nginx auth_request to gate password-protected video files.""" - original_uri = request.headers.get('X-Original-URI', '') - video_id = None - m = re.match(r'^/_content/video/([\w-]+)\.[a-z0-9]+', original_uri) - if m: - video_id = m.group(1) - if not video_id: - m = re.match(r'^/_content/derived/([\w-]+)/', original_uri) - if m: - video_id = m.group(1) + """ + Internal endpoint called by nginx auth_request to gate password-protected video files. + + Prefers the id nginx already captured from its own location regex (X-Fireshare-Video-Id), + which is the same value it uses to pick the file, so the two cannot disagree. Falls back + to parsing the URI, and refuses the request if no id can be resolved — an unresolvable id + means the gate cannot do its job, so it must not let the request through. + """ + video_id = request.headers.get('X-Fireshare-Video-Id') + if not video_id or not _VIDEO_ID_RE.match(video_id): + video_id = _video_id_from_original_uri(request.headers.get('X-Original-URI', '')) if not video_id: - return '', 200 + logger.warning( + f"nginx-auth could not resolve a video id for " + f"{request.headers.get('X-Original-URI', '')!r}; denying request" + ) + return '', 403 video_info = VideoInfo.query.filter_by(video_id=video_id).first() if not video_info or not video_info.password_hash: return '', 200