Solr
  1. Solr
  2. SOLR-8308

Core gets inaccessible after RENAME operation with special characters

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 5.5, 6.0
    • Component/s: None
    • Labels:
      None

      Description

      You can rename a core using the following modified URL https://SOLR:PORT/solr/admin/cores?wt=json&indexInfo=false&action=RENAME&core=test_app_shared2_replica2&other=%3Csvg+onload%3Dalert(1)%3E&_=1445468005152. The core becomes inaccessible / unusable. There should be more form validation to the core name assignment

      1. SOLR-8308.patch
        10 kB
        Erick Erickson
      2. SOLR-8308.patch
        5 kB
        Erick Erickson
      3. SOLR-8308.patch
        1 kB
        Erik Hatcher
      4. SOLR-8308.patch
        1 kB
        Erik Hatcher

        Issue Links

          Activity

          Hide
          Erik Hatcher added a comment -

          Using a clean branch_5x checkout, I'm using these steps to reproduce:

          $ bin/solr start
          $ tail -f server/logs/solr.log &
          $ bin/solr create -c foo
          $ curl "http://localhost:8983/solr/admin/cores?wt=json&indexInfo=false&action=RENAME&core=foo&other=%3Csvg+onload%3Dalert(1)%3E"
          {"responseHeader":{"status":0,"QTime":1}}
          

          The rename worked, and is logged as:

          2015-11-19 19:20:24.521 INFO  (qtp434176574-42) [   ] o.a.s.c.CoreContainer registering core: <svg onload=alert(1)>
          2015-11-19 19:20:24.522 INFO  (qtp434176574-42) [   ] o.a.s.s.SolrDispatchFilter [admin] webapp=null path=/admin/cores params={core=foo&other=<svg+onload%3Dalert(1)>&indexInfo=false&action=RENAME&wt=json} status=0 QTime=1 
          

          And is reported fine in the /admin/cores?wt=json&indent=on:

            "status":{
              "<svg onload=alert(1)>":{
                "name":"<svg onload=alert(1)>",
          

          It wasn't able to /select on that renamed core though, but maybe there's some funky URL escaping that can be done to achieve even that?

          In the default admin UI (the "old" one), the renamed core is not really selectable (it appears as a grey rectangle but otherwise seemingly inactive).

          Clicking over to the "New UI" with the top-right link, navigating to Core Admin I see the silly named core just fine, it's selectable and seems to otherwise work fine core-admin-ui-wise.

          In neither old or new admin UI's did I get the alert (though does it work that way on an svg tag?) or seem to have problems other than not being able to get a URL to /select to work. No doubt we shouldn't allow such "non-identifier"-like names, but I'm not seeing an XSS vulnerability per se.

          How can the XSS vulnerability be demonstrated? Or maybe/hopefully it's just an annoyance that the core is no longer really addressable?

          Show
          Erik Hatcher added a comment - Using a clean branch_5x checkout, I'm using these steps to reproduce: $ bin/solr start $ tail -f server/logs/solr.log & $ bin/solr create -c foo $ curl "http: //localhost:8983/solr/admin/cores?wt=json&indexInfo= false &action=RENAME&core=foo&other=%3Csvg+onload%3Dalert(1)%3E" { "responseHeader" :{ "status" :0, "QTime" :1}} The rename worked, and is logged as: 2015-11-19 19:20:24.521 INFO (qtp434176574-42) [ ] o.a.s.c.CoreContainer registering core: <svg onload=alert(1)> 2015-11-19 19:20:24.522 INFO (qtp434176574-42) [ ] o.a.s.s.SolrDispatchFilter [admin] webapp= null path=/admin/cores params={core=foo&other=<svg+onload%3Dalert(1)>&indexInfo= false &action=RENAME&wt=json} status=0 QTime=1 And is reported fine in the /admin/cores?wt=json&indent=on: "status" :{ "<svg onload=alert(1)>" :{ "name" : "<svg onload=alert(1)>" , It wasn't able to /select on that renamed core though, but maybe there's some funky URL escaping that can be done to achieve even that? In the default admin UI (the "old" one), the renamed core is not really selectable (it appears as a grey rectangle but otherwise seemingly inactive). Clicking over to the "New UI" with the top-right link, navigating to Core Admin I see the silly named core just fine, it's selectable and seems to otherwise work fine core-admin-ui-wise. In neither old or new admin UI's did I get the alert (though does it work that way on an svg tag?) or seem to have problems other than not being able to get a URL to /select to work. No doubt we shouldn't allow such "non-identifier"-like names, but I'm not seeing an XSS vulnerability per se. How can the XSS vulnerability be demonstrated? Or maybe/hopefully it's just an annoyance that the core is no longer really addressable?
          Hide
          Erik Hatcher added a comment -

          strawman patch. running tests now, and see some failures so it's already too strict: "Invalid core name: .system_shard1_replica1". what's the right pattern to allow for core names?

          Show
          Erik Hatcher added a comment - strawman patch. running tests now, and see some failures so it's already too strict: "Invalid core name: .system_shard1_replica1". what's the right pattern to allow for core names?
          Hide
          Erik Hatcher added a comment -

          Here's an updated patch that is more lenient:

          ^[\._A-Za-z0-9]*$
          Show
          Erik Hatcher added a comment - Here's an updated patch that is more lenient: ^[\._A-Za-z0-9]*$
          Hide
          Erik Hatcher added a comment -

          with the latest patch, all tests pass and the described steps above to rename the core results in a core rename exception.

          Show
          Erik Hatcher added a comment - with the latest patch, all tests pass and the described steps above to rename the core results in a core rename exception.
          Hide
          Uwe Schindler added a comment -

          Hi, we should rename this issue (summary title), so it does not mention incorrect XSS. This is no XSS issue, so we should be careful with alerting people on the release. There is no security related stuff involved. That the core gets inaccessible ist the real bug.

          Show
          Uwe Schindler added a comment - Hi, we should rename this issue (summary title), so it does not mention incorrect XSS. This is no XSS issue, so we should be careful with alerting people on the release. There is no security related stuff involved. That the core gets inaccessible ist the real bug.
          Hide
          Alan Woodward added a comment -

          Hi Erik, could you move the name checks to a separate validateCoreName() method, and call it from both createCore() and rename(), rather than in registerCore()? At the moment if the validation fails after creation there's still a SolrCore object in the container taking up resources, it's just not registered in the cluster state.

          Show
          Alan Woodward added a comment - Hi Erik, could you move the name checks to a separate validateCoreName() method, and call it from both createCore() and rename(), rather than in registerCore()? At the moment if the validation fails after creation there's still a SolrCore object in the container taking up resources, it's just not registered in the cluster state.
          Hide
          Uwe Schindler added a comment -

          Pattern p = Pattern.compile("^\\._A-Za-z0-9*$");

          You should define the pattern static final on the class. Otherwise the ^ and $ are not needed because you use Matcher#matches() that always matches whole String.

          Show
          Uwe Schindler added a comment - Pattern p = Pattern.compile("^ \\._A-Za-z0-9 *$"); You should define the pattern static final on the class. Otherwise the ^ and $ are not needed because you use Matcher#matches() that always matches whole String.
          Hide
          Uwe Schindler added a comment -

          It should also be an IllegalArgumentException instead of unspecific RuntimeException.

          Show
          Uwe Schindler added a comment - It should also be an IllegalArgumentException instead of unspecific RuntimeException .
          Hide
          Erick Erickson added a comment -

          Patch with Alan's and Uwe's suggestions (testing now).

          Show
          Erick Erickson added a comment - Patch with Alan's and Uwe's suggestions (testing now).
          Hide
          Erick Erickson added a comment -

          Patch with tests. Did you know we didn't have any test for a core rename?

          Anyway, first Git patch so let's see if I did it right.

          precommit and test pass, does anyone see any reason not to commit? This changes one bit of behavior, you can't rename with bogus core names. It does not affect cores that already exist with bogus names though, so shouldn't break existing installations.

          Show
          Erick Erickson added a comment - Patch with tests. Did you know we didn't have any test for a core rename? Anyway, first Git patch so let's see if I did it right. precommit and test pass, does anyone see any reason not to commit? This changes one bit of behavior, you can't rename with bogus core names. It does not affect cores that already exist with bogus names though, so shouldn't break existing installations.
          Hide
          ASF subversion and git services added a comment -

          Commit 70f87420ab106989c9501870f2f851d5f5f85ea5 in lucene-solr's branch refs/heads/master from Erick Erickson
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=70f8742 ]

          SOLR-8308: Core gets inaccessible after RENAME operation with special characters

          Show
          ASF subversion and git services added a comment - Commit 70f87420ab106989c9501870f2f851d5f5f85ea5 in lucene-solr's branch refs/heads/master from Erick Erickson [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=70f8742 ] SOLR-8308 : Core gets inaccessible after RENAME operation with special characters
          Hide
          ASF subversion and git services added a comment -

          Commit 7dcb07723cdd90066857bacac7d5e31eda22f8ce in lucene-solr's branch refs/heads/branch_5x from Erick Erickson
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=7dcb077 ]

          SOLR-8308: Core gets inaccessible after RENAME operation with special characters
          (cherry picked from commit 70f8742)

          Show
          ASF subversion and git services added a comment - Commit 7dcb07723cdd90066857bacac7d5e31eda22f8ce in lucene-solr's branch refs/heads/branch_5x from Erick Erickson [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=7dcb077 ] SOLR-8308 : Core gets inaccessible after RENAME operation with special characters (cherry picked from commit 70f8742)
          Hide
          ASF subversion and git services added a comment -

          Commit 4de5f1d99d3ed36b2b09c147a5e4797caecc2ac8 in lucene-solr's branch refs/heads/branch_5x from Erick Erickson
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=4de5f1d ]

          SOLR-8308: Core gets inaccessible after RENAME operation with special characters
          (cherry picked from commit 70f8742)

          Show
          ASF subversion and git services added a comment - Commit 4de5f1d99d3ed36b2b09c147a5e4797caecc2ac8 in lucene-solr's branch refs/heads/branch_5x from Erick Erickson [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=4de5f1d ] SOLR-8308 : Core gets inaccessible after RENAME operation with special characters (cherry picked from commit 70f8742)
          Hide
          Erick Erickson added a comment -

          OK, my first-ever Git fix, let me know what's wrong.

          Sorry for the gratuitous newline changes, I think I've fixed my IntelliJ to not do that any more, but this patch is small enough that I don't think they're worth fixing.

          P.S. I already know about the double entry for this JIRA in CHANGES.txt, fixing momentarily.

          Show
          Erick Erickson added a comment - OK, my first-ever Git fix, let me know what's wrong. Sorry for the gratuitous newline changes, I think I've fixed my IntelliJ to not do that any more, but this patch is small enough that I don't think they're worth fixing. P.S. I already know about the double entry for this JIRA in CHANGES.txt, fixing momentarily.
          Hide
          ASF subversion and git services added a comment -

          Commit 2c0a66bfbc5937907d290b799e025d15c0ba098e in lucene-solr's branch refs/heads/master from Erick Erickson
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=2c0a66b ]

          Took out duplicate mention of SOLR-8308 (was only in trunk)

          Show
          ASF subversion and git services added a comment - Commit 2c0a66bfbc5937907d290b799e025d15c0ba098e in lucene-solr's branch refs/heads/master from Erick Erickson [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=2c0a66b ] Took out duplicate mention of SOLR-8308 (was only in trunk)
          Hide
          ASF subversion and git services added a comment -

          Commit 70f87420ab106989c9501870f2f851d5f5f85ea5 in lucene-solr's branch refs/heads/lucene-6835 from Erick Erickson
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=70f8742 ]

          SOLR-8308: Core gets inaccessible after RENAME operation with special characters

          Show
          ASF subversion and git services added a comment - Commit 70f87420ab106989c9501870f2f851d5f5f85ea5 in lucene-solr's branch refs/heads/lucene-6835 from Erick Erickson [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=70f8742 ] SOLR-8308 : Core gets inaccessible after RENAME operation with special characters
          Hide
          ASF subversion and git services added a comment -

          Commit 2c0a66bfbc5937907d290b799e025d15c0ba098e in lucene-solr's branch refs/heads/lucene-6835 from Erick Erickson
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=2c0a66b ]

          Took out duplicate mention of SOLR-8308 (was only in trunk)

          Show
          ASF subversion and git services added a comment - Commit 2c0a66bfbc5937907d290b799e025d15c0ba098e in lucene-solr's branch refs/heads/lucene-6835 from Erick Erickson [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=2c0a66b ] Took out duplicate mention of SOLR-8308 (was only in trunk)

            People

            • Assignee:
              Erick Erickson
              Reporter:
              Adam Johnson
            • Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development