Solr
  1. Solr
  2. SOLR-2631

PingRequestHandler can infinite loop if called with a qt that points to itsself

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.4, 3.1, 3.2, 3.3
    • Fix Version/s: 3.4, 4.0-ALPHA
    • Component/s: search, web gui
    • Labels:

      Description

      We got a security report to private@lucene.apache.org, that Solr can infinite loop, use 100% CPU and stack overflow, if you execute the following HTTP request:

      The qt paramter instructs PingRequestHandler to call the given request handler. This leads to an infinite loop. This is not an security issue, but for an unprotected Solr server with unprotected /solr/select path this makes it stop working.

      The fix is to prevent infinite loop by disallowing calling itsself.

      1. SOLR-2631.patch
        0.7 kB
        Uwe Schindler

        Activity

        Hide
        Uwe Schindler added a comment -

        Edoardo Tosca, who reported the issue, gave the following workaround for solrconfig.xml to fix this by configuration:

        Ok,
        to solve the Ping problem you can add an invariant:
        <lst name="defaults">
        <str name="q">solrpingquery</str>
        <str name="echoParams">all</str>
        </lst>
        <lst name="invariants">
        <str name="qt">search</str>
        </lst>

        in this case you avoid generating recursive calls to /admin/ping handler

        Edo

        Show
        Uwe Schindler added a comment - Edoardo Tosca, who reported the issue, gave the following workaround for solrconfig.xml to fix this by configuration: Ok, to solve the Ping problem you can add an invariant: <lst name="defaults"> <str name="q">solrpingquery</str> <str name="echoParams">all</str> </lst> <lst name="invariants"> <str name="qt">search</str> </lst> in this case you avoid generating recursive calls to /admin/ping handler Edo
        Hide
        Uwe Schindler added a comment -

        This patch fixes the bug.

        Hoss said, we could also simply check the qt param but I decided to do the instanceof check: If the PingRequestHandler is registered multiple times in the solrconfig.xml (e.g. by different URI paths or different names), the infinite loop could still occur. The PingRequestHandler should generally disallow calling itsself.

        Show
        Uwe Schindler added a comment - This patch fixes the bug. Hoss said, we could also simply check the qt param but I decided to do the instanceof check: If the PingRequestHandler is registered multiple times in the solrconfig.xml (e.g. by different URI paths or different names), the infinite loop could still occur. The PingRequestHandler should generally disallow calling itsself.
        Hide
        Uwe Schindler added a comment -

        Committed trunk revision: 1142179
        Merged 3.x revision: 1142180

        Thanks Edoardo!

        Show
        Uwe Schindler added a comment - Committed trunk revision: 1142179 Merged 3.x revision: 1142180 Thanks Edoardo!
        Hide
        Hoss Man added a comment -

        we shouldn't consider tis resolved until we also apply the "work around" provided by Edoardo to the example and test solrconfig.xml files as well.

        that way people who copy them in the future won't get any error at all.

        Show
        Hoss Man added a comment - we shouldn't consider tis resolved until we also apply the "work around" provided by Edoardo to the example and test solrconfig.xml files as well. that way people who copy them in the future won't get any error at all.
        Hide
        Uwe Schindler added a comment -

        Sorry, I don't understand what you want to say - please give details. With this fix there is no problem at all, so why change default configs?

        Show
        Uwe Schindler added a comment - Sorry, I don't understand what you want to say - please give details. With this fix there is no problem at all, so why change default configs?
        Hide
        Hoss Man added a comment -

        Uwe, sorry for my brevity – my point was that you had fixed the infinite loop by adding an sanity check that will throw an error, but the example & test configs should also be improved to demonstrate better practices when using the PingRequestHandler so people using them can never encounter the sanity checking you added.

        Committed revision 1142722. - trunk
        Committed revision 1142730. - trunk stupid mistake
        Committed revision 1142731. - 3x

        Show
        Hoss Man added a comment - Uwe, sorry for my brevity – my point was that you had fixed the infinite loop by adding an sanity check that will throw an error, but the example & test configs should also be improved to demonstrate better practices when using the PingRequestHandler so people using them can never encounter the sanity checking you added. Committed revision 1142722. - trunk Committed revision 1142730. - trunk stupid mistake Committed revision 1142731. - 3x
        Hide
        Robert Muir added a comment -

        bulk close for 3.4

        Show
        Robert Muir added a comment - bulk close for 3.4

          People

          • Assignee:
            Uwe Schindler
            Reporter:
            Uwe Schindler
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development