# Patch Analysis — CVE-2026-48611 (phpBB login-link auth bypass)

## Target threat model / security scope

`SECURITY.md` in the phpBB repo only describes a vulnerability-reporting
process (HackerOne / security tracker / `security@phpbb.com`); it does **not**
carve out any "not a vulnerability" exclusion for authentication-bypass
issues. There is no stated threat-model document that excludes auth-bypass
or unauthenticated-account-hijack from scope. The CVE itself scopes the issue
to **default `auth_method=db` installations**, which are vulnerable "out of the
box" because the login-link flow let an attacker *select* the bundled
password-less `apache` provider regardless of the board's configured
`auth_method`. A board that is **deliberately** configured with
`auth_method=apache` (i.e. the admin set up Apache/LDAP front-auth and intends
the `apache` provider to trust `PHP_AUTH_USER`) is the *intended* use case of
that provider and is out of scope of this CVE — the trust model there is
by-design, not a bypass.

## What the fix changes (files / functions / logic)

The fix (phpBB 3.3.17, `[ticket/17659]` series) moves the login-link handling
out of the legacy `ucp.php` / `includes/ucp/ucp_login_link.php` module into a
dedicated Symfony-style controller and, crucially, **removes the
attacker-controlled `auth_provider` argument from provider resolution** in that
flow.

### Before (3.3.16, vulnerable) — `includes/ucp/ucp_login_link.php::main()`
```php
$provider_collection = $phpbb_container->get('auth.provider_collection');
$auth_provider = $provider_collection->get_provider($request->variable('auth_provider', ''));
...
$login_result = $auth_provider->login($login_username, $login_password);
...
$user->session_create($login_result['user_row']['user_id'], false, false, true);
$this->perform_redirect();
```
The `auth_provider` value is taken verbatim from GET/POST, so an attacker can
force **any** registered provider (e.g. `apache`) to handle the login. The
password-less `apache::login()` returns `LOGIN_SUCCESS` for any existing
username carried in the HTTP Basic `Authorization` header
(`PHP_AUTH_USER === $username`, no password-hash comparison), after which
`session_create()` mints a session as that user (including founders/admins).

### After (3.3.17, fixed)
1. `includes/ucp/ucp_login_link.php` is **deleted**.
2. `ucp.php` `case 'login_link'` now redirects to the new controller:
   ```php
   // ucp.php (3.3.17)
   case 'login_link':
       $get_params_array = $request->get_super_global(request_interface::GET);
       phpbb_redirect_to_controller('phpbb_ucp_oauth_link_account_controller', $get_params_array);
       break;
   ```
   (It forwards *all* GET params, including any `auth_provider`, to the
   controller — but the controller ignores `auth_provider`, see below.)
3. New controller `phpbb/ucp/controller/oauth.php` with three actions:
   - `link_account()` (route `/oauth/link_account`, the replacement for
     `ucp_login_link::main()`):
     ```php
     $auth_provider = $this->auth_collection->get_provider();   // NO argument
     ...
     $login_result = $auth_provider->login($login_username, $login_password);
     ...
     $this->user->session_create($login_result['user_row']['user_id']);
     ```
   - `authenticate(string $oauth_service)` (route `/oauth/authenticate/{svc}`):
     `$this->auth_collection->get_provider();` — no argument.
   - `login(string $oauth_service)` (route `/oauth/login/{svc}`):
     `$this->auth_collection->get_provider();` — no argument, **plus** an
     `instanceof \phpbb\auth\provider\oauth\oauth` guard that throws
     `HTTP 401 NOT_AUTHORISED` if the configured provider is not the OAuth
     provider.
4. Routing wired in `config/default/routing/ucp.yml`
   (`phpbb_ucp_oauth_link_account_controller`, `..._authenticate_controller`,
   `..._login_controller`) and the service registered in
   `config/default/container/services_ucp.yml` (`phpbb.ucp.controller.oauth`).

`provider_collection::get_provider()` with **no argument** resolves the
board-configured `auth_method` (`$this->config['auth_method']`, default `db`).
With `db`, `login()` actually verifies the password hash, so a wrong password
(`x`) is rejected and no session is created.

