From 25d4decb542ddf2298686bbd9a9701c3ad463123 Mon Sep 17 00:00:00 2001 From: Richard Allitt Date: Fri, 15 Nov 2024 21:37:40 +0000 Subject: [PATCH 1/4] Prevent addition of non-existent domains --- control/templates/shared/add_vhost_test.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/control/templates/shared/add_vhost_test.html b/control/templates/shared/add_vhost_test.html index fe00636..306eea0 100644 --- a/control/templates/shared/add_vhost_test.html +++ b/control/templates/shared/add_vhost_test.html @@ -54,7 +54,7 @@

Test custom domain

{% if valid[domain] == (true, true) or valid[prefixed] == (true, true) %} - {% else %} + {% elif valid[domain] != (none, none) or valid[domain] != (none, none) %} {% endif %} From 0cb9651df2ef110ae3392a90c9160425f83c8ebf Mon Sep 17 00:00:00 2001 From: Richard Allitt Date: Fri, 15 Nov 2024 22:10:44 +0000 Subject: [PATCH 2/4] Improve docroot validation, make add/edit consistent --- control/webapp/member.py | 24 ++++++++++++++---------- control/webapp/society.py | 22 ++++++++++++++-------- control/webapp/utils.py | 30 +++++++++++++++++++++++++++++- 3 files changed, 57 insertions(+), 19 deletions(-) diff --git a/control/webapp/member.py b/control/webapp/member.py index 7856eba..47541eb 100644 --- a/control/webapp/member.py +++ b/control/webapp/member.py @@ -1,6 +1,3 @@ -import re -import string - from flask import Blueprint, redirect, render_template, request, url_for from werkzeug.exceptions import Forbidden, NotFound @@ -10,7 +7,10 @@ from srcf.database import Domain from . import inspect_services, utils -from .utils import create_job_maybe_email_and_redirect, effective_member, parse_domain_name, srcf_db_sess as sess +from .utils import ( + create_job_maybe_email_and_redirect, effective_member, parse_domain_name, + validate_domain_docroot, srcf_db_sess as sess, +) bp = Blueprint("member", __name__) @@ -252,6 +252,7 @@ def add_vhost(): domain = request.form.get("domain", "").strip() root = request.form.get("root", "").strip() + if domain: parsed = parse_domain_name(domain) if domain != parsed: @@ -270,6 +271,11 @@ def add_vhost(): else: errors["domain"] = "Please enter a domain or subdomain." + if root: + root, msg = validate_domain_docroot(mem, root) + if msg: + errors["root"] = msg + if request.form.get("edit") or errors: return render_template("member/add_vhost.html", member=mem, domain=domain, root=root, errors=errors) elif not request.form.get("confirm"): @@ -303,12 +309,10 @@ def change_vhost_docroot(domain): if request.method == "POST": root = request.form.get("root", "").strip() - if any([ch in root for ch in string.whitespace + "\\" + "\"" + "\'"]) or ".." in root: - errors["root"] = "This document root is invalid." - try: - domain = parse_domain_name(domain) - except ValueError as e: - errors["domain"] = e.args[0] + if root: + root, msg = validate_domain_docroot(mem, root) + if msg: + errors["root"] = msg if request.method == "POST" and not errors: return create_job_maybe_email_and_redirect( diff --git a/control/webapp/society.py b/control/webapp/society.py index 9afc872..fd24774 100644 --- a/control/webapp/society.py +++ b/control/webapp/society.py @@ -1,5 +1,4 @@ import re -import string from flask import Blueprint, redirect, render_template, request, url_for from werkzeug.exceptions import BadRequest, Forbidden, NotFound @@ -10,7 +9,10 @@ from srcf.database import Domain from . import inspect_services, utils -from .utils import create_job_maybe_email_and_redirect, find_mem_society, parse_domain_name, srcf_db_sess as sess +from .utils import ( + create_job_maybe_email_and_redirect, find_mem_society, parse_domain_name, + validate_domain_docroot, srcf_db_sess as sess, +) bp = Blueprint("society", __name__) @@ -286,6 +288,7 @@ def add_vhost(society): domain = request.form.get("domain", "").strip() root = request.form.get("root", "").strip() + if domain: parsed = parse_domain_name(domain) if domain != parsed: @@ -304,6 +307,11 @@ def add_vhost(society): else: errors["domain"] = "Please enter a domain or subdomain." + if root: + root, msg = validate_domain_docroot(soc, root) + if msg: + errors["root"] = msg + if request.form.get("edit") or errors: return render_template("society/add_vhost.html", society=soc, member=mem, domain=domain, root=root, errors=errors) elif not request.form.get("confirm"): @@ -337,12 +345,10 @@ def change_vhost_docroot(society, domain): if request.method == "POST": root = request.form.get("root", "").strip() - if any([ch in root for ch in string.whitespace + "\\" + "\"" + "\'"]) or ".." in root: - errors["root"] = "This document root is invalid." - try: - domain = parse_domain_name(domain) - except ValueError as e: - errors["domain"] = e.args[0] + if root: + root, msg = validate_domain_docroot(soc, root) + if msg: + errors["root"] = msg if request.method == "POST" and not errors: return create_job_maybe_email_and_redirect( diff --git a/control/webapp/utils.py b/control/webapp/utils.py index 44bf8bb..dd54f84 100644 --- a/control/webapp/utils.py +++ b/control/webapp/utils.py @@ -1,6 +1,7 @@ from datetime import datetime from functools import partial import os +import string import sys import traceback from urllib.parse import urlparse @@ -15,7 +16,7 @@ from srcf.controllib.jobs import CreateSociety, Reactivate, Signup, SocietyJob from srcf.controllib.utils import email_re, is_admin, ldapsearch, mysql_conn -from srcf.database import JobLog, queries, Session +from srcf.database import Member, JobLog, queries, Session from srcf.mail import mail_sysadmins import ucam_webauth import ucam_webauth.flask_glue @@ -91,6 +92,33 @@ def parse_domain_name(domain): return domain.encode("idna").decode("ascii") +def validate_domain_docroot(owner, path): + if not path: + return path, None + if any(ch in path for ch in string.whitespace + "\\" + '"' + "'"): + return path, "Document roots cannot contain spaces or quotes." + if path.startswith("public_html/"): + path = path.replace("public_html/", "", 1) + if isinstance(owner, Member): + username = owner.crsid + top = "home" + else: + username = owner.society + top = "societies" + base = os.path.join("/public", top, username, "public_html") + target = os.path.abspath(os.path.join(base, path)) + if not target.startswith(base): + return path, "Document roots must be inside your public_html directory." + elif base == target: + return "", "We've cleared your document root as it appears to be your public_html directory." + elif not os.path.exists(target): + return path, "This document root doesn't exist, or isn't accessible to the webserver. Create the directory first, then try again." + clean = target[len(base) + 1:] + if clean != path: + return clean, "We've fixed your document root to its canonical version; submit again to confirm." + return path, None + + # Template helpers def sif(variable, val): """"string if": `val` if `variable` is defined and truthy, else ''""" From 0eae847f7c26d67069d7791f95fd7f1314b8f51f Mon Sep 17 00:00:00 2001 From: Richard Allitt Date: Sat, 16 Nov 2024 00:29:17 +0000 Subject: [PATCH 3/4] Typo fix and error message tweak --- control/templates/shared/add_vhost_test.html | 2 +- control/webapp/utils.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/control/templates/shared/add_vhost_test.html b/control/templates/shared/add_vhost_test.html index 306eea0..8339ae9 100644 --- a/control/templates/shared/add_vhost_test.html +++ b/control/templates/shared/add_vhost_test.html @@ -54,7 +54,7 @@

Test custom domain

{% if valid[domain] == (true, true) or valid[prefixed] == (true, true) %} - {% elif valid[domain] != (none, none) or valid[domain] != (none, none) %} + {% elif valid[domain] != (none, none) or valid[prefixed] != (none, none) %} {% endif %} diff --git a/control/webapp/utils.py b/control/webapp/utils.py index dd54f84..971e65e 100644 --- a/control/webapp/utils.py +++ b/control/webapp/utils.py @@ -96,7 +96,7 @@ def validate_domain_docroot(owner, path): if not path: return path, None if any(ch in path for ch in string.whitespace + "\\" + '"' + "'"): - return path, "Document roots cannot contain spaces or quotes." + return path, "Document roots cannot contain spaces, backslashes or quotes." if path.startswith("public_html/"): path = path.replace("public_html/", "", 1) if isinstance(owner, Member): From d14a52c28761f4cd5dcb7f3b56ffb94405ef05c4 Mon Sep 17 00:00:00 2001 From: Richard Allitt Date: Sat, 16 Nov 2024 09:40:39 +0000 Subject: [PATCH 4/4] Use os.path to properly check docroot base path --- control/webapp/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/control/webapp/utils.py b/control/webapp/utils.py index 971e65e..1537f35 100644 --- a/control/webapp/utils.py +++ b/control/webapp/utils.py @@ -107,7 +107,7 @@ def validate_domain_docroot(owner, path): top = "societies" base = os.path.join("/public", top, username, "public_html") target = os.path.abspath(os.path.join(base, path)) - if not target.startswith(base): + if os.path.commonpath((base, target)) != base: return path, "Document roots must be inside your public_html directory." elif base == target: return "", "We've cleared your document root as it appears to be your public_html directory."