apache / apache/infrastructure-asfquart
Open Redirect via Backslash Normalization in OAuth Redirect URI Validation
- Dominant language
- Python
- Stars
- 7
- Forks
- 12
- PR merge metrics
- No merged PRs in 30d
Description
## Issue: FINDING-114 - Open Redirect via Backslash Normalization in OAuth Redirect URI Validation
**Labels:** bug, security, priority:medium, asvs-level:L2
**ASVS Level(s):** [L2-only]
**Description:**
### Summary
The OAuth redirect URI validation in the login and logout flows rejects URIs that don't start with '/' or that start with '//', but does not account for the WHATWG URL Standard backslash normalization. Per the spec (§4.4 'relative slash' state), when a URL parser encounters '\' after an initial '/' in a special URL scheme (http/https), it treats '\' identically to '/'. Thus '\' acts as a path separator, and '/\evil.com' is parsed as '//evil.com', a protocol-relative URL pointing to evil.com. The validation passes '/\evil.com' because it starts with '/' and doesn't start with '//', but browsers normalize this to '//evil.com', causing an open redirect.
### Details
Affected locations:
- `src/asfquart/generics.py` lines 48-53: Login redirect validation
- `src/asfquart/generics.py` lines 69-75: Login redirect usage
- `src/asfquart/generics.py` lines 113-119: Logout redirect validation
- `src/asfquart/generics.py` lines 164-172: Logout redirect usage
- `tests/generics.py` lines 53-58: Tests don't cover backslash case
Impact includes post-authentication phishing where users complete OAuth login and are then redirected to an attacker's site mimicking the application.
### Recommended Remediation
Add backslash normalization check to redirect URI validation. Create a helper function `_is_safe_redirect`:
```python
def _is_safe_redirect(uri: str) -> bool:
"""Validate redirect URI is safe (relative, same-origin only)."""
if not uri.startswith("/"):
return False
if uri.startswith("//"):
return False
# Block backslash and URL-encoded backslash
if "\\" in uri or "%5c" in uri.lower() or "%5C" in uri:
return False
# Verify no netloc present after parsing
parsed = urllib.parse.urlsplit(uri)
if parsed.netloc:
return False
return True
```
Add test cases in `tests/generics.py` to verify rejection of '/%5Cevil.com' and '/\evil.com' for both login and logout flows.
### Acceptance Criteria
- [ ] Backslash-based open redirects are blocked
- [ ] URL-encoded backslashes are blocked
- [ ] Test cases verify backslash rejection
- [ ] Both login and logout flows are protected
- [ ] Unit test verifying the fix
### References
- Source reports: L2:3.5.4.md
- Related findings: None
- ASVS sections: 3.5.4
### Priority
Medium
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.