Skip to content

Commit 466e0a5

Browse files
Pijukatelclaude
andauthored
fix: Cookie domain matching (#488)
Replace substring match by matching according to [RFC 6265, §5.1.3](https://www.rfc-editor.org/rfc/rfc6265#section-5.1.3). Add test. # Issues: Closes: #473 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 311c6a8 commit 466e0a5

2 files changed

Lines changed: 104 additions & 1 deletion

File tree

impit-python/src/cookies.rs

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ impl CookieStore for PythonCookieJar {
123123
.and_then(|attr| attr.extract::<bool>())
124124
.unwrap_or_default();
125125

126-
if !domain.is_empty() && !url.host_str().unwrap_or_default().contains(&domain) {
126+
if !domain_matches(url.host_str().unwrap_or_default(), &domain) {
127127
return None;
128128
}
129129
if !url.path().starts_with(&path) {
@@ -175,6 +175,39 @@ impl CookieStore for PythonCookieJar {
175175
}
176176
}
177177

178+
/// Checks whether a request `host` may receive a cookie scoped to `cookie_domain`,
179+
/// following the domain matching rules of
180+
/// [RFC 6265, §5.1.3](https://www.rfc-editor.org/rfc/rfc6265#section-5.1.3).
181+
///
182+
/// Leading dot on the cookie domain (e.g. `.example.com`) is ignored, as
183+
/// permitted by [RFC 6265, §4.1.2.3](https://www.rfc-editor.org/rfc/rfc6265#section-4.1.2.3).
184+
///
185+
/// An empty cookie domain imposes no host restriction and therefore matches any host.
186+
fn domain_matches(host: &str, cookie_domain: &str) -> bool {
187+
let cookie_domain = cookie_domain.strip_prefix('.').unwrap_or(cookie_domain);
188+
189+
if cookie_domain.is_empty() {
190+
return true;
191+
}
192+
193+
// Host names are case-insensitive; normalise both sides instead of trusting the
194+
// caller to pass a lowercased host.
195+
let host = host.to_ascii_lowercase();
196+
let cookie_domain = cookie_domain.to_ascii_lowercase();
197+
198+
// RFC 6265 §5.1.3: an IP-address host only matches an identical cookie domain.
199+
if host.parse::<std::net::IpAddr>().is_ok() {
200+
return host == cookie_domain;
201+
}
202+
203+
// Exact match, or subdomain match where the cookie domain is a suffix of the host
204+
// on a `.` label boundary (e.g. host `www.example.com`, cookie domain `example.com`).
205+
host == cookie_domain
206+
|| host
207+
.strip_suffix(cookie_domain.as_str())
208+
.is_some_and(|prefix| prefix.ends_with('.'))
209+
}
210+
178211
impl PythonCookieJar {
179212
pub fn new(py: Python<'_>, cookie_jar: Py<PyAny>) -> Self {
180213
let httpmodule = PyModule::import(py, "http.cookiejar").unwrap();

impit-python/test/no_client_test.py

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
import socket
33
import threading
44
import time
5+
import urllib.parse
56
from http.cookiejar import CookieJar
67

78
import pytest
@@ -30,6 +31,31 @@ def thread_server(port_holder: list[int]) -> None:
3031
server.close()
3132

3233

34+
def cookie_echo_server(port_holder: list[int], captured: dict[str, str]) -> None:
35+
"""Serve a single HTTP/1.1 request on 127.0.0.1 and record its `Cookie` header."""
36+
server = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
37+
server.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
38+
server.bind(('127.0.0.1', 0))
39+
port_holder[0] = server.getsockname()[1]
40+
server.listen(1)
41+
42+
conn, _ = server.accept()
43+
request = b''
44+
while b'\r\n\r\n' not in request:
45+
chunk = conn.recv(4096)
46+
if not chunk:
47+
break
48+
request += chunk
49+
50+
for line in request.decode('utf-8', errors='replace').split('\r\n'):
51+
if line.lower().startswith('cookie:'):
52+
captured['cookie'] = line.split(':', 1)[1].strip()
53+
54+
conn.send(b'HTTP/1.1 200 OK\r\nContent-Length: 2\r\n\r\nok')
55+
conn.close()
56+
server.close()
57+
58+
3359
class TestBasicRequests:
3460
@pytest.mark.parametrize(
3561
('protocol'),
@@ -174,6 +200,50 @@ def test_cookies_param_works(self) -> None:
174200
assert cookies.get('preset-cookie') == '123'
175201
assert cookies.get('set-by-server') == '321'
176202

203+
def test_cookie_domain_matching_does_not_leak_to_lookalike_hosts(self) -> None:
204+
url = get_httpbin_url('/cookies')
205+
host = urllib.parse.urlparse(url).hostname or ''
206+
# The host must have at least two labels for this test to be meaningful.
207+
assert '.' in host
208+
209+
first_label, _, parent_domain = host.partition('.')
210+
211+
cookies = Cookies()
212+
# These domain-match the host and must be sent.
213+
cookies.set('exact_match', 'yes', domain=host) # host == domain
214+
cookies.set('subdomain_ok', 'yes', domain=parent_domain) # host is a subdomain of the cookie domain
215+
216+
# These only match the host as a substring (not on a label boundary) and must NOT
217+
# be sent. With the old `.contains()` check both of these would leak.
218+
cookies.set('substring_leak', 'no', domain=host[1:]) # e.g. host `httpbin.org` vs `ttpbin.org`
219+
cookies.set('prefix_leak', 'no', domain=first_label) # e.g. host `httpbin.org` vs `httpbin`
220+
221+
response = impit.get(url, cookies=cookies).json()
222+
223+
assert response['cookies'] == {'exact_match': 'yes', 'subdomain_ok': 'yes'}
224+
225+
def test_cookie_domain_matching_for_ip_hosts(self) -> None:
226+
# RFC 6265 §5.1.3: subdomain/suffix matching applies to host names only, so an
227+
# IP-address host must match the cookie domain exactly. Without that rule a
228+
# cookie scoped to `0.0.1` would leak to host `127.0.0.1` (it is a label-boundary
229+
# suffix of the IP). A local server is used so the request host is a real IP.
230+
port_holder = [0]
231+
captured: dict[str, str] = {}
232+
thread = threading.Thread(target=cookie_echo_server, args=(port_holder, captured))
233+
thread.start()
234+
time.sleep(0.1)
235+
236+
cookies = Cookies()
237+
cookies.set('exact_ip', 'yes', domain='127.0.0.1') # exact IP match -> sent
238+
cookies.set('ip_suffix_leak', 'no', domain='0.0.1') # IP host matches exactly -> NOT sent
239+
240+
impit.get(f'http://127.0.0.1:{port_holder[0]}/', cookies=cookies, timeout=5)
241+
thread.join()
242+
243+
sent = captured.get('cookie', '')
244+
sent_names = {part.split('=')[0].strip() for part in sent.split(';') if '=' in part}
245+
assert sent_names == {'exact_ip'}
246+
177247
@pytest.mark.skip(reason='Flaky under the CI environment')
178248
def test_http3_works(self) -> None:
179249
response = impit.get('https://curl.se', force_http3=True)

0 commit comments

Comments
 (0)