Chia v2.6.0#1050
Conversation
| landing_path = os.path.join(current_app.root_path, 'static', 'landings', '{0}.txt'.format(lang)) | ||
|
|
||
| # Fallback if specific lang not found? Original code just opened it contextually. | ||
| if not os.path.exists(landing_path): |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
In general, to fix uncontrolled path use, normalize or constrain any user-influenced value before using it in a filesystem path and ensure the resulting path stays within an intended directory (or comes from a strict allowlist). Here, the path is supposed to point to a landing text file within static/landings, with the filename based on a language code. The safest and simplest fix without changing functionality is to validate lang against current_app.config['LANGUAGES'] (and possibly a small set of hardcoded fallbacks) and fall back to "en" if it’s not allowed. This ensures we never insert arbitrary header contents into the filename.
Concretely in web/blueprints/landing.py, inside landing() after lang = get_lang(request), we can clamp lang to an allowlist: check if lang is in current_app.config['LANGUAGES'], and if not, set it to "en". Optionally, we can also ensure that lang does not contain path separators just in case. Then we build landing_path exactly as before, but now with a guaranteed safe lang. No changes are required in web/utils.py; we only add a small block of validation code in landing(). This does not alter intended behavior because get_lang is already meant to produce a value from LANGUAGES or "en"; we’re simply enforcing that expectation defensively at the point where the path is built.
| @@ -15,6 +15,11 @@ | ||
| current_app.logger.info("ACCEPT IS {0}".format(accept)) | ||
| current_app.logger.info("LANGUAGES IS {0}".format(current_app.config['LANGUAGES'])) | ||
| lang = get_lang(request) | ||
| # Ensure lang used in filesystem path is restricted to known safe values. | ||
| allowed_langs = current_app.config.get('LANGUAGES', []) | ||
| if lang not in allowed_langs: | ||
| current_app.logger.debug("LOCALE: Unrecognized lang '%s' from get_lang, falling back to 'en'", lang) | ||
| lang = "en" | ||
|
|
||
| # Need to handle path to static files correctly if cwd changes, but Flask handles it relative to app root usually. | ||
| # Original code: open('web/static/landings/{0}.txt'...) |
| landing_path = os.path.join(current_app.root_path, 'static', 'landings', 'en.txt') # Fallback | ||
|
|
||
| try: | ||
| msg = random.choice(list(open(landing_path))) |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
In general, to fix uncontrolled path usage you either (a) validate the untrusted component (lang) against an allow list of safe values before using it in a path, or (b) validate the final path against a safe base directory using normalization (for example with os.path.normpath) to ensure no traversal outside that directory. Here, the cleanest, least-invasive fix is to enforce that lang must be one of the configured languages in current_app.config['LANGUAGES'], and to fall back to a default known-safe language (like "en") otherwise. This keeps existing behavior (using config-defined languages and falling back to English) while making the taint boundary explicit.
Concretely, in web/blueprints/landing.py inside the landing() view, right after lang = get_lang(request), we’ll add a simple validation step:
- Normalize
langto a string and check membership incurrent_app.config['LANGUAGES']. - If it’s not in the allow list, log a debug or warning message and set
lang = 'en'(or another safe default). - Optionally, we can add a secondary safety net by explicitly constructing the landing directory as a
base_path, usingos.path.joinand thenos.path.normpath, and verifying that the normalizedlanding_pathstill resides underbase_path. Given the allow list, this is somewhat redundant but further strengthens the defense.
We do not need to modify web/utils.py because get_lang already returns values tied to app.config['LANGUAGES']; the security-relevant fix is to enforce this at the point of filesystem access. No new imports are needed beyond os, which is already imported in landing.py. All changes are localized to web/blueprints/landing.py in the landing function.
| @@ -15,17 +15,26 @@ | ||
| current_app.logger.info("ACCEPT IS {0}".format(accept)) | ||
| current_app.logger.info("LANGUAGES IS {0}".format(current_app.config['LANGUAGES'])) | ||
| lang = get_lang(request) | ||
|
|
||
|
|
||
| # Ensure that lang is one of the configured, trusted languages before using it in a path. | ||
| allowed_langs = current_app.config.get('LANGUAGES', []) | ||
| if lang not in allowed_langs: | ||
| current_app.logger.debug( | ||
| "LOCALE: Unrecognized or untrusted locale '%s', falling back to 'en'", lang | ||
| ) | ||
| lang = 'en' | ||
|
|
||
| # Need to handle path to static files correctly if cwd changes, but Flask handles it relative to app root usually. | ||
| # Original code: open('web/static/landings/{0}.txt'...) | ||
| # We should use os.path.join(current_app.root_path, 'static', 'landings', ...) | ||
|
|
||
| landing_path = os.path.join(current_app.root_path, 'static', 'landings', '{0}.txt'.format(lang)) | ||
|
|
||
|
|
||
| base_path = os.path.join(current_app.root_path, 'static', 'landings') | ||
| landing_path = os.path.join(base_path, '{0}.txt'.format(lang)) | ||
|
|
||
| # Fallback if specific lang not found? Original code just opened it contextually. | ||
| if not os.path.exists(landing_path): | ||
| landing_path = os.path.join(current_app.root_path, 'static', 'landings', 'en.txt') # Fallback | ||
| landing_path = os.path.join(base_path, 'en.txt') # Fallback | ||
|
|
||
| try: | ||
| msg = random.choice(list(open(landing_path))) | ||
| except Exception as e: |
| if log_type in [ 'alerts', 'farming', 'plotting', 'archiving', 'apisrv', 'webui', 'pooling', 'rewards']: | ||
| log_id = request.args.get("log_id") | ||
| blockchain = request.args.get("blockchain") | ||
| return log_handler.get_log_lines(get_lang(request), w, log_type, log_id, blockchain) |
Check failure
Code scanning / CodeQL
Reflected server-side cross-site scripting High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
In general, to fix reflected XSS, any value derived from user input must be properly escaped before being embedded in an HTTP response that may be interpreted as HTML. Since we are operating in a Flask context, we can use flask.escape (or markupsafe.escape via Flask) to HTML‑escape user-provided strings before returning them. This preserves the visible content for users but neutralizes any HTML/JS they may contain.
The concrete issue is in web/actions/log_handler.py in get_log_lines. The blockchain argument can contain user input, and when an exception occurs, blockchain is interpolated into the error message returned from line 31. To fix this without changing existing functionality, we should escape blockchain at the point of use in the error message. The successful path (response.content.decode('utf-8')) can remain unchanged, since response content comes from another component and may intentionally contain log data; the CodeQL path specifically flags the error message string.
Steps:
- In
web/actions/log_handler.py, importescapefromflaskalongside the existing imports. - In
get_log_lines, inside theexceptblock, before formatting the error message, applyescapetoblockchainand use the escaped value in the formatted string. To avoid double-escaping ifblockchainisNoneor non-string, coerce to string first.
This keeps behavior identical for normal text but prevents interpreted HTML/JS when the error message is rendered in a browser.
| @@ -13,7 +13,7 @@ | ||
| import traceback | ||
| import yaml | ||
|
|
||
| from flask import Flask, jsonify, abort, request, flash | ||
| from flask import Flask, jsonify, abort, request, flash, escape | ||
|
|
||
| from web import app, db, utils | ||
|
|
||
| @@ -28,4 +28,5 @@ | ||
| return response.content.decode('utf-8') | ||
| except: | ||
| app.logger.info(traceback.format_exc()) | ||
| return 'Failed to load log file from {0}:{1} running {2}'.format(worker.hostname, worker.port, blockchain) | ||
| safe_blockchain = escape(str(blockchain)) if blockchain is not None else '' | ||
| return 'Failed to load log file from {0}:{1} running {2}'.format(worker.hostname, worker.port, safe_blockchain) |
| response = None | ||
| try: | ||
| if config_type == "alerts": | ||
| response = make_response(chiadog.load_config(w, request.args.get('blockchain')), 200) |
Check failure
Code scanning / CodeQL
Reflected server-side cross-site scripting High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
In general, to prevent this class of issue you either (a) ensure that any data derived from user input and reflected back to the browser is safely encoded for the context in which it is used, or (b) constrain and validate the user input so it cannot lead to attacker‑controlled content being produced. Since this endpoint is serving raw YAML and not HTML, HTML escaping the entire config would break functionality. The better approach is to ensure that the blockchain parameter is constrained to a safe, known set of values, so a user cannot use it to influence what is fetched and returned.
The single best minimal fix here is to validate request.args.get('blockchain') against the blockchains that are already known/enabled in the application, and reject any value that is not in that set before it is passed into chiadog.load_config (and the other load_* calls). The project already has globals.enabled_blockchains(), which returns the blockchains the app supports, so we can reuse that. Concretely, in web/blueprints/settings.py in the config view, directly after reading config_type (and before calling worker.get_worker or any load_* functions), we should:
- Read
blockchain = request.args.get('blockchain'). - Fetch the list of enabled blockchains via
globals.enabled_blockchains(). - If
blockchainis missing or not in that list, abort with400(bad request).
Then, reuse this validated local blockchain variable when calling worker.get_worker and all subsequent load_* calls, as well as in logging and error messages, instead of calling request.args.get('blockchain') repeatedly. This keeps existing behavior (only valid blockchains work) while ensuring tainted input is validated before influencing an external call or reflected content. No changes are needed in web/actions/chiadog.py or web/utils.py for this fix.
| @@ -29,31 +29,35 @@ | ||
| @settings_bp.route('/settings/config/<path:path>') | ||
| def config(path): | ||
| config_type = request.args.get('type') | ||
| w = worker.get_worker(request.args.get('worker'), request.args.get('blockchain')) | ||
| blockchain = request.args.get('blockchain') | ||
| enabled_blockchains = globals.enabled_blockchains() | ||
| if not blockchain or blockchain not in enabled_blockchains: | ||
| abort(400, "Unsupported or missing blockchain: {0}".format(escape(blockchain) if blockchain else "None")) | ||
| w = worker.get_worker(request.args.get('worker'), blockchain) | ||
| if not w: | ||
| current_app.logger.info(_l("No worker at %(worker)s for blockchain %(blockchain)s. Please select another blockchain.", | ||
| worker=request.args.get('worker'), blockchain=request.args.get('blockchain'))) | ||
| worker=request.args.get('worker'), blockchain=blockchain)) | ||
| abort(404) | ||
| response = None | ||
| try: | ||
| if config_type == "alerts": | ||
| response = make_response(chiadog.load_config(w, request.args.get('blockchain')), 200) | ||
| response = make_response(chiadog.load_config(w, blockchain), 200) | ||
| elif config_type == "farming": | ||
| response = make_response(chia.load_config(w, request.args.get('blockchain')), 200) | ||
| response = make_response(chia.load_config(w, blockchain), 200) | ||
| elif config_type == "plotting": | ||
| [replaced, config] = plotman.load_config(w, request.args.get('blockchain')) | ||
| [replaced, config] = plotman.load_config(w, blockchain) | ||
| response = make_response(config, 200) | ||
| response.headers.set('ConfigReplacementsOccurred', replaced) | ||
| elif config_type == "plotting_dirs": | ||
| response = make_response(plotman.load_dirs(w, request.args.get('blockchain')), 200) | ||
| response = make_response(plotman.load_dirs(w, blockchain), 200) | ||
| elif config_type == "plotting_schedule": | ||
| response = make_response(plotman.load_schedule(w, request.args.get('blockchain')), 200) | ||
| response = make_response(plotman.load_schedule(w, blockchain), 200) | ||
| elif config_type == "tools": | ||
| response = make_response(forktools.load_config(w, request.args.get('blockchain')), 200) | ||
| response = make_response(forktools.load_config(w, blockchain), 200) | ||
| else: | ||
| abort(400, "Unsupported config type: {0}".format(config_type)) | ||
| except requests.exceptions.ConnectionError as ex: | ||
| response = make_response(_("No responding fullnode found for %(blockchain)s. Please check your workers.", blockchain=escape(request.args.get('blockchain')))) | ||
| response = make_response(_("No responding fullnode found for %(blockchain)s. Please check your workers.", blockchain=escape(blockchain))) | ||
|
|
||
| if response: | ||
| response.mimetype = "application/x-yaml" |
| if config_type == "alerts": | ||
| response = make_response(chiadog.load_config(w, request.args.get('blockchain')), 200) | ||
| elif config_type == "farming": | ||
| response = make_response(chia.load_config(w, request.args.get('blockchain')), 200) |
Check failure
Code scanning / CodeQL
Reflected server-side cross-site scripting High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
General approach: Prevent arbitrary user-controlled strings from being used as the blockchain path segment in the internal "/configs/farming/"+ blockchain URL and from flowing unvalidated through the system. The safest fix, preserving existing functionality, is to normalize and validate the blockchain parameter in the controller (settings.config) before passing it to chia.load_config (and all similar calls), ensuring it matches an allowed set of blockchain identifiers. If invalid, we return a 400 error. This prevents path manipulation and constrains what can be reflected back, mitigating XSS and related issues.
Best concrete fix without changing external behavior: In web/blueprints/settings.py, inside the config view, read blockchain once from request.args, then:
- Validate it to contain only safe characters (e.g., letters, digits, underscore, hyphen) with a simple regular expression.
- Optionally, if the application already has a canonical list of enabled blockchains (
globals.enabled_blockchains()), restrictblockchainto that set (without changing imports). - Use this validated value (
safe_blockchain) everywhere in the function: when callingworker.get_worker(...),chiadog.load_config,chia.load_config,plotman.*,forktools.load_config, and when constructing the error message. Do not pass rawrequest.args.get('blockchain')to any of these calls anymore.
This change is localized entirely to web/blueprints/settings.py. We don’t need to alter web/actions/chia.py or web/utils.py, because once the blockchain argument is validated, the subsequent use in a URL path and response content is constrained. To implement the change we need:
- A small helper inside
configto validate/normalizeblockchain, using Python’s standardremodule. - A new import
import reat the top ofweb/blueprints/settings.py. - Replacing direct uses of
request.args.get('blockchain')inconfigwith the validatedblockchainvariable.
| @@ -5,6 +5,7 @@ | ||
| from web.actions import forktools, worker, chiadog, chia, plotman | ||
| from web.utils import find_selected_worker | ||
| import requests | ||
| import re | ||
|
|
||
| settings_bp = Blueprint('settings', __name__) | ||
|
|
||
| @@ -29,31 +30,38 @@ | ||
| @settings_bp.route('/settings/config/<path:path>') | ||
| def config(path): | ||
| config_type = request.args.get('type') | ||
| w = worker.get_worker(request.args.get('worker'), request.args.get('blockchain')) | ||
| blockchain = request.args.get('blockchain', '') | ||
| worker_name = request.args.get('worker') | ||
|
|
||
| # Validate blockchain to avoid using arbitrary user input in internal paths/responses | ||
| if not blockchain or not re.match(r'^[A-Za-z0-9_-]+$', blockchain): | ||
| abort(400, "Invalid blockchain parameter") | ||
|
|
||
| w = worker.get_worker(worker_name, blockchain) | ||
| if not w: | ||
| current_app.logger.info(_l("No worker at %(worker)s for blockchain %(blockchain)s. Please select another blockchain.", | ||
| worker=request.args.get('worker'), blockchain=request.args.get('blockchain'))) | ||
| worker=worker_name, blockchain=blockchain)) | ||
| abort(404) | ||
| response = None | ||
| try: | ||
| if config_type == "alerts": | ||
| response = make_response(chiadog.load_config(w, request.args.get('blockchain')), 200) | ||
| response = make_response(chiadog.load_config(w, blockchain), 200) | ||
| elif config_type == "farming": | ||
| response = make_response(chia.load_config(w, request.args.get('blockchain')), 200) | ||
| response = make_response(chia.load_config(w, blockchain), 200) | ||
| elif config_type == "plotting": | ||
| [replaced, config] = plotman.load_config(w, request.args.get('blockchain')) | ||
| [replaced, config] = plotman.load_config(w, blockchain) | ||
| response = make_response(config, 200) | ||
| response.headers.set('ConfigReplacementsOccurred', replaced) | ||
| elif config_type == "plotting_dirs": | ||
| response = make_response(plotman.load_dirs(w, request.args.get('blockchain')), 200) | ||
| response = make_response(plotman.load_dirs(w, blockchain), 200) | ||
| elif config_type == "plotting_schedule": | ||
| response = make_response(plotman.load_schedule(w, request.args.get('blockchain')), 200) | ||
| response = make_response(plotman.load_schedule(w, blockchain), 200) | ||
| elif config_type == "tools": | ||
| response = make_response(forktools.load_config(w, request.args.get('blockchain')), 200) | ||
| response = make_response(forktools.load_config(w, blockchain), 200) | ||
| else: | ||
| abort(400, "Unsupported config type: {0}".format(config_type)) | ||
| except requests.exceptions.ConnectionError as ex: | ||
| response = make_response(_("No responding fullnode found for %(blockchain)s. Please check your workers.", blockchain=escape(request.args.get('blockchain')))) | ||
| response = make_response(_("No responding fullnode found for %(blockchain)s. Please check your workers.", blockchain=escape(blockchain))) | ||
|
|
||
| if response: | ||
| response.mimetype = "application/x-yaml" |
| response = make_response(chia.load_config(w, request.args.get('blockchain')), 200) | ||
| elif config_type == "plotting": | ||
| [replaced, config] = plotman.load_config(w, request.args.get('blockchain')) | ||
| response = make_response(config, 200) |
Check failure
Code scanning / CodeQL
Reflected server-side cross-site scripting High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
General approach: Ensure that configuration content sent back to the client cannot contain executable HTML/JS derived from untrusted sources. There are two common strategies: (1) encode/escape the data before returning it, or (2) restrict the data so only safe characters are allowed (validation / sanitization), especially if the content is intended to be structured text like YAML. Since this endpoint already uses a specific mimetype and seems to serve configuration files, the least invasive fix is to sanitize the config string before returning it, removing potentially dangerous constructs such as HTML tags or <script>-like payloads, while keeping the YAML structure as intact as possible.
Best concrete fix here: Introduce a small helper in web/actions/plotman.py to “sanitize” any config text returned from the worker and use it in load_config. This helper can use a simple, well-known pattern: remove any <...> tags and strip control characters that are not typical in YAML. This does not change existing logic (replacements and structure are preserved aside from suspicious markup), but it breaks any attempted HTML/JS payloads an attacker might inject. After that, the config string that reaches settings.config is no longer directly reflecting raw untrusted markup.
Changes needed:
- In
web/actions/plotman.py:- Add a helper function
sanitize_config(config_text)nearload_configthat:- Uses
re.subto strip potentially dangerous HTML tags and script/style blocks from the text. - Optionally removes NULL bytes which can be used for obfuscation.
- Uses
- Update
load_configso that after readingconfigfromutils.send_get(...).content.decode('utf-8')it passes the content throughsanitize_configbefore further processing (splitting into lines and applying replacements).
- Add a helper function
- No change is needed in
web/blueprints/settings.py, because by the time it receivesconfigfromplotman.load_config, the content is already sanitized.
This keeps existing behavior (still YAML text, still supports replacements, same API) while preventing direct reflection of untrusted HTML/JS markup.
| @@ -196,6 +196,24 @@ | ||
| replacements.append([ r'pool_contract_address:\s+REPLACE_WITH_THE_REAL_VALUE.*$', 'pool_contract_address: '+ pool_contract_address]) | ||
| return replacements | ||
|
|
||
| def sanitize_config(config_text): | ||
| """ | ||
| Perform basic sanitization on configuration text to avoid returning | ||
| unexpected executable markup (for example, HTML/JS) in responses. | ||
|
|
||
| This is a defensive measure and should not affect normal YAML content. | ||
| """ | ||
| if not isinstance(config_text, str): | ||
| return config_text | ||
| # Remove NULL bytes which are not valid in text responses and can be used | ||
| # for obfuscation. | ||
| cleaned = config_text.replace('\x00', '') | ||
| # Strip out any obvious HTML tags to avoid accidental script/style injection | ||
| # if this content is ever rendered in a browser context. | ||
| cleaned = re.sub(r'<[^>]+>', '', cleaned) | ||
| return cleaned | ||
|
|
||
|
|
||
| def load_config(plotter, blockchain): | ||
| replacements = [] | ||
| try: | ||
| @@ -204,7 +222,8 @@ | ||
| app.logger.info("Unable to load replacements on install with mode={0}".format(os.environ['mode'])) | ||
| app.logger.info(traceback.format_exc()) | ||
| lines = [] | ||
| config = utils.send_get(plotter, "/configs/plotting/" + blockchain, debug=False).content.decode('utf-8') | ||
| raw_config = utils.send_get(plotter, "/configs/plotting/" + blockchain, debug=False).content.decode('utf-8') | ||
| config = sanitize_config(raw_config) | ||
| replaces = 0 | ||
| for line in config.splitlines(): | ||
| for replacement in replacements: |
| response = make_response(config, 200) | ||
| response.headers.set('ConfigReplacementsOccurred', replaced) | ||
| elif config_type == "plotting_dirs": | ||
| response = make_response(plotman.load_dirs(w, request.args.get('blockchain')), 200) |
Check failure
Code scanning / CodeQL
Reflected server-side cross-site scripting High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
In general, to fix reflected XSS you must ensure that any data that could be influenced by users is either (a) properly escaped before being put into an HTML/JavaScript context, or (b) served with a content type and headers that prevent the browser from interpreting it as active content (e.g., disabling MIME sniffing, forcing download). Here, the problematic flow is that arbitrary content retrieved via plotman.load_dirs (ultimately from utils.send_get) is returned directly to the client; even though the mimetype is application/x-yaml, we should ensure the browser never executes it as HTML/JS and, ideally, avoid embedding it in HTML at all.
The minimal, non-breaking fix is to keep returning the YAML content as-is, but explicitly force it to be treated as a plain text download and prevent MIME sniffing. We can achieve this by adding a Content-Disposition: attachment header so the browser downloads the file instead of rendering it inline, and a X-Content-Type-Options: nosniff header to stop browsers from guessing a more permissive content type. These headers should be set on all branches in the config view that return YAML configurations, not just plotting_dirs, since they all share the same risk pattern: untrusted or semi-trusted content being reflected. We’ll implement this directly in web/blueprints/settings.py by, after constructing a non-None response, setting those security headers alongside the already-set mimetype = "application/x-yaml".
Concretely, in web/blueprints/settings.py, inside the config function, we’ll keep the existing logic for determining response and setting response.mimetype. Immediately after if response:, we will also set response.headers["X-Content-Type-Options"] = "nosniff" and response.headers["Content-Disposition"] = "attachment; filename=config.yaml". No changes are needed in web/utils.py or web/actions/plotman.py for this particular XSS fix.
| @@ -57,4 +57,6 @@ | ||
|
|
||
| if response: | ||
| response.mimetype = "application/x-yaml" | ||
| response.headers['X-Content-Type-Options'] = 'nosniff' | ||
| response.headers['Content-Disposition'] = 'attachment; filename="config.yaml"' | ||
| return response |
| elif config_type == "plotting_dirs": | ||
| response = make_response(plotman.load_dirs(w, request.args.get('blockchain')), 200) | ||
| elif config_type == "plotting_schedule": | ||
| response = make_response(plotman.load_schedule(w, request.args.get('blockchain')), 200) |
Check failure
Code scanning / CodeQL
Reflected server-side cross-site scripting High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
General approach: ensure that the blockchain parameter coming from the request is constrained to an expected set of safe values before it is passed down into plotman.load_schedule (and other config loaders) and ultimately reflected back to the client. Instead of escaping here (the value is not directly injected into HTML), the right fix is to validate and reject/normalize unexpected values, preventing attacker‑controlled arbitrary strings from influencing downstream behavior.
Best concrete fix without changing existing functionality: in web/blueprints/settings.py, within the config view, normalize and validate the blockchain query parameter early, before any call to worker.get_worker or the various *.load_config / plotman.load_schedule functions. The project already has globals.enabled_blockchains() used in the same file, so we can reuse this to define an allowlist. If the supplied blockchain is not in the enabled list, respond with a 400 error. Then, use this validated blockchain variable instead of repeatedly calling request.args.get('blockchain'). This keeps behavior unchanged for valid inputs while blocking malicious ones, and removes the taint from CodeQL’s perspective.
Concretely:
- In
config(path), after readingconfig_type, obtainblockchain_raw = request.args.get('blockchain')andenabled = globals.enabled_blockchains(). - If
blockchain_rawis missing or not inenabled, abort with a 400 and a short message. - Store the validated value in a local
blockchainvariable. - Replace all uses of
request.args.get('blockchain')in this function withblockchain, including:- the call to
worker.get_worker - all
*.load_config/plotman.load_*calls - the 404 log message
- the ConnectionError message (still wrapped by
escape).
No new imports are needed;globalsandescapeare already imported.
- the call to
| @@ -29,31 +29,35 @@ | ||
| @settings_bp.route('/settings/config/<path:path>') | ||
| def config(path): | ||
| config_type = request.args.get('type') | ||
| w = worker.get_worker(request.args.get('worker'), request.args.get('blockchain')) | ||
| blockchain = request.args.get('blockchain') | ||
| enabled_blockchains = globals.enabled_blockchains() | ||
| if not blockchain or blockchain not in enabled_blockchains: | ||
| abort(400, "Unsupported or missing blockchain: {0}".format(blockchain)) | ||
| w = worker.get_worker(request.args.get('worker'), blockchain) | ||
| if not w: | ||
| current_app.logger.info(_l("No worker at %(worker)s for blockchain %(blockchain)s. Please select another blockchain.", | ||
| worker=request.args.get('worker'), blockchain=request.args.get('blockchain'))) | ||
| worker=request.args.get('worker'), blockchain=blockchain)) | ||
| abort(404) | ||
| response = None | ||
| try: | ||
| if config_type == "alerts": | ||
| response = make_response(chiadog.load_config(w, request.args.get('blockchain')), 200) | ||
| response = make_response(chiadog.load_config(w, blockchain), 200) | ||
| elif config_type == "farming": | ||
| response = make_response(chia.load_config(w, request.args.get('blockchain')), 200) | ||
| response = make_response(chia.load_config(w, blockchain), 200) | ||
| elif config_type == "plotting": | ||
| [replaced, config] = plotman.load_config(w, request.args.get('blockchain')) | ||
| [replaced, config] = plotman.load_config(w, blockchain) | ||
| response = make_response(config, 200) | ||
| response.headers.set('ConfigReplacementsOccurred', replaced) | ||
| elif config_type == "plotting_dirs": | ||
| response = make_response(plotman.load_dirs(w, request.args.get('blockchain')), 200) | ||
| response = make_response(plotman.load_dirs(w, blockchain), 200) | ||
| elif config_type == "plotting_schedule": | ||
| response = make_response(plotman.load_schedule(w, request.args.get('blockchain')), 200) | ||
| response = make_response(plotman.load_schedule(w, blockchain), 200) | ||
| elif config_type == "tools": | ||
| response = make_response(forktools.load_config(w, request.args.get('blockchain')), 200) | ||
| response = make_response(forktools.load_config(w, blockchain), 200) | ||
| else: | ||
| abort(400, "Unsupported config type: {0}".format(config_type)) | ||
| except requests.exceptions.ConnectionError as ex: | ||
| response = make_response(_("No responding fullnode found for %(blockchain)s. Please check your workers.", blockchain=escape(request.args.get('blockchain')))) | ||
| response = make_response(_("No responding fullnode found for %(blockchain)s. Please check your workers.", blockchain=escape(blockchain))) | ||
|
|
||
| if response: | ||
| response.mimetype = "application/x-yaml" |
| elif config_type == "plotting_schedule": | ||
| response = make_response(plotman.load_schedule(w, request.args.get('blockchain')), 200) | ||
| elif config_type == "tools": | ||
| response = make_response(forktools.load_config(w, request.args.get('blockchain')), 200) |
Check failure
Code scanning / CodeQL
Reflected server-side cross-site scripting High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
In general, to fix this kind of problem you either (a) escape or encode any user input before placing it into an HTML/JS context, and/or (b) validate and constrain user input so that only safe, expected values are accepted and anything else is rejected. In this case, blockchain appears to be an identifier that should come from a finite set of allowed blockchains, not arbitrary user-controlled strings. Validating it against the known enabled blockchains before using it in worker.get_worker and forktools.load_config closes the taint chain and prevents abuse.
The least intrusive, functionality-preserving fix is to sanitize request.args.get('blockchain') in the config view in web/blueprints/settings.py. We can load the allowed blockchains via globals.enabled_blockchains() (already used in tools()), normalize the input, and ensure it’s a member of that set. If it is not, we abort with 400 rather than passing the untrusted string through. That way forktools.load_config always receives a vetted blockchain name. No changes are needed to forktools.load_config itself or to web/utils.py, since the path parameter will no longer be tainted by arbitrary user input.
Concretely:
- In
config(path)inweb/blueprints/settings.py, immediately after readingconfig_type, read theblockchainargument into a local variable, validate it against the list fromglobals.enabled_blockchains(), and abort on invalid values. - Use this validated
blockchainvariable when callingworker.get_workerand all downstream*.load_config/load_dirs/load_schedulecalls, instead of repeatedly callingrequest.args.get('blockchain'). - Keep the existing use of
escapein the error message as-is, since that is already safe and does not affect the taint chain once we stop using rawrequest.args.
No new imports are needed; globals is already imported in this file.
| @@ -29,31 +29,36 @@ | ||
| @settings_bp.route('/settings/config/<path:path>') | ||
| def config(path): | ||
| config_type = request.args.get('type') | ||
| w = worker.get_worker(request.args.get('worker'), request.args.get('blockchain')) | ||
| # Validate blockchain parameter against enabled blockchains | ||
| blockchain = request.args.get('blockchain') | ||
| enabled_blockchains = globals.enabled_blockchains() | ||
| if blockchain not in enabled_blockchains: | ||
| abort(400, "Unsupported blockchain: {0}".format(escape(blockchain) if blockchain is not None else "None")) | ||
| w = worker.get_worker(request.args.get('worker'), blockchain) | ||
| if not w: | ||
| current_app.logger.info(_l("No worker at %(worker)s for blockchain %(blockchain)s. Please select another blockchain.", | ||
| worker=request.args.get('worker'), blockchain=request.args.get('blockchain'))) | ||
| worker=request.args.get('worker'), blockchain=blockchain)) | ||
| abort(404) | ||
| response = None | ||
| try: | ||
| if config_type == "alerts": | ||
| response = make_response(chiadog.load_config(w, request.args.get('blockchain')), 200) | ||
| response = make_response(chiadog.load_config(w, blockchain), 200) | ||
| elif config_type == "farming": | ||
| response = make_response(chia.load_config(w, request.args.get('blockchain')), 200) | ||
| response = make_response(chia.load_config(w, blockchain), 200) | ||
| elif config_type == "plotting": | ||
| [replaced, config] = plotman.load_config(w, request.args.get('blockchain')) | ||
| [replaced, config] = plotman.load_config(w, blockchain) | ||
| response = make_response(config, 200) | ||
| response.headers.set('ConfigReplacementsOccurred', replaced) | ||
| elif config_type == "plotting_dirs": | ||
| response = make_response(plotman.load_dirs(w, request.args.get('blockchain')), 200) | ||
| response = make_response(plotman.load_dirs(w, blockchain), 200) | ||
| elif config_type == "plotting_schedule": | ||
| response = make_response(plotman.load_schedule(w, request.args.get('blockchain')), 200) | ||
| response = make_response(plotman.load_schedule(w, blockchain), 200) | ||
| elif config_type == "tools": | ||
| response = make_response(forktools.load_config(w, request.args.get('blockchain')), 200) | ||
| response = make_response(forktools.load_config(w, blockchain), 200) | ||
| else: | ||
| abort(400, "Unsupported config type: {0}".format(config_type)) | ||
| except requests.exceptions.ConnectionError as ex: | ||
| response = make_response(_("No responding fullnode found for %(blockchain)s. Please check your workers.", blockchain=escape(request.args.get('blockchain')))) | ||
| response = make_response(_("No responding fullnode found for %(blockchain)s. Please check your workers.", blockchain=escape(blockchain))) | ||
|
|
||
| if response: | ||
| response.mimetype = "application/x-yaml" |
No description provided.