From 9491346128b2878e69ae5804d98f943f452b007a Mon Sep 17 00:00:00 2001 From: David Wayne Smiley Date: Mon, 26 Mar 2012 04:43:37 +0000 Subject: [PATCH] SOLR-3161 limit qt=/... (leading /) to refer to a SearchHandler for safety git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1305217 13f79535-47bb-0310-9956-ffa450edef68 --- solr/CHANGES.txt | 3 ++ .../solr/servlet/SolrDispatchFilter.java | 8 ++++- .../src/test-files/solr/conf/solrconfig.xml | 8 +++++ .../solr/request/TestRemoteStreaming.java | 35 ++++++++++++++++--- 4 files changed, 48 insertions(+), 6 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 6b81a956ea1..bf270b7adea 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -495,6 +495,9 @@ Upgrading from Solr 3.5 configuring /select as is done in the example solrconfig.xml, and register your other search handlers with a leading '/' which is a recommended practice. (David Smiley, Erik Hatcher) +* SOLR-3161: Don't use the 'qt' parameter with a leading '/'. It probably won't work in 4.0 + and it's now limited in 3.6 to SearchHandler subclasses that aren't lazy-loaded. + New Features ---------------------- * SOLR-2854: Now load URL content stream data (via stream.url) when called for during request handling, diff --git a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java b/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java index 4c5656dcfb9..6a17ea6b8f0 100644 --- a/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java +++ b/solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java @@ -28,6 +28,7 @@ import java.util.Map; import java.util.Map.Entry; import java.util.Set; import java.util.WeakHashMap; + import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.xml.sax.InputSource; @@ -49,10 +50,10 @@ import org.apache.solr.common.cloud.Slice; import org.apache.solr.common.cloud.ZkNodeProps; import org.apache.solr.common.cloud.ZkStateReader; import org.apache.solr.common.params.CommonParams; -import org.apache.solr.common.params.ModifiableSolrParams; import org.apache.solr.common.util.FastWriter; import org.apache.solr.common.util.ContentStreamBase; import org.apache.solr.core.*; +import org.apache.solr.handler.component.SearchHandler; import org.apache.solr.request.*; import org.apache.solr.response.BinaryQueryResponseWriter; import org.apache.solr.response.QueryResponseWriter; @@ -229,6 +230,11 @@ public class SolrDispatchFilter implements Filter if( handler == null ) { throw new SolrException( SolrException.ErrorCode.BAD_REQUEST, "unknown handler: "+qt); } + if( qt != null && qt.startsWith("/") && !(handler instanceof SearchHandler)) { + //For security reasons it's a bad idea to allow a leading '/', ex: /select?qt=/update see SOLR-3161 + //There was no restriction from Solr 1.4 thru 3.5 and it's now only supported for SearchHandlers. + throw new SolrException( SolrException.ErrorCode.BAD_REQUEST, "Invalid query type. Do not use /select to access: "+qt); + } } } } diff --git a/solr/core/src/test-files/solr/conf/solrconfig.xml b/solr/core/src/test-files/solr/conf/solrconfig.xml index 8ea536b36a1..9ab9ae200c4 100644 --- a/solr/core/src/test-files/solr/conf/solrconfig.xml +++ b/solr/core/src/test-files/solr/conf/solrconfig.xml @@ -502,6 +502,14 @@ + + + + explicit + true + + + solr solrconfig.xml scheam.xml admin-extra.html diff --git a/solr/core/src/test/org/apache/solr/request/TestRemoteStreaming.java b/solr/core/src/test/org/apache/solr/request/TestRemoteStreaming.java index f663320225e..0f0795f49e6 100644 --- a/solr/core/src/test/org/apache/solr/request/TestRemoteStreaming.java +++ b/solr/core/src/test/org/apache/solr/request/TestRemoteStreaming.java @@ -23,9 +23,10 @@ import org.apache.solr.client.solrj.SolrQuery; import org.apache.solr.client.solrj.SolrServer; import org.apache.solr.client.solrj.SolrServerException; import org.apache.solr.client.solrj.impl.CommonsHttpSolrServer; +import org.apache.solr.client.solrj.request.QueryRequest; import org.apache.solr.client.solrj.response.QueryResponse; +import org.apache.solr.common.SolrException; import org.apache.solr.common.SolrInputDocument; -import org.apache.solr.util.ExternalPaths; import org.junit.Before; import org.junit.BeforeClass; import org.junit.Test; @@ -44,7 +45,8 @@ public class TestRemoteStreaming extends SolrJettyTestBase { @BeforeClass public static void beforeTest() throws Exception { - createJetty(ExternalPaths.EXAMPLE_HOME, null, null); + //this one has handleSelect=true which a test here needs + createJetty("solr/", null, null); } @Before @@ -52,7 +54,7 @@ public class TestRemoteStreaming extends SolrJettyTestBase { //add document and commit, and ensure it's there SolrServer server1 = getSolrServer(); SolrInputDocument doc = new SolrInputDocument(); - doc.addField( "id", "xxxx" ); + doc.addField( "id", "1234" ); server1.add(doc); server1.commit(); assertTrue(searchFindsIt()); @@ -71,7 +73,7 @@ public class TestRemoteStreaming extends SolrJettyTestBase { String getUrl = solrServer.getBaseURL()+"/debug/dump?wt=xml&stream.url="+URLEncoder.encode(streamUrl,"UTF-8"); String content = getUrlForString(getUrl); - assertTrue(content.contains("xxxx")); + assertTrue(content.contains("1234")); //System.out.println(content); } @@ -100,6 +102,29 @@ public class TestRemoteStreaming extends SolrJettyTestBase { assertTrue(searchFindsIt());//still there } + /** SOLR-3161 + * Technically stream.body isn't remote streaming, but there wasn't a better place for this test method. */ + @Test(expected = SolrException.class) + public void testQtUpdateFails() throws SolrServerException { + SolrQuery query = new SolrQuery(); + query.setQuery( "*:*" );//for anything + query.add("echoHandler","true"); + //sneaky sneaky + query.add("qt","/update"); + query.add("stream.body","*:*"); + + QueryRequest queryRequest = new QueryRequest(query) { + @Override + public String getPath() { //don't let superclass substitute qt for the path + return "/select"; + } + }; + QueryResponse rsp = queryRequest.process(getSolrServer()); + //!! should *fail* above for security purposes + String handler = (String) rsp.getHeader().get("handler"); + System.out.println(handler); + } + /** Compose a url that if you get it, it will delete all the data. */ private String makeDeleteAllUrl() throws UnsupportedEncodingException { CommonsHttpSolrServer solrServer = (CommonsHttpSolrServer) getSolrServer(); @@ -109,7 +134,7 @@ public class TestRemoteStreaming extends SolrJettyTestBase { private boolean searchFindsIt() throws SolrServerException { SolrQuery query = new SolrQuery(); - query.setQuery( "id:xxxx" ); + query.setQuery( "id:1234" ); QueryResponse rsp = getSolrServer().query(query); return rsp.getResults().getNumFound() != 0; }