You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "adityamparikh (via GitHub)" <gi...@apache.org> on 2026/05/02 21:41:09 UTC
[PR] fix(security): validate filter queries, sort fields, and pagination bounds [solr-mcp]
adityamparikh opened a new pull request, #127:
URL: https://github.com/apache/solr-mcp/pull/127
## Summary
The [MCP Tools specification](https://modelcontextprotocol.io/specification/2025-06-18/server/tools#security-considerations) requires servers to *"validate all tool inputs"*. The \`search\` tool previously passed three categories of caller input straight through to Solr without sanitization. This PR fixes all three.
### 1. Filter-query injection (\`fq\`)
\`filterQueries: List<String>\` was passed straight to \`solrQuery.addFilterQuery(...)\`. Solr's \`fq\` parameter uses the same standard query parser as \`q\`, so the \`{!xmlparser …}\`, \`{!join …}\`, and \`{!frange …}\` local-param injection vectors that #122 fixed for \`q\` are equally reachable via \`fq\`. Apply the same parameter-dereferencing pattern: bind each user-supplied filter to a separate \`fq{n}\` param and reference it from a constant \`{!edismax v=\$fq{n}}\`. ([CWE-943](https://cwe.mitre.org/data/definitions/943.html))
### 2. Sort-field injection
The \`item\` field of each \`sortClauses\` entry flowed into Solr's \`sort=\` parameter, which accepts function queries (\`sort=if(rord(_val_:…),1,0) asc\`) — enabling expensive query plans (lower-severity DoS). Validate field names against a strict \`^[a-zA-Z_][a-zA-Z0-9_.]*\$\` regex and reject anything else with \`IllegalArgumentException\`. The \`order\` value must parse as a valid \`SolrQuery.ORDER\` (\`asc\`/\`desc\`, case-insensitive).
### 3. Pagination bounds
\`rows\` and \`start\` were unbounded — a caller could request \`rows=2_000_000_000\` and OOM the JVM, or set \`start\` deep enough to trigger Solr's own deep-paging DoS surface. Clamp at \`MAX_ROWS=1000\` and \`MAX_START=100_000\` (both private static constants for easy adjustment), with a \`Math.max(value, 0)\` floor for negative inputs. ([CWE-1284](https://cwe.mitre.org/data/definitions/1284.html))
## Test plan
- [x] 6 new unit tests for \`fq\` (plain, xmlparser injection, join injection, frange injection, blank skipped, multiple filters)
- [x] 6 new unit tests for sort (valid plain, valid dotted, function-query injection, brace injection, \`_val_\` injection, invalid order)
- [x] 3 new unit tests for rows/start clamping (excessive rows clamped, null rows not set, excessive start clamped)
- [x] 2 existing tests updated to match the dereferenced form
- [x] \`./gradlew build\` passes (full suite, 37s)
## Note on PR ordering
Mirrors the parameter-dereferencing pattern from #122 (\`q\` injection). Both PRs touch \`SearchService.search\`; whichever merges later will need a trivial rebase.
## Out of scope
Rate limiting — the spec's other input-related \`MUST\`. Tracked separately so it can use a deliberate dependency choice (Bucket4j / resilience4j / Spring AOP).
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] fix(security): validate filter queries, sort fields, and pagination bounds [solr-mcp]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #127:
URL: https://github.com/apache/solr-mcp/pull/127#issuecomment-4710115193
i think these we don't want to merge in.. let's make Solr responsible for security.....
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] fix(security): validate filter queries, sort fields, and pagination bounds [solr-mcp]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh closed pull request #127: fix(security): validate filter queries, sort fields, and pagination bounds
URL: https://github.com/apache/solr-mcp/pull/127
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org