apache / apache/infrastructure-asfquart

Open Redirect via Backslash Normalization in OAuth Redirect URI Validation

Open
#90 0 comments 0 reactions 0 assignees View on GitHub
ASVS
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.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.