Voidauth upstream issue #390

Closed
opened 2026-04-26 00:24:38 -05:00 by pblohrndz · 2 comments
pblohrndz commented 2026-04-26 00:24:38 -05:00 (Migrated from github.com)
<html><head></head>

proxyAuthPath argument shift causes 500 on session expiry; unsafe new URL() in redirectUriAllowed validator amplifies the failure

Version: 1.12.0 Git revision: 2099ecb70f86e82f2507444055c40238a9b745b0 Image: voidauth/voidauth:latest (digest sha256:e8c8cf1d…) Deployment: voidauth + caddy forward-auth, postgres backend, multiple proxy_auth rows with non-null maxSessionLength

Summary

A real (not scanner) authenticated user hitting any forward-auth-protected service after their session age exceeds the row's maxSessionLength gets "Internal Server Error" instead of being re-prompted to log in. Two compounding bugs in the bundled dist/index.mjs:

  • Bug A: the session-expiry redirect calls proxyAuthPath with positional arguments shifted by one, producing a malformed redirect_uri
  • Bug B: the redirectUriAllowed override for proxyauth_internal_client uses an unsafe new URL() on the inner proxyauth_url query parameter, which throws on the malformed input from Bug A

Either fix alone resolves the user-visible 500. I think both warrant fixing.

Bug A — argument shift in proxyAuthPath call

The function signature (dist/index.mjs, function definition near byte 13261896 in this build):

function proxyAuthPath(baseUrl, redirectUrl, prompt) {
  const encodedRedirectUrl = encodeURIComponent(redirectUrl);
  const redirectParam = `redirect_uri=${encodeURIComponent(
    baseUrl + `/api/proxyauth_cb?proxyauth_url=${encodedRedirectUrl}`
  )}`;
  let queryParams = `client_id=proxyauth_internal_client&response_type=none&scope=openid&${redirectParam}`;
  queryParams += prompt ? `&prompt=${prompt}` : "";
  return `/oidc/auth?${queryParams}`;
}

Three positional parameters: baseUrl, redirectUrl, prompt.

The buggy call site is in proxyAuth()'s session-expiry branch (call site near byte 13263067):

if (req.user) {
  user = req.user;
  amr = req.user.amr;
  const session = await getSession(req, res);
  if (match?.maxSessionLength && session?.past(match.maxSessionLength * 60)) {
    res.redirect(redirCode, `${config_default.APP_URL}${proxyAuthPath(url2.href, "login")}`);
    return;
  }
}

The call passes only 2 positional arguments where the function expects 3:

Slot Function expects Call passes Should pass
1st baseUrl url2.href (the protected service URL) config_default.APP_URL
2nd redirectUrl "login" (string literal) url2.href
3rd prompt (missing) "login"

The intent appears to be prompt=login for forced re-authentication. The arguments got shifted by one — "login" ends up encoded as the proxyauth_url query parameter, and the protected service URL ends up as the OIDC base URL.

Concrete output

With url2.href = "http://protected.example.com/":

Plug the buggy values into proxyAuthPath:

baseUrl + "/api/proxyauth_cb?proxyauth_url=" + encodeURIComponent("login")
= "http://protected.example.com/" + "/api/proxyauth_cb?proxyauth_url=" + "login"
= "http://protected.example.com//api/proxyauth_cb?proxyauth_url=login"

This becomes the redirect_uri value sent to /oidc/auth. Two structural problems:

  • Double slash after host (the baseUrl already ends in /, and the function unconditionally adds another)
  • proxyauth_url=login — a bare token, not a URL

Suggested fix

Pass the correct arguments:

res.redirect(redirCode, `${config_default.APP_URL}${proxyAuthPath(config_default.APP_URL, url2.href, "login")}`);

Or, more idiomatically, since the prefix ${config_default.APP_URL} is now redundant with the first proxyAuthPath arg, the whole construction could be simplified.

Other call sites worth auditing

proxyAuthPath is called from 6 sites in this build (function defined at byte 13261896; call sites at bytes 13263067, 13263545, 13264196, 13264651, 13264962, 13265287). Only the 13263067 site was audited end-to-end against a live reproduction. The other 5 may or may not have similar issues. Worth reviewing all 6 against the signature.

Bug B — unsafe new URL() in redirectUriAllowed validator

