# Patch Analysis: Horilla protected_media Authorization Bypass Variant

## Scope

- Repository: `https://github.com/horilla/horilla-hr.git`
- Submitted vulnerable target: tag `1.5.0`, commit `61bd5173220d19925ad8220db9152a75c881ea73`
- Fixed target tested: tag `1.6.0`, commit `b3bd29d15819cbece45c58e6268ddd0614e387d6`
- Source fix commit: `b6eaec1386d8b8741a42fe7c78f318f073375791`
- Runtime-confirmed bypass target: Horilla 1.6.0 through the real `/media/<path>` HTTP route
- Latest/default source inspected: `origin/master`, commit `11c4e3a2596c58f2381bda4c6bbc319a4430b097`

## Target Security / Threat Model Scope

Horilla's `SECURITY.md` instructs reporters to privately report security vulnerabilities and lists authentication/authorization and user-input validation as project security best practices. It does not exclude protected-media authorization bypasses from scope. The variant remains within the same attacker-facing trust boundary as the parent claim: an unauthenticated remote HTTP client controls the request path for the real `/media/<path>` endpoint and receives a server response containing or denying a private media file.

Relevant `SECURITY.md` guidance includes:

- "Authentication and Authorization: Implement strong authentication and authorization mechanisms to prevent unauthorized access to sensitive resources."
- "Data Validation: Validate and sanitize all user inputs..."

The variant is therefore not a local-file/self-attack issue and not normal documented behavior. It is an unauthenticated remote authorization bypass for a private in-root media file.

## What the Fix Changes

In Horilla 1.5.0, `base/views.py::protected_media()`:

1. builds `media_path = os.path.join(settings.MEDIA_ROOT, path)`;
2. parses `referer_path = urlparse(request.META.get("HTTP_REFERER", "")).path`;
3. skips authentication if the parsed `Referer` path is one of public pages such as `/login`; and
4. serves the target with `FileResponse(open(media_path, "rb"))`.

In Horilla 1.6.0 / source fix commit `b6eaec1386d8b8741a42fe7c78f318f073375791`, the patch:

1. removes the `public_pages` list and `Referer`-derived authorization decision;
2. introduces `public_media_prefixes`:
   - `base/icon/`
   - `base/company/icon/`
   - `recruitment/candidate/profile/`
3. calls `safe_join(settings.MEDIA_ROOT, path)` to prevent escaping `MEDIA_ROOT`;
4. rejects missing paths and non-files with `if not os.path.exists(media_path) or not os.path.isfile(media_path)`; and
5. only requires authenticated user/JWT access when `is_public_asset = any(path.startswith(prefix) for prefix in public_media_prefixes)` is false.

The fixed logic in tag `1.6.0` is anchored at `base/views.py` lines 7508-7540. The route remains unchanged at `base/urls.py` lines 1075-1076:

```python
urlpatterns.append(
    re_path(r"^media/(?P<path>.*)$", views.protected_media, name="protected_media"),
)
```

## Fix Assumptions

The fix relies on two separate assumptions:

1. `safe_join(settings.MEDIA_ROOT, path)` is sufficient to prevent filesystem traversal outside the media root.
2. If the raw attacker-controlled route parameter starts with a public prefix, the resolved file is also inside that public prefix and safe to serve unauthenticated.

Assumption 1 is valid for this variant: the demonstrated file remains inside `MEDIA_ROOT`. The variant does not escape to `/etc/passwd` or another path outside media.

Assumption 2 is false. A raw URL path can start with an allowlisted public prefix and then include dot-segments that normalize to a nonpublic sibling subtree under the same `MEDIA_ROOT`.

Example raw route parameter:

```text
base/icon/../../private-data/<run>/fixed-secret.txt
```

- Raw prefix check: starts with `base/icon/`, so `is_public_asset` is true.
- Normalized filesystem target after `safe_join`: `MEDIA_ROOT/private-data/<run>/fixed-secret.txt`, which is not under `MEDIA_ROOT/base/icon/` and is not public.
- Authentication branch: skipped.
- Sink reached: `FileResponse(open(media_path, "rb"))` returns the private canary.

## Code Paths / Inputs the Fix Covers

The fix successfully covers the parent `Referer` bypass:

