Merge branch 'main' into security/ssrf-dns-pinning-and-object-iri

semantica/explorer/routes/sparql.py

@@ -51,8 +51,26 @@ _FORBIDDEN_KEYWORDS = re.compile(
 # the query. BASE declarations have no prefix name between the keyword and
 # the IRI (`BASE <...>`, vs. `PREFIX ex: <...>`), so the prefix-name token
 # is optional.
+#
+# ReDoS fix (CodeQL py/polynomial-redos, issue #1897):
+#
+# The original pattern `<[^>]*>\s*` was vulnerable because `\s*` (which
+# matches newlines) could overlap with `[^>]*` on inputs that contain no
+# closing `>` (e.g. `base<!!<!<...`), forcing the engine to explore every
+# possible split between the two quantifiers — O(n²) backtracking.
+#
+# The fix uses `<[^>\r\n]*>` for the IRI body: excluding CR and LF from
+# the character class means the IRI match can never span a line boundary,
+# and the disjoint trailing `[ \t]*` (horizontal whitespace only) has zero
+# character-class overlap with `[^>\r\n]*`, so the engine has exactly one
+# way to match.  No end-of-line anchor is needed or used, which correctly
+# handles both inline prologues (`PREFIX ex: <...> SELECT ...` on one line)
+# and CRLF line endings (`\r\n`) without any special casing.
 _COMMENT_LINE = re.compile(r"(?:^|(?<=\s))#[^\n]*", re.MULTILINE)
-_PREFIX_DECL = re.compile(r"^\s*(?:PREFIX\s+\S+|BASE)\s*<[^>]*>\s*", re.IGNORECASE | re.MULTILINE)
+_PREFIX_DECL = re.compile(
+    r"^[ \t]*(?:PREFIX[ \t]+\S+|BASE)[ \t]*<[^>\r\n]*>[ \t]*",
+    re.IGNORECASE | re.MULTILINE,
+)
 
 
 def _is_read_only_query(query: str) -> bool:
@@ -62,6 +80,10 @@ def _is_read_only_query(query: str) -> bool:
     checking the first keyword. Also rejects queries containing SPARQL Update
     keywords anywhere in the body, preventing injection via embedded strings
     or multi-statement tricks.
+
+    Note: callers are responsible for enforcing any input-length limit *before*
+    calling this function so that an oversized-query rejection can be surfaced
+    as a distinct, actionable error rather than the generic read-only message.
     """
     # 1. Remove single-line comments that could hide the real query type
     cleaned = _COMMENT_LINE.sub("", query)
@@ -156,6 +178,11 @@ _SPARQL_MAX_ROWS = 5_000     # hard cap on returned rows
 _SPARQL_TIMEOUT_S = 30       # seconds before abandoning the await
 _SPARQL_MAX_CONCURRENT = 4   # semaphore: max simultaneous executions
 _SPARQL_MAX_GRAPH_NODES = 50_000  # cap on graph nodes/edges to prevent OOM
+# Defense-in-depth against ReDoS: reject inputs longer than this before any
+# regex work so that even a future regex regression is bounded.  Checked in
+# execute_sparql() (not inside _is_read_only_query) so the route can return
+# a distinct, actionable error message rather than the generic read-only one.
+_SPARQL_MAX_QUERY_LEN = 10_000  # chars
 
 # Semaphore caps how many graph.query calls run concurrently so that
 # timed-out threads (which keep running in the pool) cannot crowd out
@@ -184,6 +211,24 @@ async def execute_sparql(
     req: SparqlRequest,
     session: GraphSession = Depends(get_session),
 ):
+    # Resource-limit check: reject oversized queries before any regex work.
+    # This is intentionally a separate, earlier check from _is_read_only_query
+    # so clients receive a specific, actionable message rather than the generic
+    # read-only rejection, and operators can tune _SPARQL_MAX_QUERY_LEN without
+    # touching query-semantics code.
+    if len(req.query) > _SPARQL_MAX_QUERY_LEN:
+        return SparqlResponse(
+            columns=[],
+            rows=[],
+            total=0,
+            error=(
+                f"Query exceeds the maximum allowed length of "
+                f"{_SPARQL_MAX_QUERY_LEN:,} characters "
+                f"({len(req.query):,} received). "
+                f"Please shorten your query."
+            ),
+        )
+
     if not _is_read_only_query(req.query):
         return SparqlResponse(
             columns=[],

tests/explorer/test_sparql_route.py

@@ -317,6 +317,47 @@ def test_oversized_graph_returns_clean_error_not_a_crash(client):
     assert payload["rows"] == []
 
 
+def test_oversized_query_returns_distinct_length_error(client):
+    """A query exceeding _SPARQL_MAX_QUERY_LEN must be rejected with a
+    specific, actionable error message — not the generic read-only message.
+    Clients need to distinguish a size-limit rejection from an actual
+    non-read-only query rejection to react correctly (e.g. split the query
+    vs. rewrite it)."""
+    with patch.object(sparql_mod, "_SPARQL_MAX_QUERY_LEN", 10):
+        resp = _post(client, "SELECT ?s WHERE { ?s ?p ?o }")  # 30 chars > 10
+    assert resp.status_code == 200
+    payload = resp.json()
+    assert payload["error"] is not None
+    # Must mention the limit, not the generic read-only message
+    assert "length" in payload["error"].lower() or "characters" in payload["error"].lower()
+    assert "Only SELECT" not in payload["error"]
+    assert payload["rows"] == []
+    assert payload["columns"] == []
+    assert payload["total"] == 0
+
+
+def test_oversized_query_never_touches_the_graph(client):
+    """An oversized query must be rejected before _build_rdflib_graph is
+    called — the length guard must short-circuit the entire pipeline."""
+    with patch.object(sparql_mod, "_SPARQL_MAX_QUERY_LEN", 10):
+        with patch.object(sparql_mod, "_build_rdflib_graph") as mock_build:
+            resp = _post(client, "SELECT ?s WHERE { ?s ?p ?o }")
+    assert resp.status_code == 200
+    assert resp.json()["error"] is not None
+    mock_build.assert_not_called()
+
+
+def test_query_exactly_at_length_limit_is_accepted(client):
+    """A query whose length equals the limit exactly must not be rejected —
+    the guard is strictly greater-than, not greater-than-or-equal."""
+    short_query = "ASK {}"
+    with patch.object(sparql_mod, "_SPARQL_MAX_QUERY_LEN", len(short_query)):
+        resp = _post(client, short_query)
+    assert resp.status_code == 200
+    payload = resp.json()
+    assert payload["error"] is None
+
+
 # ---------------------------------------------------------------------------
 # Data-mapping fidelity: does the graph->RDF projection reflect session state?
 # ---------------------------------------------------------------------------

tests/test_security_regression.py

@@ -123,6 +123,43 @@ class TestSparqlReadOnlyValidation:
         )
         assert _is_read_only_query(query)
 
+    def test_inline_prefix_before_select_allowed(self):
+        """PREFIX declaration on the same line as the query keyword (inline
+        prologue) must be stripped correctly so SELECT is seen first.
+        Regression for the [ \\t]*(?:\\n|$) anchor that rejected this form."""
+        query = "PREFIX ex: <http://example.org/> SELECT ?s WHERE { ?s ex:p ?o }"
+        assert _is_read_only_query(query)
+
+    def test_crlf_line_endings_with_prefix(self):
+        """Windows CRLF line endings (\\r\\n) between PREFIX and SELECT must
+        be handled correctly.  The previous (?:\\n|$) anchor did not allow
+        the \\r before \\n, causing stripping to fail."""
+        query = "PREFIX ex: <http://example.org/>\r\nSELECT ?s WHERE { ?s ?p ?o }"
+        assert _is_read_only_query(query)
+
+    def test_crlf_multiple_prefixes_then_select(self):
+        """Multiple PREFIX lines with CRLF endings should all be stripped."""
+        query = (
+            "PREFIX rdf: <http://www.w3.org/1999/02/22-rdf-syntax-ns#>\r\n"
+            "PREFIX ex: <http://example.org/>\r\n"
+            "SELECT ?s WHERE { ?s rdf:type ex:Thing }"
+        )
+        assert _is_read_only_query(query)
+
+    def test_inline_prefix_before_insert_still_blocked(self):
+        """An inline PREFIX followed by INSERT must not be allowed — the
+        inline stripping fix must not open a bypass for write operations."""
+        query = "PREFIX ex: <http://example.org/> INSERT DATA { ex:s ex:p ex:o }"
+        assert not _is_read_only_query(query)
+
+    def test_long_valid_query_not_rejected_by_is_read_only(self):
+        """_is_read_only_query must not enforce the length limit itself —
+        that responsibility belongs to execute_sparql() so the route can
+        return a distinct, actionable error.  A syntactically valid but long
+        SELECT query must still return True from this function."""
+        long_query = "SELECT ?s WHERE { ?s ?p ?o } # " + ("x" * 20_000)
+        assert _is_read_only_query(long_query)
+
 
 # ===================================================================
 # 2. Cypher injection prevention