In the redirectUriAllowed override (single occurrence at byte 4251559, in the proxyauth_internal_client branch):

provider.Client.prototype.redirectUriAllowed = function newRedirectUriAllowed(redirectUri2) {
  if (oidc.params.client_id === "auth_internal_client") {
    const redirectURL = URL.parse(redirectUri2);
    return !!redirectURL && sessionDomainReaches(redirectURL.hostname);
  }
  if (oidc.params.client_id === "proxyauth_internal_client") {
    const redirectURL = URL.parse(redirectUri2);                                  // safe (returns null)
    const proxyAuthURLParam = redirectURL?.searchParams.get("proxyauth_url");
    const proxyAuthURL = proxyAuthURLParam ? new URL(proxyAuthURLParam) : null;   // throws on malformed input
    return !!redirectURL
        && sessionDomainReaches(redirectURL.hostname)
        && redirectURL.pathname.endsWith("/api/proxyauth_cb")
        && !!proxyAuthURL
        && sessionDomainReaches(proxyAuthURL.hostname);
  }
  // ...
};

The outer URI uses safe URL.parse() (returns null on failure). The inner proxyauth_url parameter uses throwing new URL(). When proxyauth_url contains anything that fails URL parsing — e.g., a bare token like "login" per Bug A — new URL() throws, the throw propagates into authorizationErrorHandler, and the user gets 500.

Stack trace (from production logs)

