fix(oidc): reject backslash authority forms in relative redirects

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 - **Download filenames can no longer inject extra `Content-Disposition` parameters.** The `csv` and `download` components now build the `Content-Disposition` header with a properly quoted and escaped filename instead of plain string concatenation. Before this fix, a filename containing characters such as `;`, `"`, or `=` could add a second header parameter (for example a `filename*=...` value), and some browsers prefer that injected value over the intended one. You are affected if your app puts untrusted data (such as a user-provided name or a value from the database) into the `filename` of a `csv` or `download` component. No SQL change is required: the supplied filename now always appears as a single, safely quoted `filename` value.
 - **Security fix: reserved and private files could be served directly over HTTP after a trusted page loaded them.** Files that SQLPage normally refuses to serve to direct HTTP requests (anything under the reserved `sqlpage/` prefix such as migrations, configuration, and database connection details, as well as dotfiles, parent-directory traversal paths like `../secret.sql`, and absolute paths) could briefly become reachable. This happened only after a trusted page loaded that same file with `sqlpage.run_sql(...)`, which loads files with elevated privileges and caches the parsed result. While that cache entry was fresh, a direct request such as `GET /sqlpage/secret.sql` (or the extensionless alias `GET /sqlpage/secret`) returned `200 OK` and executed the private SQL instead of returning `403 Forbidden`. Worst case, an attacker who can reach your site could read or execute internal SQL that was meant to stay private, including migration and configuration logic. You are affected if your app calls `sqlpage.run_sql()` on files inside `sqlpage/` (or on dotfiles or paths outside the web root) and is reachable by untrusted users. The fix enforces the unprivileged path guard on every direct HTTP request regardless of the cache, so these paths now always return `403 Forbidden`. Upgrade to get the fix; no configuration change is required. Legitimate `sqlpage.run_sql()` includes of such files from your own pages keep working as before.
