Skip to content

Commit efc2c96

Browse files
committed
fix(oidc): apply protected path rules to the decoded URL path
The OIDC middleware classified requests as public/protected using the raw request path, while the router percent-decodes the path before resolving the SQL file. An unauthenticated request to an encoded protected prefix (e.g. /%70rotected/ for a /protected rule) was treated as public and served, even though the protected SQL file was executed. Percent-decode the path in OidcConfig::is_public_path the same way routing does, so the protection decision matches the file that runs.
1 parent fe13e4a commit efc2c96

2 files changed

Lines changed: 59 additions & 0 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
- **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.
66
- **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.
7+
- **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 `/`. The path is now percent-decoded before the protected/public rules are applied, so encoded and plain URLs are classified identically. No configuration change is required; just upgrade.
78

89
## v0.44.0
910

src/webserver/oidc.rs

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,13 @@ impl TryFrom<&AppConfig> for OidcConfig {
140140
impl OidcConfig {
141141
#[must_use]
142142
pub fn is_public_path(&self, path: &str) -> bool {
143+
// Percent-decode the request path before matching, so that an
144+
// encoded protected prefix (e.g. "/%70rotected" == "/protected")
145+
// is classified the same way the router resolves it once decoded.
146+
// Otherwise an unauthenticated request could bypass the protected
147+
// path rules while still executing the protected SQL file.
148+
let decoded = percent_encoding::percent_decode_str(path).decode_utf8_lossy();
149+
let path = decoded.as_ref();
143150
!self.protected_paths.iter().any(|p| path.starts_with(p))
144151
|| self.public_paths.iter().any(|p| path.starts_with(p))
145152
}
@@ -1250,6 +1257,57 @@ mod tests {
12501257
verify_logout_params(&params, secret).expect("generated URL should validate");
12511258
}
12521259

1260+
fn test_oidc_config_with_paths(
1261+
protected_paths: Vec<String>,
1262+
public_paths: Vec<String>,
1263+
) -> OidcConfig {
1264+
OidcConfig {
1265+
issuer_url: IssuerUrl::new("https://example.com".to_string()).unwrap(),
1266+
client_id: "test_client".to_string(),
1267+
client_secret: "secret".to_string(),
1268+
protected_paths,
1269+
public_paths,
1270+
app_host: "example.com".to_string(),
1271+
scopes: vec![],
1272+
additional_audience_verifier: AudienceVerifier::new(None),
1273+
site_prefix: "/".to_string(),
1274+
redirect_uri: SQLPAGE_REDIRECT_URI.to_string(),
1275+
logout_uri: SQLPAGE_LOGOUT_URI.to_string(),
1276+
}
1277+
}
1278+
1279+
#[test]
1280+
fn percent_encoded_protected_path_is_not_public() {
1281+
let config = test_oidc_config_with_paths(vec!["/protected".to_string()], vec![]);
1282+
1283+
// Sanity check: the plain protected path is correctly classified as non-public.
1284+
assert!(
1285+
!config.is_public_path("/protected/"),
1286+
"/protected/ must be protected"
1287+
);
1288+
1289+
// The vulnerability: percent-encoding bytes of the protected prefix
1290+
// (%70 == 'p') must not bypass the protection, because routing later
1291+
// percent-decodes the path and serves the protected SQL file.
1292+
assert!(
1293+
!config.is_public_path("/%70rotected/"),
1294+
"/%70rotected/ decodes to /protected/ and must stay protected"
1295+
);
1296+
}
1297+
1298+
#[test]
1299+
fn percent_encoded_public_path_stays_public() {
1300+
let config = test_oidc_config_with_paths(
1301+
vec!["/protected".to_string()],
1302+
vec!["/protected/public".to_string()],
1303+
);
1304+
1305+
assert!(
1306+
config.is_public_path("/protected/%70ublic/page.sql"),
1307+
"/protected/%70ublic/... decodes to a public path and must stay public"
1308+
);
1309+
}
1310+
12531311
#[test]
12541312
fn evicts_excess_tmp_login_flow_state_cookies() {
12551313
let request = (0..MAX_OIDC_PARALLEL_LOGIN_FLOWS)

0 commit comments

Comments
 (0)