- Direct private request without `Referer` on 1.6.0: denied (`302`) and no canary returned.
- Direct private request with `Referer: http://attacker.invalid/login` on 1.6.0: denied (`302`) and no canary returned.

The fix also covers traversal attempts that would leave `MEDIA_ROOT`, because `safe_join()` rejects those before the file open.

## Code Paths / Inputs the Fix Does Not Cover

The fix does not cover public-prefix confusion where the raw route parameter and normalized filesystem path disagree about whether the file is under a public subtree. The uncovered inputs are attacker-controlled `/media/<path>` values that:

1. begin with an allowlisted public prefix, and
2. contain dot-segments after that prefix, and
3. normalize to an existing nonpublic file that remains inside `MEDIA_ROOT`.

Confirmed example on the fixed release:

```text
/media/base/icon/../../private-data/20260722T061846Z-17685/fixed-secret.txt
```

This is a different trigger from the parent `Referer` bypass. It uses no `Referer` and no authentication headers; the only exploit primitive is control of the path captured by the endpoint.

The latest/default branch source inspected at `origin/master` commit `11c4e3a2596c58f2381bda4c6bbc319a4430b097` retained the same raw-prefix pattern in `protected_media()`, although this stage's runtime-confirmed bypass target is the fixed 1.6.0 release.

## Behavior Before and After the Fix

### Vulnerable 1.5.0

Runtime matrix behavior:

- Public liveness under `base/icon/`: `200`, exact public canary returned.
- Direct private path with no `Referer`: denied (`302`), no private canary.
- Direct private path with `Referer: http://attacker.invalid/login`: `200`, exact private canary returned (parent trigger).
- Dot-segment public-prefix variant with no `Referer`: `200`, exact private canary returned.

### Fixed 1.6.0

Runtime matrix behavior:

- Public liveness under `base/icon/`: `200`, exact public canary returned.
- Direct private path with no `Referer`: denied (`302`), no private canary.
- Direct private path with `Referer: http://attacker.invalid/login`: denied (`302`), no private canary. This confirms the original Referer fix works.
- Dot-segment public-prefix variant with no `Referer`: `200`, exact private canary returned. This confirms the bypass.

Latest successful proof summary: `bundle/logs/vuln_variant/latest_matrix_summary.json`.

## Completeness Assessment

The fix is incomplete. It removes the specific `Referer` trust flaw but leaves an authorization bypass in the same protected-media endpoint and sink because public-media classification is performed on an unnormalized attacker-controlled string.

The fix should be extended so that the public-prefix decision is made on a canonical normalized relative path, or preferably on normalized absolute containment under known public directories, after `safe_join()` has resolved the path. A request should be considered public only if the resolved absolute path is under one of the resolved public directory roots. Raw string prefix checks should not be used for authorization.

## Recommended Patch Direction

1. Resolve the requested path with `safe_join(settings.MEDIA_ROOT, path)`.
2. Compute a canonical normalized relative path from `settings.MEDIA_ROOT` to the resolved file using `os.path.realpath()` / `pathlib.Path.resolve()` and `os.path.relpath()` semantics appropriate for Django deployment.
3. For public assets, compare canonical absolute paths:
   - `resolved_file_path` must be under one of the canonical allowlisted public directory roots.
   - Do not rely on `raw_path.startswith(prefix)`.
4. Reject raw paths containing `..`, encoded dot-segments, backslashes, or other ambiguous separators when the request is unauthenticated.
5. Add regression tests at the real route level for dot-segment and encoded-dot-segment variants against every public prefix.
6. Keep the original `Referer` removal; do not reintroduce client-controlled headers as authorization signals.

## Evidence References

- Variant reproducer: `bundle/vuln_variant/reproduction_steps.sh`
- Latest runtime summary: `bundle/logs/vuln_variant/latest_matrix_summary.json`
- Latest runtime log: `bundle/logs/vuln_variant/reproduction_steps_20260722T061846Z-17685.log`
- Fixed-version identity: `bundle/logs/vuln_variant/fixed_version.txt`
- Latest/default source identity: `bundle/logs/vuln_variant/latest_version.txt`
- Source diff/security evidence: `bundle/logs/vuln_variant/patch_and_security_scope.log`
- Structured verdict: `bundle/vuln_variant/validation_verdict.json`
- Exact tested source identity: `bundle/vuln_variant/source_identity.json`