Key commits in `release-3.3.16..release-3.3.17`: `12c4cf6f78`,
`c05603cc3a`, `4a2962bfc8` (`[ticket/17659]` — "Add new oauth link controller"
/ "Make account linking work via controller" / "Move login linking for oauth
to controller").

## What invariant / assumptions the fix relies on

1. **Provider resolution in the login-link flow always uses the
   board-configured `auth_method`.** The fix's central assumption, stated in
   the RCA, is: *"authentication providers must never be selectable from
   untrusted request input; provider resolution should always use the
   board-configured `auth_method`."* The controller enforces this by calling
   `get_provider()` with no argument.
2. **The only dangerous sink is `apache::login()` (password-less) reached via
   an attacker-selected provider followed by `session_create()`.** The fix
   assumes that if `get_provider()` cannot be steered by request input, the
   `apache` provider is unreachable from this flow on a default `db` board.
3. **The new oauth `login()` controller is additionally guarded** by an
   `instanceof oauth\oauth` check so that even a misconfigured board cannot
   drive an arbitrary provider's `login()` through that endpoint.

## What code paths / inputs the fix does NOT cover

A residual instance of the *anti-pattern* the fix was meant to eliminate
remains:

### `includes/ucp/ucp_register.php` (registration flow) — NOT touched by the fix
```php
// ucp_register.php (3.3.17, line ~120) — unchanged from 3.3.16
$login_link_data = $this->get_login_link_data_array();      // reads login_link_* from POST
if (!empty($login_link_data))
{
    $provider_collection = $phpbb_container->get('auth.provider_collection');
    $auth_provider = $provider_collection->get_provider($request->variable('auth_provider', ''));  // <-- attacker-controlled
    $result = $auth_provider->login_link_has_necessary_data($login_link_data);
    ...
}
...
// later, after a NEW user is registered:
$user->session_create($user_id, 0, false, 1);              // session for the NEW user_id
...
$result = $auth_provider->link_account($login_link_data);   // on the attacker-selected provider
```
This is the **exact same `get_provider($request->variable('auth_provider',''))`
pattern** that was the root cause of CVE-2026-48611, and the fix did **not**
modify it. An unauthenticated attacker can still steer provider selection here
by sending `auth_provider=apache` (GET) together with `login_link_*` POST data
to `ucp.php?mode=register`.

**However — and this is the decisive point — this residual instance does NOT
reach the dangerous sink.** In the register flow the attacker-selected
`$auth_provider` is used **only** for:
- `login_link_has_necessary_data($data)` — for the `apache` provider this
  inherits `base::login_link_has_necessary_data()`, which unconditionally
  returns the error string `'LOGIN_LINK_MISSING_DATA'` (the `apache` provider
  does not override it), so `$error[]` is populated and registration is
  blocked; and
- `link_account($data)` — for `apache` this inherits `base::link_account()`,
  a no-op (`return;`).

The register flow **never calls `->login()` on the attacker-selected
provider**. The only `session_create()` in this flow is for the *newly
registered* user's `user_id` (a fresh, low-privilege account), never for an
arbitrary existing account, and never as a result of the `apache` provider's
password-less `LOGIN_SUCCESS`. So the residual instance violates the fix's
*stated principle* (provider resolution should never be attacker-steerable)
but does **not** reproduce the CVE's impact (unauthenticated hijack of an
arbitrary existing account incl. admins).

This was confirmed at runtime: the register flow with
`auth_provider=apache` + `login_link_*` produced **no** admin session
(`*_u=1`, anonymous) on **both** the vulnerable 3.3.16 and the fixed 3.3.17
builds (see V1 in the variant matrix). I.e. the register flow was never a
variant of the auth-bypass, on either version.

## Other entry points checked (all use the configured provider — safe)

- `phpbb/auth/auth.php::login()` (the normal `login_box` / `ucp.php?mode=login`
  / ACP `adm/index.php` path): `$provider_collection->get_provider();` — no
  argument → configured `db`. Attacker cannot steer the provider here.
- `phpbb/session.php` (`autologin` / `validate_session` / `logout`):
  `get_provider();` — no argument.
- `includes/functions.php` `phpbb_login_box()` / login-data display:
  `get_provider();` — no argument.
- `includes/ucp/ucp_auth_link.php`: `get_provider();` — no argument.
- New `oauth.php` controller `authenticate()` / `login()` / `link_account()`:
  all `get_provider();` no argument; `login()` additionally guarded by
  `instanceof oauth\oauth`.

A whole-repo search for `get_provider($request->variable('auth_provider'` in
3.3.17 returns **exactly one** hit: `ucp_register.php:120` (the residual
instance above). Every other `get_provider(` call site passes no argument.

## Behavior comparison before vs after the fix

| Request (default `auth_method=db` board) | 3.3.16 (vuln) | 3.3.17 (fixed) |
|---|---|---|
| `POST ucp.php?mode=login_link&auth_provider=apache` + `Authorization: Basic admin:x` | `302`, `Set-Cookie *_u=2` (admin), redirect to index → admin session | `301` redirect to `/app.php/user/oauth/link_account`, then controller uses `db` provider → wrong password rejected, `*_u=1` (anonymous) |
| `POST /app.php/user/oauth/link_account?login_link_*=1` + `Authorization: Basic admin:x` (n/a on 3.3.16) | n/a | `200` login-link form with `class="error"` block, `*_u=1` |
| `POST /app.php/user/oauth/link_account?auth_provider=apache&login_link_*=1` + Basic header (controller ignores `auth_provider`) | n/a | `200` form, `class="error"`, `*_u=1` — `auth_provider` ignored |
| `GET /app.php/user/oauth/login/apache` + Basic header (oauth `login()` controller) | n/a | `401 Unauthorized` — `instanceof oauth\oauth` guard rejects `db`-configured board |
| `POST ucp.php?mode=register&auth_provider=apache` + `login_link_*` (register flow) | `*_u=1`, no admin session (register never calls `->login()` on the selected provider) | `*_u=1`, no admin session (same) |

## Is the fix complete?

**Yes — with respect to the disclosed impact (unauthenticated hijack of an
arbitrary existing account, incl. admins, on a default `db` board).** The fix
removes the only request-steerable `get_provider()` that was followed by a
`->login()` + `session_create()` chain capable of minting a session for an
arbitrary existing user. The runtime variant matrix (V1–V4) shows the fixed
3.3.17 blocks every materially distinct entry/data path that could reach the
`apache` password-less sink, while the 3.3.16 control still hijacks admin
(`_u=2`, ACP link present).

**Gap (defense-in-depth, not a bypass):** `ucp_register.php` still resolves
the provider from the attacker-controlled `auth_provider` request parameter.
This violates the fix's own stated invariant ("provider resolution should
always use the board-configured `auth_method`") and is the kind of latent
anti-pattern that a future code change could accidentally turn into a real
bypass (e.g. if a later refactor added a `->login()` call on
`$auth_provider` in the register flow). It should be closed for consistency
even though it currently has no security impact.