From 86c40aee91be9d96e85c9d937426057a348388b9 Mon Sep 17 00:00:00 2001 From: justanamelessguy Date: Sun, 7 Jun 2026 23:08:29 +0200 Subject: [PATCH] Sanitize user text input --- backend/app.py | 154 ++++++++++++++++++++++++++++++++++++--------- journey-edit.html | 4 +- js/auth.js | 16 ++++- js/blog-list.js | 11 ++-- js/blog-post.js | 13 ++-- js/journey-edit.js | 13 ++-- js/map-page.js | 15 +++-- login.html | 8 +-- map-page.html | 4 ++ 9 files changed, 179 insertions(+), 59 deletions(-) diff --git a/backend/app.py b/backend/app.py index d4d0d60..8d0adcb 100644 --- a/backend/app.py +++ b/backend/app.py @@ -2,6 +2,8 @@ import os import time import json import uuid +import math +import re from datetime import datetime from werkzeug.security import generate_password_hash, check_password_hash from werkzeug.utils import secure_filename @@ -19,10 +21,110 @@ USERS_FILE = os.path.join(DATA_DIR, "users.json") JOURNEYS_FILE = os.path.join(DATA_DIR, 'journeys.json') UPLOAD_DIR = os.path.join(BASE_DIR, "uploads") ALLOWED_IMAGE_EXTENSIONS = {"png", "jpg", "jpeg", "gif", "webp"} +VALID_VISIBILITIES = {"private", "public", "shared"} +MAX_MARKERS_PER_JOURNEY = 500 os.makedirs(DATA_DIR, exist_ok=True) os.makedirs(UPLOAD_DIR, exist_ok=True) +class ValidationError(ValueError): + pass + + +@app.errorhandler(ValidationError) +def handle_validation_error(error): + return jsonify({"error": str(error)}), 400 + + +def get_json_object(): + data = request.get_json(silent=True) + if not isinstance(data, dict): + raise ValidationError("Request body must be a JSON object") + return data + + +def clean_text(value, field, max_length, required=False, strip=True): + if value is None: + value = "" + if not isinstance(value, str): + raise ValidationError(f"{field} must be text") + + # Keep newlines/tabs for Markdown, but remove non-printing control characters. + value = re.sub(r"[\x00-\x08\x0b\x0c\x0e-\x1f\x7f]", "", value) + if strip: + value = value.strip() + if required and not value: + raise ValidationError(f"{field} is required") + if len(value) > max_length: + raise ValidationError(f"{field} must be at most {max_length} characters") + return value + + +def clean_number(value, field, minimum, maximum): + if isinstance(value, bool): + raise ValidationError(f"{field} must be a number") + try: + value = float(value) + except (TypeError, ValueError): + raise ValidationError(f"{field} must be a number") + if not math.isfinite(value) or not minimum <= value <= maximum: + raise ValidationError(f"{field} must be between {minimum} and {maximum}") + return value + + +def clean_visibility(value): + if value not in VALID_VISIBILITIES: + raise ValidationError("Invalid journey visibility") + return value + + +def clean_images(images): + if images is None: + return [] + if not isinstance(images, list): + raise ValidationError("Marker images must be a list") + if len(images) > 20: + raise ValidationError("A marker can contain at most 20 images") + + cleaned = [] + for image in images: + if isinstance(image, str): + cleaned.append(clean_text(image, "Image URL", 2048, required=True)) + continue + if not isinstance(image, dict): + raise ValidationError("Invalid marker image") + url = clean_text(image.get("url"), "Image URL", 2048, required=True) + cleaned.append({ + "filename": clean_text(image.get("filename"), "Image filename", 255), + "originalName": clean_text(image.get("originalName"), "Original image name", 255), + "url": url, + }) + return cleaned + + +def clean_markers(markers): + if markers is None: + return [] + if not isinstance(markers, list): + raise ValidationError("Markers must be a list") + if len(markers) > MAX_MARKERS_PER_JOURNEY: + raise ValidationError(f"A journey can contain at most {MAX_MARKERS_PER_JOURNEY} markers") + + cleaned = [] + for marker in markers: + if not isinstance(marker, dict): + raise ValidationError("Invalid marker") + cleaned.append({ + "lat": clean_number(marker.get("lat"), "Marker latitude", -90, 90), + "lng": clean_number(marker.get("lng"), "Marker longitude", -180, 180), + "title": clean_text(marker.get("title"), "Marker title", 200), + "date": clean_text(marker.get("date"), "Marker date", 20), + "description": clean_text(marker.get("description"), "Marker description", 10000), + "images": clean_images(marker.get("images", [])), + }) + return cleaned + + # ==================== User helpers ==================== def require_login(): @@ -97,12 +199,11 @@ def allowed_image_file(filename): # ==================== Authentication endpoints ==================== @app.route("/api/register", methods=["POST"]) def register(): - data = request.get_json() - username = data.get("username") - password = data.get("password") - - if not username or not password: - return jsonify({"error": "Username and password required"}), 400 + data = get_json_object() + username = clean_text(data.get("username"), "Username", 50, required=True) + password = clean_text(data.get("password"), "Password", 200, required=True, strip=False) + if len(password) < 4: + raise ValidationError("Password must be at least 4 characters") # Check if username already exists if get_user_by_username(username): @@ -131,9 +232,9 @@ def register(): @app.route("/api/login", methods=["POST"]) def login(): - data = request.get_json() - username = data.get("username") - password = data.get("password") + data = get_json_object() + username = clean_text(data.get("username"), "Username", 50, required=True) + password = clean_text(data.get("password"), "Password", 200, required=True, strip=False) user = get_user_by_username(username) if not user or not check_password_hash(user["password_hash"], password): @@ -242,13 +343,9 @@ def get_journeys(): def create_journey(): if not require_login(): return jsonify({'error': 'Authentication required'}), 401 - data = request.get_json() - if not data: - return jsonify({'error': 'No data provided'}), 400 + data = get_json_object() - title = data.get('title') - if not title: - return jsonify({'error': 'Journey title is required'}), 400 + title = clean_text(data.get('title'), "Journey title", 200, required=True) user_id = get_current_user_id() journeys = load_all_journeys() @@ -258,13 +355,13 @@ def create_journey(): 'id': new_id, 'owner_id': user_id, 'title': title, - 'description': data.get('description', ''), - 'markers': data.get('markers', []), + 'description': clean_text(data.get('description'), "Journey description", 20000), + 'markers': clean_markers(data.get('markers', [])), 'created_at': datetime.now().isoformat(), - 'visibility': data.get('visibility', 'private'), + 'visibility': clean_visibility(data.get('visibility', 'private')), 'shared_read': normalize_user_ids(data.get('shared_read', [])), 'shared_edit': normalize_user_ids(data.get('shared_edit', [])), - 'comments': data.get('comments', []) + 'comments': [] } journeys.append(new_journey) @@ -295,26 +392,23 @@ def update_journey(journey_id): if not user_can_edit_journey(journey, user_id): return jsonify({'error': 'Not authorized to edit this journey'}), 403 - data = request.get_json() + data = get_json_object() if 'title' in data: - journey['title'] = data['title'] + journey['title'] = clean_text(data['title'], "Journey title", 200, required=True) if 'description' in data: - journey['description'] = data['description'] + journey['description'] = clean_text(data['description'], "Journey description", 20000) if 'markers' in data: - journey['markers'] = data['markers'] + journey['markers'] = clean_markers(data['markers']) sharing_fields = {'visibility', 'shared_read', 'shared_edit'} if sharing_fields.intersection(data.keys()): if journey['owner_id'] != user_id: return jsonify({'error': 'Only the owner can update sharing settings'}), 403 if 'visibility' in data: - journey['visibility'] = data['visibility'] + journey['visibility'] = clean_visibility(data['visibility']) if 'shared_read' in data: journey['shared_read'] = normalize_user_ids(data['shared_read']) if 'shared_edit' in data: journey['shared_edit'] = normalize_user_ids(data['shared_edit']) - if 'comments' in data: - journey['comments'] = data ['comments'] - save_all_journeys(journeys) return jsonify(journey) @@ -362,10 +456,8 @@ def add_journey_comment(journey_id): user_id = session.get('user_id') if not user_id: return jsonify({'error': 'Authentication required'}), 401 - data = request.get_json() - text = data.get('text') - if not text: - return jsonify({'error': 'Comment text required'}), 400 + data = get_json_object() + text = clean_text(data.get('text'), "Comment", 2000, required=True) journey = get_journey_by_id(journey_id) if not journey: diff --git a/journey-edit.html b/journey-edit.html index 6d48f9d..3f1d505 100644 --- a/journey-edit.html +++ b/journey-edit.html @@ -335,11 +335,11 @@
- +
- +
diff --git a/js/auth.js b/js/auth.js index df2c6c6..5507441 100644 --- a/js/auth.js +++ b/js/auth.js @@ -104,14 +104,28 @@ async function logout() { function escapeHtml(str) { if (!str) return ""; - return str.replace(/[&<>]/g, function (m) { + return String(str).replace(/[&<>"']/g, function (m) { if (m === "&") return "&"; if (m === "<") return "<"; if (m === ">") return ">"; + if (m === '"') return """; + if (m === "'") return "'"; return m; }); } +function escapeAttribute(str) { + return escapeHtml(str); +} + +function sanitizeDisplayUrl(url) { + const value = String(url || "").trim().replace(/[\u0000-\u001f\u007f\s]+/g, ""); + if (/^(https?:\/\/|\/uploads\/|uploads\/)/i.test(value)) { + return value; + } + return ""; +} + function showToast(msg, isError = false) { const toast = document.getElementById("toast"); if (!toast) return; diff --git a/js/blog-list.js b/js/blog-list.js index 1e1d6de..f44e511 100644 --- a/js/blog-list.js +++ b/js/blog-list.js @@ -25,11 +25,13 @@ function renderJourneys(journeys) { return; } - container.innerHTML = journeys.map(journey => ` + container.innerHTML = journeys.map(journey => { + const imageUrl = sanitizeDisplayUrl(journey.image || ''); + return `
- ${journey.image ? `${journey.title}` : '
'} + ${imageUrl ? `${escapeAttribute(journey.title)}` : '
'}
-

${escapeHtml(journey.title)}

+

${escapeHtml(journey.title)}

${new Date(journey.created_at).toLocaleDateString()} ${journey.markers ? ` ${journey.markers.length} chapters` : ''} @@ -38,7 +40,8 @@ function renderJourneys(journeys) {
${escapeHtml(journey.description || journey.markers?.[0]?.text?.substring(0, 150) + '…')}
- `).join(''); + `; + }).join(''); } function getJourneyBadges(journey) { diff --git a/js/blog-post.js b/js/blog-post.js index 0bc599a..9b2c8d4 100644 --- a/js/blog-post.js +++ b/js/blog-post.js @@ -36,7 +36,7 @@ function renderJourney() { marker.images = getMarkerImages(marker); const images = getMarkerImagesHtml(marker.images, idx, canEdit); chaptersHtml += ` -
+

${escapeHtml(title)}

${date} @@ -60,7 +60,7 @@ function renderJourney() { ${currentJourney.visibility === 'public' ? 'Public' : ''} ${currentJourney.visibility === 'shared' && !isOwner ? `${canEdit ? 'Shared edit' : 'Shared'}` : ''}
- ${currentJourney.image ? `${currentJourney.title}` : ''} + ${currentJourney.image ? `${escapeAttribute(currentJourney.title)}` : ''}
${renderMarkdown(currentJourney.description)}
${chaptersHtml} ${canEdit || isOwner ? ` @@ -82,7 +82,7 @@ function renderJourney() { function getUploadUrl(path) { if (!path) return ''; - if (path.startsWith('http')) return path; + if (path.startsWith('http')) return sanitizeDisplayUrl(path); return API_BASE.replace('/api', path); } @@ -115,10 +115,11 @@ function getMarkerImagesHtml(images, markerIndex, canEdit) { const imageHtml = images.map((image, imageIndex) => { const url = typeof image === 'string' ? image : image.url; const alt = typeof image === 'string' ? 'Journey image' : image.originalName || 'Journey image'; - if (!url) return ''; + const displayUrl = sanitizeDisplayUrl(getUploadUrl(url)); + if (!displayUrl) return ''; return `
- ${escapeAttribute(alt)} + ${escapeAttribute(alt)} ${canEdit ? `
`; diff --git a/js/journey-edit.js b/js/journey-edit.js index 736ff7d..512fbf1 100644 --- a/js/journey-edit.js +++ b/js/journey-edit.js @@ -21,7 +21,7 @@ function showToast(message, isError = false) { function getUploadUrl(path) { if (!path) return ''; - if (path.startsWith('http')) return path; + if (path.startsWith('http')) return sanitizeDisplayUrl(path); return API_BASE.replace('/api', path); } @@ -130,10 +130,11 @@ function renderMarkerImages(marker, idx) { ${images.map((image, imageIdx) => { const url = typeof image === 'string' ? image : image.url; const alt = typeof image === 'string' ? 'Chapter image' : image.originalName || 'Chapter image'; - if (!url) return ''; + const displayUrl = sanitizeDisplayUrl(getUploadUrl(url)); + if (!displayUrl) return ''; return `
- ${escapeAttribute(alt)} + ${escapeAttribute(alt)} @@ -183,15 +184,15 @@ function renderMarkers() {
- +
- +
- +
${renderMarkdownPreview(marker.description)}
diff --git a/js/map-page.js b/js/map-page.js index e240c58..a833e4e 100644 --- a/js/map-page.js +++ b/js/map-page.js @@ -59,7 +59,7 @@ }).addTo(map); marker.bindPopup( - `${content.title || "Untitled"}`, + `${escapeHtml(content.title || "Untitled")}`, ); marker.on("click", () => openMarkerEditor(marker)); marker.on("dragend", () => { @@ -90,7 +90,7 @@ div.dataset.lat = latlng.lat; div.dataset.lng = latlng.lng; div.innerHTML = ` -
${content.title || "Untitled"}
+
${escapeHtml(content.title || "Untitled")}
${latlng.lat.toFixed(4)}, ${latlng.lng.toFixed(4)}
`; div.addEventListener("click", () => { @@ -110,7 +110,7 @@ function getUploadUrl(path) { if (!path) return ""; - if (path.startsWith("http")) return path; + if (path.startsWith("http")) return sanitizeDisplayUrl(path); return API_BASE.replace("/api", path); } @@ -121,8 +121,13 @@ images.forEach((image, index) => { const item = document.createElement("div"); item.className = "image-preview"; + const imagePath = typeof image === "string" ? image : image.url; + const imageName = typeof image === "string" ? "Marker image" : image.originalName; + const url = sanitizeDisplayUrl(getUploadUrl(imagePath)); + const alt = escapeAttribute(imageName || "Marker image"); + if (!url) return; item.innerHTML = ` - ${image.originalName || + ${alt} @@ -208,7 +213,7 @@ // Update marker's tooltip title and popup activeMarker.options.title = title; - activeMarker.setPopupContent(`${title}`); + activeMarker.setPopupContent(`${escapeHtml(title)}`); // Store content for saving activeMarker._content = { title, date, description, images }; diff --git a/login.html b/login.html index d31cef5..43dc2e3 100644 --- a/login.html +++ b/login.html @@ -135,22 +135,22 @@
- +
- +
- +
- +
diff --git a/map-page.html b/map-page.html index c4caf95..479359a 100644 --- a/map-page.html +++ b/map-page.html @@ -641,6 +641,7 @@ @@ -660,6 +661,7 @@ >
@@ -842,6 +844,7 @@
@@ -853,6 +856,7 @@