[jira] [Commented] (SOLR-12891) Injection Dangers in Streaming Expressions

["David Smiley (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/SOLR-12891?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17197901#comment-17197901 ] 

David Smiley commented on SOLR-12891:
-------------------------------------

I appreciate the attention to detail on identifying this problem, and the peer review here, but respectfully, IMO the committed solution of modifying MacroExpander to check for one special parameter is a total hack.  It feels weird/yucky that user input is subject to parameter substitution -- "q" param or whatever else you concoct into other parameters.  I'm not sure if the worst that could happen is merely lots of confusion (e.g. why does my attempt to search for code snippets not work when this syntax is used), or wether it might have sinister effects in some systems.  I'd instead rather see macro expansion simply not work on parameters sent into the request, except maybe an allow-list that would include the "json" parameter and maybe one or two others, at most.  I could also imagine a local-param for a query to explicitly opt-in.  Parameters specified in configuration (request handler or params.json) would always be eligible for macro-expansion.  WDYT?  I realize I'm proposing a breaking change that would only be acceptable for a major release (e.g. 9.0).

> Injection Dangers in Streaming Expressions
> ------------------------------------------
>
>                 Key: SOLR-12891
>                 URL: https://issues.apache.org/jira/browse/SOLR-12891
>             Project: Solr
>          Issue Type: Bug
>          Components: streaming expressions
>    Affects Versions: 7.5, 8.0
>            Reporter: Gus Heck
>            Priority: Minor
>              Labels: security
>             Fix For: 8.1, master (9.0)
>
>         Attachments: SOLR-12891.patch, SOLR-12891.patch, SOLR-12891.patch, SOLR-12891.patch, SOLR12819example.java
>
>
> I just spent some time fiddling with streaming expressions for fun, reading Erick Erickson's blog ([https://lucidworks.com/2017/12/06/streaming-expressions-in-solrj/)] and the example given in the ref guide ([https://lucene.apache.org/solr/guide/7_5/streaming-expressions.html#streaming-requests-and-responses)] and it occurred to me that we are recommending string concatenation into an expression language with the power to harm the server, or other network services visible from the server. I'm starting this Jira as a security issue to avoid creating a public impression of insecurity, feel free to undo that if I have guessed wrong. I haven't developed an exploit example, but it would go something like this:
>  # Some portion of an expression is built including user supplied data using the techniques we're recommending in the ref guide
>  # Malicious user constructs input data that breaks out of the expression (SOLR-10894 is relevant here), probably somewhere inside a let() expression where one could simply define an additional variable taking the value of a malicious expression...
>  # update() expression is executed to add/overwrite data, jdbc() makes a JDBC connection to a database visible to the server, or the malicious expression executes some very expensive expression for DOS effect.
> Technically this is of course the fault of the end user who allowed unchecked input into programmatic execution, but when I think about how to check the input I realize that the only way to be sure is to construct for myself a notion of exactly how the parser behaves and then determine what needs to be escaped. To do this I need to dig into the expression parser code...
> How to escape input is also already unclear as shown by SOLR-10894
> There's another important wrinkle that would easily be missed by someone trying to construct their own escaping/protection system relating to parameter substitution as discussed here: SOLR-8458 
> I think the solution to this is that SolrJ API should be enhanced to provide an escaping utility at a minimum and possibly a "prepared expression" similar to SQL prepared statements and call this issue to attention in the ref guide once these tools are available... 
> Additionally, templating features might be a useful addition to help folks manage large expressions and facilitate re-use of patterns... such templating should also have this issue in mind when/if they are added.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org