Skip to content

Commit 054de71

Browse files
phanak-sapfilak-sap
phanak-sap
authored and
filak-sap
committed
service: guard against cross-origin __next URL redirection
 OData servers can legitimately paginate across path segments, but the __next value is server-controlled and untrusted. Validate that its origin (scheme + host + port) matches the configured service root before dispatching, so session-level credentials are never forwarded to a host the application did not configure. Raises PyODataException on mismatch. Fix is applied in ODataHttpRequest._build_request(), the single point both execute() (sync) and async_execute() (async) pass through, so one check covers all dispatch paths. Depends on stdlib urllib.parse only, keeping pyodata networking-library agnostic.
1 parent a7f298f commit 054de71

3 files changed

Lines changed: 50 additions & 3 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,16 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
66

77
## [Unreleased]
88

9-
- service: `response_hook` parameter, enables inspection or rejection of raw responses (e.g. header-encoded SAP domain errors) without leaking HTTP transport objects through the OData API boundary.
10-
- vendor/SAP: `sap_header_error_hook(response)` — a stateless hook that detects SAP domain errors encoded in the `sap-message` response header and raises `BusinessGatewayError` before pyodata's domain handler runs.
9+
### Added
10+
11+
- service: `response_hook` parameter, enables inspection or rejection of raw responses (e.g. header-encoded SAP domain errors) without leaking HTTP transport objects through the OData API boundary. - Petr Hanak
12+
- vendor/SAP: `sap_header_error_hook(response)` — a stateless hook that detects SAP domain errors encoded in the `sap-message` response header and raises `BusinessGatewayError` before pyodata's domain handler runs. - Petr Hanak
1113
- service: let FunctionRequests return a list of EntityProxies instead of the raw json, when the `ReturnType` is a Collection. - Emil B.
14+
15+
### Fixed
16+
1217
- model: replace regexp-based ISO datetime parsing with `datetime.fromisoformat` for `Edm.DateTime` and `Edm.DateTimeOffset` - Petr Hanak
18+
- service: guard against cross-origin __next URL redirection - Petr Hanak
1319

1420
### Removed
1521
- Python 3.9 is no longer supported by pyodata. Python 3.10 is now the minimal supported version.

‎pyodata/v2/service.py‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
from email.parser import Parser
1414
from http.client import HTTPResponse
1515
from io import BytesIO
16-
from urllib.parse import urlencode, quote
16+
from urllib.parse import urlencode, quote, urlparse
1717

1818

1919
from pyodata.exceptions import HttpError, PyODataException, ExpressionError, ProgramError
@@ -296,6 +296,13 @@ def add_headers(self, value):
296296

297297
def _build_request(self):
298298
if self._next_url:
299+
parsed_next = urlparse(self._next_url)
300+
parsed_base = urlparse(self._url)
301+
if (parsed_next.scheme, parsed_next.netloc) != (parsed_base.scheme, parsed_base.netloc):
302+
raise PyODataException(
303+
f'cross-origin __next URL rejected: {self._next_url!r} differs from '
304+
f'service root {self._url!r}'
305+
)
299306
url = self._next_url
300307
else:
301308
url = urljoin(self._url, self.get_path())

‎tests/test_service_v2.py‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2429,6 +2429,40 @@ def test_partial_listing(service):
24292429
assert result.next_url is None
24302430

24312431

2432+
@responses.activate
2433+
def test_next_url_cross_origin_raises(service):
2434+
"""__next URL pointing to a different origin must be refused before any request is dispatched."""
2435+
# pylint: disable=redefined-outer-name
2436+
cross_origin_next = "http://attacker.example.com/collect?$skiptoken=opaque"
2437+
2438+
request = service.entity_sets.Employees.get_entities().next_url(cross_origin_next)
2439+
with pytest.raises(PyODataException, match="cross-origin"):
2440+
request.execute()
2441+
assert len(responses.calls) == 0
2442+
2443+
2444+
@responses.activate
2445+
def test_next_url_same_origin_allowed(service):
2446+
"""__next URL on the same origin (scheme + host + port) must be followed normally."""
2447+
# pylint: disable=redefined-outer-name
2448+
same_origin_next = f"{service.url}/Employees?$skiptoken=safe"
2449+
2450+
responses.add(
2451+
responses.GET,
2452+
same_origin_next,
2453+
json={'d': {
2454+
'results': [
2455+
{'ID': 23, 'NameFirst': 'Rob', 'NameLast': 'Ickes'}
2456+
]
2457+
}},
2458+
status=200)
2459+
2460+
request = service.entity_sets.Employees.get_entities().next_url(same_origin_next)
2461+
result = request.execute()
2462+
assert len(result) == 1
2463+
assert result[0].ID == 23
2464+
2465+
24322466
@responses.activate
24332467
def test_count_with_chainable_filter_lt_operator(service):
24342468
"""Check getting $count with $filter with new filter syntax using multiple filters"""

0 commit comments

Comments
 (0)