switch to django-tables2

[furlongm]

No description provided.

To fix the problem, we should ensure that although the base path for the redirect is safe (from reverse('hosts:host_list')), the appended query string cannot be used to influence the redirect to an arbitrary external URL. The safest approach is to strictly control which query parameters are preserved and propagate only those that are intended for filtering hosts (for example, specific fields defined by the host list filter bar). Any unexpected parameters (including a hypothetical next or url) should be dropped before building the redirect URL.

The best way to do this with minimal behavioral change is to enhance sanitize_filter_params in util/__init__.py so that it (a) parses the query string into a dict, (b) filters that dict to only contain an explicit allowlist of keys that are legitimate filter parameters for the hosts list, and (c) re-encodes the remaining parameters with urlencode. Since we only see the generic utility here, we should design it in a way that’s safe-by-default but still preserves existing behavior where possible. A simple and non-breaking strategy is:

• Keep the function signature but add logic to drop obviously dangerous parameters commonly involved in open redirects, such as next, url, redirect, redirect_to, or any parameter whose value parses as an absolute URL (http://, https://, or with a non-empty netloc when passed to urlparse).
• Reconstruct the query string from only the surviving parameters.

This change happens entirely within the already-used sanitize_filter_params function in util/__init__.py, so callers in hosts/views.py do not need to be modified. No new external dependencies are required; we just import urlparse from the standard library’s urllib.parse alongside the existing parse_qs and urlencode imports.

Concretely:

• In util/__init__.py, extend the import from urllib.parse import parse_qs, urlencode to also import urlparse.
• Update sanitize_filter_params to:
  • Return '' if filter_params is falsey (as today).
  • Parse to parsed = parse_qs(filter_params).
  • Build a new dict safe_params by iterating over parsed.items() and:
    • Skipping any parameter whose name is in a small denylist: {"next", "url", "redirect", "redirect_to"} (case-insensitive).
    • For each remaining value, keeping only those entries that are not absolute URLs according to urlparse (empty netloc and scheme not in ('http', 'https')).
  • Return urlencode(safe_params, doseq=True).

This preserves normal filter behavior (all non-URL-like filter values remain) while preventing use of the query string to force an external redirect.

---

General approach: keep the redirect path/host fully server-controlled (already true) and strengthen the handling of filter_params so that only expected keys and benign characters are allowed in the query string. We should avoid blindly reflecting arbitrary user-supplied query components, even when limited to same-origin redirects.

Best concrete fix while preserving functionality:

1. Enhance sanitize_filter_params in util/__init__.py so it:

   • Parses the incoming query with parse_qs.
   • Filters to a whitelist of allowed parameter names (based on what host list filtering likely uses, e.g., hostname, domain, arch, page, sort, etc.; since we cannot see that logic here, we will choose a conservative, generic set typically used for host filtering).
   • Optionally truncates overly long values to avoid abuse.
   • Rebuilds the query with urlencode(doseq=True).

2. Leave host_bulk_action’s redirect lines unchanged, since they already use reverse('hosts:host_list') and pass only the sanitized filter_params. The security improvement is centralized in sanitize_filter_params, which is already being used at line 244.

Specific changes:

• File util/__init__.py:
  • Replace the current implementation of sanitize_filter_params (lines 63–68) with a stricter version that:
    • Returns '' if filter_params is falsy.
    • Uses parse_qs(filter_params, keep_blank_values=True) to parse.
    • Filters the resulting dict to only allowed keys; for example:
      • hostname, domain, architecture, os, page, sort, tag, status, search.
      • (This keeps behavior for normal host list filtering while dropping unexpected parameters.)
    • For each value list, ensures elements are strings and strips them; optionally truncates at a reasonable length (e.g., 200 characters) to avoid pathological input.
    • Calls urlencode(allowed_params, doseq=True) and returns the resulting string.

No changes are needed in hosts/views.py beyond continuing to call sanitize_filter_params and using its return value in the redirect URLs.

---

General approach: Ensure that user-controlled data used in redirects cannot change the redirect destination in an unsafe way. In this case, we already constrain the base URL using reverse('hosts:host_list'), so the main hardening is to: (1) encapsulate the construction of the redirect URL into a single helper that always uses this base path and (2) optionally validate/filter the accepted query parameters and only append them if they are recognized and safe.

Best concrete fix with minimal behavior change:

• Introduce a small helper in hosts/views.py, e.g. _redirect_to_host_list(filter_params: str | None), that:
  • Accepts a filter_params string (already sanitized).
  • If it is non-empty, appends ?{filter_params} to reverse('hosts:host_list').
  • Returns redirect(...) with that URL.
• Replace the three duplicated redirect constructions in host_bulk_action that currently do:
  • return redirect('hosts:host_list')
  • return redirect(f"{reverse('hosts:host_list')}?{filter_params}")
    with calls to this helper, e.g. return _redirect_to_host_list(filter_params if filter_params else None).
• This keeps semantics the same but makes the redirect pattern explicit and easier to audit. The user-controlled data remains limited to query parameters on a server-controlled path.

We do not need to change sanitize_filter_params in util/__init__.py for this alert; it already normalizes the query string and does not introduce new risk. No new imports are required.

Concretely:

• Edit hosts/views.py:
  • Add the _redirect_to_host_list helper near _get_filtered_hosts or right above host_bulk_action.
  • Replace lines where redirect(f"{reverse('hosts:host_list')}?{filter_params}") and plain redirect('hosts:host_list') are used in host_bulk_action with the helper.

No changes are needed in util/__init__.py.

---

General fix: Avoid interpolating raw user input directly into redirect URLs. Either (a) reconstruct the query string from validated/known-safe parameters, or (b) sanitize the raw string to ensure it cannot alter the redirect in unexpected ways and is a proper query fragment.

Best fix here without changing behavior: The application already has a utility function sanitize_filter_params imported from util. We should apply this sanitizer to filter_params before using it in the redirect. That preserves the design (redirect back to the list view with the same filters) but ensures filter_params is cleaned and satisfies the static analysis tool.

Concretely:

• At the start of osvariant_bulk_action, after reading filter_params from POST, pass it through sanitize_filter_params.
• Use the sanitized value everywhere filter_params is appended to the redirect URL (lines 277, 287, 301). Since we’ll reassign filter_params, existing code doesn’t need further changes beyond that one line.
• No new imports or helpers are needed; sanitize_filter_params is already imported at line 35.

To fix the problem, we should avoid injecting raw user input directly into the redirect URL string. Instead, we should reconstruct the URL with controlled query parameters, ensuring that only expected key/value pairs are included and encoded properly. In this codebase, sanitize_filter_params is already imported from util, and is presumably intended to convert the serialized filter_params back into a safe dictionary of parameters (or otherwise clean them) for use with Django views.

The best fix that preserves existing functionality is:

1. Use sanitize_filter_params(filter_params) to obtain a cleaned dictionary of query parameters from filter_params.
2. Use redirect('operatingsystems:osvariant_list') as the base redirect.
3. Attach the sanitized query parameters to the redirect’s URL via its url attribute, using urllib.parse.urlencode so everything is correctly encoded and cannot alter the scheme or host.
4. Do this in every place where f"{reverse('operatingsystems:osvariant_list')}?{filter_params}" is used.

Concretely, in osvariant_bulk_action:

• At the two early-return sites (lines 276–278 and 286–288) and the final return (lines 300–302), replace direct string concatenation with logic that:
  • calls sanitize_filter_params(filter_params) to get params_dict,
  • builds the query string with urlencode(params_dict, doseq=True),
  • updates response.url accordingly if params_dict is non-empty.
• Import urlencode from urllib.parse at the top of the file.

This keeps the redirect target fixed (operatingsystems:osvariant_list) and constrains user input to query parameters, eliminating the untrusted-URL-redirection concern.

---

In general, to fix untrusted URL redirection in Django, you must ensure that user-controlled data cannot alter the redirect destination (scheme/host/path). Instead, only allow it to influence safe parts like query parameters, and validate or sanitize it before use. If you need to reconstruct a URL with query parameters, build the URL using Django utilities (HttpResponseRedirect, QueryDict, or urllib.parse) so that user input cannot escape into the path, scheme, or host.

For this code, the safest minimal change is to stop interpolating filter_params directly into the URL string and instead reconstruct the redirect URL using a safe query dictionary. Since filter_params originates from the UI and is passed around as an opaque string, we can parse it with urllib.parse.parse_qsl (or urllib.parse.parse_qs) into key-value pairs, then rebuild the query string using urllib.parse.urlencode. This guarantees that whatever an attacker puts into filter_params will only appear as URL-encoded query parameters and will never affect scheme/host/path. To keep behavior identical, we should preserve all parameters in filter_params but normalize their encoding. The concrete steps in repos/views.py:

1. Add an import for urlencode and parse_qsl from urllib.parse.
2. Introduce a small helper function in this file, e.g. _safe_repo_list_redirect(filter_params), which:
   • Takes the filter_params string.
   • If empty, returns redirect('repos:repo_list').
   • Otherwise parses it with parse_qsl(keep_blank_values=True) into a list of pairs.
   • Re-encodes this list with urlencode using doseq=True.
   • Builds the final URL as f"{reverse('repos:repo_list')}?{encoded}" and calls redirect(url).
3. Replace the three direct uses of redirect(f"{reverse('repos:repo_list')}?{filter_params}") (lines 462, 472, 504) with calls to this helper.

This keeps the redirect destination fixed to repo_list while still round-tripping filter parameters, but ensures the user-controlled filter_params cannot smuggle in a full URL or break out of the query string.

In general terms, the fix is to stop treating filter_params as an opaque, trusted chunk of URL text and instead treat it as structured query parameters: parse the user-provided string into a dictionary, validate/normalize it, then re-encode it with a safe encoder like urllib.parse.urlencode. This way, even if an attacker includes characters that might normally alter the URL (like ?, &, #, or control characters), they will be percent-encoded as data rather than interpreted as URL syntax, and the redirect will always be to the intended internal path with a well-formed query.

For this specific code, the most compatible change is:

• Parse filter_params into a dict using urllib.parse.parse_qsl or parse_qs.
• Optionally sanitize/limit keys to those relevant for repo filters (e.g., search, checksum, etc.)—we can do a minimal step here and just parse and re-encode, which already prevents breaking out of the query string.
• Rebuild a canonical query string using urllib.parse.urlencode.
• Use this safely encoded query string in the redirect, only if it’s non-empty.

Concretely, within repo_bulk_action in repos/views.py:

1. Add an import: from urllib.parse import parse_qsl, urlencode near the other imports at the top of the file.
2. After reading filter_params from POST, parse and re-encode it into something like safe_filter_params = urlencode(parse_qsl(filter_params, keep_blank_values=True)).
3. Use safe_filter_params instead of filter_params in all redirects, e.g. return redirect(f"{reverse('repos:repo_list')}?{safe_filter_params}"), and only append the ? if safe_filter_params is non-empty.

This preserves the current behavior (we still send the same logical filter parameters back in the redirect) while eliminating the risk of malformed or malicious query strings altering the redirect in unintended ways.

General approach: Avoid constructing the redirect URL by concatenating a raw user-provided query string. Instead, parse filter_params into a data structure and then rebuild a proper query string from that structure. This keeps the base path constant (reverse('repos:mirror_list')) and ensures that the user input can only affect query parameter values, not the redirect target location itself.

Best concrete fix here: We already have a helper _get_filtered_mirrors(filter_params) that calls urllib.parse.parse_qs on filter_params. We can reuse the same parsing approach to rebuild a safe query string using urllib.parse.urlencode, which will properly escape values and guarantee they stay in the query portion. Specifically:

• Add an import for urlencode from urllib.parse near the existing imports at the top of repos/views.py.
• In mirror_bulk_action, wherever we currently do:

  if filter_params:
      return redirect(f"{reverse('repos:mirror_list')}?{filter_params}")

  replace this with logic that:
  • parses filter_params into a dict using parse_qs
  • flattens the list values (since parse_qs returns lists)
  • builds a new query string via urlencode
  • appends that encoded query string to the fixed reverse('repos:mirror_list') path and passes it to redirect.

This preserves existing behavior (we still redirect back to mirror_list with whatever filters the user had applied) while mitigating the flagged issue and avoiding direct interpolation of raw user input into the URL.

Concretely, we will:

• Add: from urllib.parse import parse_qs, urlencode to the imports (note: _get_filtered_mirrors currently does a local import of parse_qs; we’ll keep that as-is, to avoid touching unrelated code).
• Replace each if filter_params: return redirect(f"{reverse('repos:mirror_list')}?{filter_params}") block with a small helper snippet that safely rebuilds the query string from filter_params using parse_qs + urlencode.

---

In general, the problem should be fixed by ensuring that any user-supplied value incorporated into a redirect URL is either (a) validated/whitelisted, or (b) safely sanitized so it cannot transform the redirect into an external or otherwise dangerous URL. For Django, protecting the path with reverse() is good, but we should also ensure that any appended query string is safe and only contains expected keys and values.

The best targeted fix here is to stop interpolating the raw filter_params string and instead rebuild a safe query string from parsed parameters. We already have a helper _get_filtered_mirrors(filter_params) that uses urllib.parse.parse_qs to parse filter_params into a dict and then only uses known keys: 'checksum', 'repo_id', and 'search'. We can use the same parsing approach to reconstruct a safe query dictionary limited to those known keys and then encode it with urllib.parse.urlencode. This removes any dangerous characters, strips unexpected parameters, and guarantees we only redirect with a controlled query string.

Concretely, inside mirror_bulk_action we should:

1. Import parse_qs and urlencode from urllib.parse at the top of the file (or reuse the existing local import approach).
2. Add a small helper function (within the shown snippet) _sanitize_filter_params(filter_params) that:
   • Parses the string with parse_qs.
   • Builds a dict containing only known keys (checksum, repo_id, search), taking the first value for each if present.
   • Returns urlencode(safe_params) if there are any, else ''.
3. Replace all three uses of f"{reverse('repos:mirror_list')}?{filter_params}" with code that calls this helper and only appends the ? if the sanitized query string is non-empty.

This keeps the existing behavior (preserving filters as query params) while ensuring that only an allowed subset of parameters is included and that they are properly URL-encoded, eliminating the untrusted direct concatenation.

General approach: Avoid building redirect URLs via string concatenation with untrusted text. Instead, either (a) validate/whitelist the untrusted portion, or (b) pass structured data (a dict of query params) to redirect/reverse, letting Django construct the URL safely.

Best fix here: filter_params is expected to be an encoded querystring that _get_filtered_mirrors already parses using parse_qs. We can reuse parse_qs in mirror_bulk_action wherever we currently concatenate ?{filter_params} and instead do:

• Parse filter_params into a dict.
• Flatten to a simple dict[str, str] (taking first value from each list).
• Use redirect('repos:mirror_list') when no filters, and redirect('repos:mirror_list') + "?" + urlencode(parsed) or better, redirect with a URL built from reverse and urlencode.

However, to stay within Django conventions and keep code simple and safe, we will:

1. Import parse_qs and urlencode at the top (module level) instead of re‑importing parse_qs inside _get_filtered_mirrors.
2. Add a small helper _build_mirror_list_redirect(filter_params: str) that:
   • If filter_params is falsy, returns redirect('repos:mirror_list').
   • Otherwise parses it with parse_qs, flattens, uses reverse('repos:mirror_list'), and appends a query string built with urlencode.
3. Replace all occurrences of redirect(f"{reverse('repos:mirror_list')}?{filter_params}") and similar patterns with return _build_mirror_list_redirect(filter_params).

This preserves existing functionality (user stays on the mirror list with the same filters) while ensuring the redirect URL is constructed from structured, parsed values rather than raw text from the request.

Concretely, edits in repos/views.py:

• Add imports at the top: from urllib.parse import parse_qs, urlencode.
• Remove the inner from urllib.parse import parse_qs in _get_filtered_mirrors and use the top‑level import.
• Define _build_mirror_list_redirect above mirror_bulk_action.
• In mirror_bulk_action, update all branches that currently do if filter_params: return redirect(f"{reverse('repos:mirror_list')}?{filter_params}") (lines 543–544, 553–554, 564–565, 586–588) to instead call the helper.

---

In general, to fix untrusted URL redirection in Django, you must ensure that user-controlled data does not determine the redirect destination without validation. When you want to preserve filter/query parameters, you should either: (a) validate or sanitize those parameters into a safe form and then reconstruct the URL with Django’s URL utilities, or (b) avoid interpolating raw user strings into the redirect target and instead re-derive the filter parameters on the server side.

For this specific case, the redirect destination should always be the internal mirror_list view; filter_params is only meant to preserve filtering state as a query string. We can safely achieve this by using Django’s QueryDict to parse and rebuild filter_params into a properly encoded, relative query string. QueryDict.urlencode() only produces key=value pairs joined by &, so it cannot produce an absolute URL or inject a scheme/host into the redirect target. The fix is therefore:

• Import QueryDict from django.http (extending the existing import).
• In mirror_bulk_action, instead of directly using f"{reverse('repos:mirror_list')}?{filter_params}", parse filter_params with QueryDict, re-encode it with .urlencode(), and append that to the reversed path only if the result is non-empty.
• Apply this pattern to each place in mirror_bulk_action where filter_params is used to construct the redirect.

Concretely:

• Modify the import at the top of repos/views.py from from django.http import HttpResponse to from django.http import HttpResponse, QueryDict.
• Inside mirror_bulk_action, right after obtaining filter_params, build a safe query string, e.g.:

raw_filter_params = request.POST.get('filter_params', '')
safe_query = QueryDict(raw_filter_params, mutable=False).urlencode()
if safe_query:
    mirror_list_with_filters = f"{reverse('repos:mirror_list')}?{safe_query}"
else:
    mirror_list_with_filters = reverse('repos:mirror_list')

• Replace all occurrences of f"{reverse('repos:mirror_list')}?{filter_params}" with mirror_list_with_filters.
• Keep the existing behavior (redirecting back to the same list view with filters preserved) while ensuring the redirect URL is always relative and normalized.