Sling
  1. Sling
  2. SLING-1308

Node.infinity.json contains risk for DOS.

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: Servlets Get 2.0.8
    • Fix Version/s: Servlets Get 2.1.0
    • Component/s: Servlets
    • Labels:
      None

      Description

      As it is now any user can do a node.infinity.json .
      If this happens on the root node in a repository with many items, it will cause the server to slow down (eventually crash?)
      I've created a patch confirming the discussion @ http://markmail.org/search/?q=node.infinity#query:node.infinity+page:1+mid:ugqjyqdz2trfpdkr+state:results

      1. jsonRenderer.diff
        19 kB
        Simon Gaeremynck
      2. jsonRenderer.diff
        17 kB
        Simon Gaeremynck

        Activity

        Hide
        Simon Gaeremynck added a comment -

        Attached patch.

        Show
        Simon Gaeremynck added a comment - Attached patch.
        Hide
        Ian Boston added a comment -

        Happy to apply this and fix the import orders, but I am going to wait a few hours just in case anyone wants to shout.

        Show
        Ian Boston added a comment - Happy to apply this and fix the import orders, but I am going to wait a few hours just in case anyone wants to shout.
        Hide
        Alexander Klimetschek added a comment -

        Just to clarify: this patch does not set the Location header (AFAICS), but simply returns the possible URLs like /.1.json, /.2.json, etc. up until the maximum depth that returns a number of nodes below the configurable limit.

        But there seems to be a bug in the latest patch: allowedLevel should be decremented in the loop, otherwise it seems endless:

        + while (allowedLevel >= 0)

        { + writer.value(r.getPath() + "." + tidyUrl + allowedLevel + ".json"); + }
        Show
        Alexander Klimetschek added a comment - Just to clarify: this patch does not set the Location header (AFAICS), but simply returns the possible URLs like /.1.json, /.2.json, etc. up until the maximum depth that returns a number of nodes below the configurable limit. But there seems to be a bug in the latest patch: allowedLevel should be decremented in the loop, otherwise it seems endless: + while (allowedLevel >= 0) { + writer.value(r.getPath() + "." + tidyUrl + allowedLevel + ".json"); + }
        Hide
        Simon Gaeremynck added a comment -

        Yes, I removed the Location header as you said in your last mail in the thread.

        The bug must have crept in when I removed the response.setHeader line.
        Serves me right for not testing it again.

        Show
        Simon Gaeremynck added a comment - Yes, I removed the Location header as you said in your last mail in the thread. The bug must have crept in when I removed the response.setHeader line. Serves me right for not testing it again.
        Hide
        Simon Gaeremynck added a comment -

        Scrap the previous patch.
        This one fixes the bug + adds an integration test in the launchpad/testing bundle.

        Show
        Simon Gaeremynck added a comment - Scrap the previous patch. This one fixes the bug + adds an integration test in the launchpad/testing bundle.
        Hide
        Ian Boston added a comment -

        Patch applies ok and the integration tests passes.

        However, I have reverted the changes to the Sling API to eliminate the need to depend on a later version of the API.
        Also there was a license header missing, added in.

        Other than that LGTM,
        I will go and find the doc and update that as well.

        Show
        Ian Boston added a comment - Patch applies ok and the integration tests passes. However, I have reverted the changes to the Sling API to eliminate the need to depend on a later version of the API. Also there was a license header missing, added in. Other than that LGTM, I will go and find the doc and update that as well.
        Hide
        Ian Boston added a comment -

        At the moment, I cant see where to update the documentation, as I cant find any documentation on "infinity"

        Show
        Ian Boston added a comment - At the moment, I cant see where to update the documentation, as I cant find any documentation on "infinity"
        Hide
        Carsten Ziegeler added a comment -

        This has been released with 2.1.0

        Show
        Carsten Ziegeler added a comment - This has been released with 2.1.0

          People

          • Assignee:
            Ian Boston
            Reporter:
            Simon Gaeremynck
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development