+- **Security (OIDC open redirect): logout and login redirect targets could point at an external site.** SQLPage accepted a relative redirect target if it started with `/` but not `//`. A value like `/\evil.test` passed that check, but the [WHATWG URL Standard](https://url.spec.whatwg.org/#url-parsing) treats `\` as `/` for http/https URLs, so SQLPage's own URL parser (the `url` crate) turns `/\evil.test` into `http://evil.test/` when it builds the absolute `post_logout_redirect_uri`. That makes it a server-side open redirect, independent of the client. The same WHATWG rule is implemented by current browsers (Chromium, Firefox, Safari), so a `Location: /\evil.test` is also followed to the external host. (Parsers that follow RFC 3986 instead, as many proxies do, leave the backslash alone, but SQLPage cannot rely on that.) You are affected only if you use OIDC and build a redirect target from user-controlled input, for example a `redirect_uri` passed to `sqlpage.oidc_logout_url`. SQLPage now rejects any redirect target containing a backslash, on top of the existing `//` check. Normal paths such as `/foo/bar?x=1` keep working; just upgrade.
 - **Security: OIDC logout links are now bound to the session that requested them.** Previously, a logout URL produced by `sqlpage.oidc_logout_url` only signed the redirect target and a timestamp, so any visitor who opened a still-valid logout link had their SQLPage auth cookies cleared. This allowed a forced logout via CSRF (a logout link made for one user could log out another, or be replayed). It did NOT allow account takeover or access to anyone's data. Logout links are now tied to the current `sqlpage_auth` cookie, so following one only logs out the browser it was generated for. You are affected only if you use built-in OIDC authentication together with `sqlpage.oidc_logout_url`. No action is needed: the normal "click your own logout link" flow keeps working, and old links simply stop being honored for other sessions.
 - **Security fix: OIDC protected paths could be bypassed with percent-encoded URLs.** When `oidc_protected_paths` used a non-default prefix such as `["/protected"]`, an unauthenticated visitor could percent-encode a byte of the prefix (for example requesting `/%70rotected/`, where `%70` is `p`) to make the OIDC middleware treat the page as public and serve it without logging in, even though SQLPage then decoded the URL and ran the protected SQL file. You are affected if you rely on `oidc_protected_paths` or `oidc_public_paths` with prefixes other than the default `/`. SQLPage now percent-decodes the request path and the configured prefixes before applying the rules, so encoded and plain URLs are classified identically (including when `site_prefix` contains characters such as a space). No configuration change is required; just upgrade.
 - **Security: production JSON, NDJSON, SSE, and CSV responses no longer leak SQL on errors.** When `environment` is set to `production`, the HTML renderer already hid database errors behind a generic message, but the JSON, NDJSON, Server-Sent Events, and CSV renderers still wrote the full error into the response body once streaming had started. That error included the path of the `.sql` file, the exact SQL statement SQLPage sent to the database, and the raw database error text. Because the response format is chosen from the request's `Accept` header, an unauthenticated visitor could request an ordinary HTML page with `Accept: application/json` and read back the SQL of pages that were never meant to expose it. You are affected if you run with `environment = production` and serve any page that can hit a database error (most apps can). There is nothing to change in your SQL: after upgrading, every output format returns the same generic "Please contact the administrator for more information. The error has been logged." message that HTML already used, the full error is still written to the server logs, and in development the detailed error is still shown.

src/webserver/database/sqlpage_functions/functions.rs

@@ -1055,9 +1055,9 @@ async fn oidc_logout_url<'a>(
 
     let redirect_uri = redirect_uri.as_deref().unwrap_or("/");
 
-    if !redirect_uri.starts_with('/') || redirect_uri.starts_with("//") {
+    if !crate::webserver::oidc::is_safe_relative_redirect(redirect_uri) {
         anyhow::bail!(
-            "oidc_logout_url: redirect_uri must be a relative path starting with '/'. Got: {redirect_uri}"
+            "oidc_logout_url: redirect_uri must be a relative path starting with a single '/'. Got: {redirect_uri}"
         );
     }
 

src/webserver/oidc.rs

@@ -731,7 +731,7 @@ fn verify_logout_params(
         anyhow::bail!("Logout token timestamp is in the future");
     }
 
-    if !params.redirect_uri.starts_with('/') || params.redirect_uri.starts_with("//") {
+    if !is_safe_relative_redirect(&params.redirect_uri) {
         anyhow::bail!("Invalid redirect URI");
     }
 
@@ -1252,9 +1252,39 @@ impl AudienceVerifier {
     }
 }
 
+/// Returns true if the given value is a safe relative redirect target.
+///
+/// Only paths starting with a single `/` (not `//`), with no backslash and no
+/// ASCII control characters, are accepted. The WHATWG URL Standard treats `\` as
+/// equivalent to `/` for special schemes (http/https) and strips tab/newline/CR
+/// before parsing, so `/\evil.test` and `/\t/evil.test` both parse to the
+/// authority `evil.test`, i.e. `http://evil.test/`. The `url` crate that builds
+/// the absolute `post_logout_redirect_uri` is itself a WHATWG parser, so without
+/// this check a value classified as "relative" becomes an external open-redirect
+/// target on the server side, independent of the client. Browsers implementing
+/// the same standard (Chromium, Firefox, Safari) resolve a `Location: /\evil.test`
+/// the same way.
+pub(crate) fn is_safe_relative_redirect(uri: &str) -> bool {
+    // Reject backslashes and ASCII control characters. The WHATWG URL parser
+    // used by SQLPage's `url` crate (and by browsers) treats `\` as `/`, and it
+    // removes ASCII tab/newline/CR from anywhere in the input before parsing.
+    // Either can smuggle in an authority component: `/\evil.test` and
+    // `/\t/evil.test` both resolve to `https://evil.test/`.
+    if uri.contains('\\') || uri.contains(|c: char| c.is_ascii_control()) {
+        return false;
+    }
+    let mut chars = uri.chars();
+    // Must start with `/`...
+    if chars.next() != Some('/') {
+        return false;
+    }
+    // ...but the second character must not be `/` (protocol-relative authority).
+    !matches!(chars.next(), Some('/'))
+}
+
 /// Validate that a redirect URL is safe to use (prevents open redirect attacks)
 fn validate_redirect_url(url: String, redirect_uri: &str) -> String {
-    if url.starts_with('/') && !url.starts_with("//") && !url.starts_with(redirect_uri) {
+    if is_safe_relative_redirect(&url) && !url.starts_with(redirect_uri) {
         return url;
     }
     log::warn!("Refusing to redirect to {url}");
@@ -1268,6 +1298,42 @@ mod tests {
     use actix_web::{cookie::Cookie, test::TestRequest};
     use openidconnect::url::Url;
 
+    #[test]
+    fn relative_redirect_rejects_authority_and_backslash_forms() {
+        // Legitimate relative paths must be accepted.
+        for ok in ["/", "/foo", "/foo/bar?x=1", "/a/b/c#frag"] {
+            assert!(
+                is_safe_relative_redirect(ok),
+                "expected {ok:?} to be accepted"
+            );
+        }
+        // Authority and backslash forms must all be rejected: SQLPage's `url`
+        // crate (a WHATWG URL parser) normalizes these into an external
+        // `http://evil.test/` target.
+        for bad in [
+            "//evil.test",
+            "/\\evil.test",
+            "/\\/evil.test",
+            "\\evil.test",
+            "\\\\evil.test",
+            "/foo\\bar",
+            // ASCII tab/newline/CR are stripped by the WHATWG URL parser, so
+            // these resolve to an authority (`//evil.test`) after stripping.
+            "/\t/evil.test",
+            "/\n/evil.test",
+            "/\r/evil.test",
+            "/\u{0000}/evil.test",
+            "https://evil.test",
+            "evil.test",
+            "",
+        ] {
+            assert!(
+                !is_safe_relative_redirect(bad),
+                "expected {bad:?} to be rejected"
+            );
+        }
+    }
+
     #[test]
     fn login_redirects_use_see_other() {
         let response = build_redirect_response("/foo".to_string());