TypeError: Invalid URL
  at new URL (node:internal/url:819:25)
  at Client.newRedirectUriAllowed (file:///app/dist/index.mjs:473:28794)
  at authorizationErrorHandler (file:///app/dist/index.mjs:405:6526)
  at async ensureSessionSave (file:///app/dist/index.mjs:405:123414)

Suggested fix

Use URL.parse() for consistency with the outer URI handling:

const proxyAuthURL = proxyAuthURLParam ? URL.parse(proxyAuthURLParam) : null;

Then the truthiness check !!proxyAuthURL already handles the parse-failure case correctly.

Reproduction

  1. Configure a proxy_auth row with non-null maxSessionLength (e.g., 10 minutes)
  2. Authenticate to the protected subdomain
  3. Wait until session age exceeds maxSessionLength
  4. Issue any request to the protected subdomain

Expected: redirected to /oidc/auth for re-authentication, then back to the service.

Actual: 500 "Internal Server Error" at /oidc/auth. Logs show the TypeError above.

Workaround for affected operators

Setting maxSessionLength = NULL on all proxy_auth rows short-circuits the buggy code path in Bug A:

if (match?.maxSessionLength && session?.past(match.maxSessionLength * 60)) {
  // never enters when maxSessionLength is null
}

Bug B remains latent but is no longer reachable via Bug A. Per-domain forced re-authentication is disabled; sessions expire only at the global TTL. Reversible via the admin UI.

Severity assessment (operator view)

The condition is silently pervasive: any deployment with non-null maxSessionLength on proxy_auth rows hits this on every session expiry, across every protected subdomain. The error logs don't include request context, which makes diagnosis from logs alone non-trivial — the bug surfaces as a TypeError with no indication of which request triggered it.

<html><head></head><body><h1><code>proxyAuthPath</code> argument shift causes 500 on session expiry; unsafe <code>new URL()</code> in <code>redirectUriAllowed</code> validator amplifies the failure</h1> <p><strong>Version:</strong> <code>1.12.0</code> <strong>Git revision:</strong> <code>2099ecb70f86e82f2507444055c40238a9b745b0</code> <strong>Image:</strong> <code>voidauth/voidauth:latest</code> (digest <code>sha256:e8c8cf1d…</code>) <strong>Deployment:</strong> voidauth + caddy forward-auth, postgres backend, multiple <code>proxy_auth</code> rows with non-null <code>maxSessionLength</code></p> <h2>Summary</h2> <p>A real (not scanner) authenticated user hitting any forward-auth-protected service after their session age exceeds the row's <code>maxSessionLength</code> gets "Internal Server Error" instead of being re-prompted to log in. Two compounding bugs in the bundled <code>dist/index.mjs</code>:</p> <ul> <li><strong>Bug A:</strong> the session-expiry redirect calls <code>proxyAuthPath</code> with positional arguments shifted by one, producing a malformed <code>redirect_uri</code></li> <li><strong>Bug B:</strong> the <code>redirectUriAllowed</code> override for <code>proxyauth_internal_client</code> uses an unsafe <code>new URL()</code> on the inner <code>proxyauth_url</code> query parameter, which throws on the malformed input from Bug A</li> </ul> <p>Either fix alone resolves the user-visible 500. I think both warrant fixing.</p> <h2>Bug A — argument shift in <code>proxyAuthPath</code> call</h2> <p>The function signature (<code>dist/index.mjs</code>, function definition near byte 13261896 in this build):</p> <pre><code class="language-javascript">function proxyAuthPath(baseUrl, redirectUrl, prompt) { const encodedRedirectUrl = encodeURIComponent(redirectUrl); const redirectParam = `redirect_uri=${encodeURIComponent( baseUrl + `/api/proxyauth_cb?proxyauth_url=${encodedRedirectUrl}` )}`; let queryParams = `client_id=proxyauth_internal_client&amp;response_type=none&amp;scope=openid&amp;${redirectParam}`; queryParams += prompt ? `&amp;prompt=${prompt}` : ""; return `/oidc/auth?${queryParams}`; } </code></pre> <p>Three positional parameters: <code>baseUrl</code>, <code>redirectUrl</code>, <code>prompt</code>.</p> <p>The buggy call site is in <code>proxyAuth()</code>'s session-expiry branch (call site near byte 13263067):</p> <pre><code class="language-javascript">if (req.user) { user = req.user; amr = req.user.amr; const session = await getSession(req, res); if (match?.maxSessionLength &amp;&amp; session?.past(match.maxSessionLength * 60)) { res.redirect(redirCode, `${config_default.APP_URL}${proxyAuthPath(url2.href, "login")}`); return; } } </code></pre> <p>The call passes only 2 positional arguments where the function expects 3:</p> Slot | Function expects | Call passes | Should pass -- | -- | -- | -- 1st | baseUrl | url2.href (the protected service URL) | config_default.APP_URL 2nd | redirectUrl | "login" (string literal) | url2.href 3rd | prompt | (missing) | "login" <p>The intent appears to be <code>prompt=login</code> for forced re-authentication. The arguments got shifted by one — <code>"login"</code> ends up encoded as the <code>proxyauth_url</code> query parameter, and the protected service URL ends up as the OIDC base URL.</p> <h3>Concrete output</h3> <p>With <code>url2.href = "http://protected.example.com/"</code>:</p> <p>Plug the buggy values into <code>proxyAuthPath</code>:</p> <pre><code>baseUrl + "/api/proxyauth_cb?proxyauth_url=" + encodeURIComponent("login") = "http://protected.example.com/" + "/api/proxyauth_cb?proxyauth_url=" + "login" = "http://protected.example.com//api/proxyauth_cb?proxyauth_url=login" </code></pre> <p>This becomes the <code>redirect_uri</code> value sent to <code>/oidc/auth</code>. Two structural problems:</p> <ul> <li>Double slash after host (the <code>baseUrl</code> already ends in <code>/</code>, and the function unconditionally adds another)</li> <li><code>proxyauth_url=login</code> — a bare token, not a URL</li> </ul> <h3>Suggested fix</h3> <p>Pass the correct arguments:</p> <pre><code class="language-javascript">res.redirect(redirCode, `${config_default.APP_URL}${proxyAuthPath(config_default.APP_URL, url2.href, "login")}`); </code></pre> <p>Or, more idiomatically, since the prefix <code>${config_default.APP_URL}</code> is now redundant with the first <code>proxyAuthPath</code> arg, the whole construction could be simplified.</p> <h3>Other call sites worth auditing</h3> <p><code>proxyAuthPath</code> is called from 6 sites in this build (function defined at byte 13261896; call sites at bytes 13263067, 13263545, 13264196, 13264651, 13264962, 13265287). Only the 13263067 site was audited end-to-end against a live reproduction. The other 5 may or may not have similar issues. Worth reviewing all 6 against the signature.</p> <h2>Bug B — unsafe <code>new URL()</code> in <code>redirectUriAllowed</code> validator</h2> <p>In the <code>redirectUriAllowed</code> override (single occurrence at byte 4251559, in the <code>proxyauth_internal_client</code> branch):</p> <pre><code class="language-javascript">provider.Client.prototype.redirectUriAllowed = function newRedirectUriAllowed(redirectUri2) { if (oidc.params.client_id === "auth_internal_client") { const redirectURL = URL.parse(redirectUri2); return !!redirectURL &amp;&amp; sessionDomainReaches(redirectURL.hostname); } if (oidc.params.client_id === "proxyauth_internal_client") { const redirectURL = URL.parse(redirectUri2); // safe (returns null) const proxyAuthURLParam = redirectURL?.searchParams.get("proxyauth_url"); const proxyAuthURL = proxyAuthURLParam ? new URL(proxyAuthURLParam) : null; // throws on malformed input return !!redirectURL &amp;&amp; sessionDomainReaches(redirectURL.hostname) &amp;&amp; redirectURL.pathname.endsWith("/api/proxyauth_cb") &amp;&amp; !!proxyAuthURL &amp;&amp; sessionDomainReaches(proxyAuthURL.hostname); } // ... }; </code></pre> <p>The outer URI uses safe <code>URL.parse()</code> (returns <code>null</code> on failure). The inner <code>proxyauth_url</code> parameter uses throwing <code>new URL()</code>. When <code>proxyauth_url</code> contains anything that fails URL parsing — e.g., a bare token like <code>"login"</code> per Bug A — <code>new URL()</code> throws, the throw propagates into <code>authorizationErrorHandler</code>, and the user gets 500.</p> <h3>Stack trace (from production logs)</h3> <pre><code>TypeError: Invalid URL at new URL (node:internal/url:819:25) at Client.newRedirectUriAllowed (file:///app/dist/index.mjs:473:28794) at authorizationErrorHandler (file:///app/dist/index.mjs:405:6526) at async ensureSessionSave (file:///app/dist/index.mjs:405:123414) </code></pre> <h3>Suggested fix</h3> <p>Use <code>URL.parse()</code> for consistency with the outer URI handling:</p> <pre><code class="language-javascript">const proxyAuthURL = proxyAuthURLParam ? URL.parse(proxyAuthURLParam) : null; </code></pre> <p>Then the truthiness check <code>!!proxyAuthURL</code> already handles the parse-failure case correctly.</p> <h2>Reproduction</h2> <ol> <li>Configure a <code>proxy_auth</code> row with non-null <code>maxSessionLength</code> (e.g., <code>10</code> minutes)</li> <li>Authenticate to the protected subdomain</li> <li>Wait until session age exceeds <code>maxSessionLength</code></li> <li>Issue any request to the protected subdomain</li> </ol> <p>Expected: redirected to <code>/oidc/auth</code> for re-authentication, then back to the service.</p> <p>Actual: 500 "Internal Server Error" at <code>/oidc/auth</code>. Logs show the TypeError above.</p> <h2>Workaround for affected operators</h2> <p>Setting <code>maxSessionLength = NULL</code> on all <code>proxy_auth</code> rows short-circuits the buggy code path in Bug A:</p> <pre><code class="language-javascript">if (match?.maxSessionLength &amp;&amp; session?.past(match.maxSessionLength * 60)) { // never enters when maxSessionLength is null } </code></pre> <p>Bug B remains latent but is no longer reachable via Bug A. Per-domain forced re-authentication is disabled; sessions expire only at the global TTL. Reversible via the admin UI.</p> <h2>Severity assessment (operator view)</h2> <p>The condition is silently pervasive: any deployment with non-null <code>maxSessionLength</code> on <code>proxy_auth</code> rows hits this on every session expiry, across every protected subdomain. The error logs don't include request context, which makes diagnosis from logs alone non-trivial — the bug surfaces as a <code>TypeError</code> with no indication of which request triggered it.</p>
notquitenothing commented 2026-04-26 07:04:34 -05:00 (Migrated from github.com)

I have replicated the issue locally, and will release a patch shortly

I have replicated the issue locally, and will release a patch shortly
notquitenothing commented 2026-04-26 09:34:34 -05:00 (Migrated from github.com)

Should be fixed now in v1.12.1

Should be fixed now in [v1.12.1](https://github.com/voidauth/voidauth/releases/tag/v1.12.1)
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
adam/gate#390
No description provided.