suvallur / rest-assured

Automatically exported from code.google.com/p/rest-assured
0 stars 0 forks source link

Injection breach in JsonPath #328

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
Call JSONPath with a constructed String like this:
JsonPath.from(context.response.asString()).getDouble("content.find{entry -> 
entry.firstName == \"" + firstName + "\"}.lastName")
with firstName having the following value (including the quotes):
"};System.out.println("test");{x->"

What is the expected output? What do you see instead?
There is no person with such a name so I would expect an empty result or at 
least an exception. Instead I get "test" printed to the console because the 
code gets evaluated as Groovy code. This means that someone with control over 
the firstName String can execute arbitrary code. This is bad but you could 
argue that it's my problem if I insert firstName into my Json Path. Luckily 
there is a simple fix for this problem that would make it possible for me to do 
what I want (execute a parameterized query) and stay secure (see below).

What version of the product are you using?
Rest-assured 2.3.1

Please provide any additional information below.
A possible fix would be to allow users of JsonPath to add parameters to queries 
so the expression would look like this:
JsonPath
  .from(context.response.asString())
  .setParam("firstName", firstName)
  .getDouble("content.find{entry -> entry.firstName == firstName}.lastName")
This means one would have to implement a method 
com.jayway.restassured.path.json.JsonPath#setParam(java.lang.String, 
java.lang.Object) that puts the parameter into a params HashMap.
com.jayway.restassured.path.json.JsonPath#get(java.lang.String) would have to 
pass this params map to JsonAssertion, maybe via an optional constructor 
parameter. getAsJsonObject would have to construct the Groovy shell itself 
instead of calling Eval.me. This is simple since the code of eval.me is short:
        Binding b = new Binding();
        b.setVariable(symbol, object);
        GroovyShell sh = new GroovyShell(b);
        return sh.evaluate(expression);
Binding already has a constructor with a Map parameter so one could do the 
following:
        // Copy so we don't write into the input map
        Map allParams=new HashMap<String,Object>(params);
        // Add object to evaluate
        allParams.put(symbol, object);
        // Create shell with variables set
        GroovyShell sh = new GroovyShell(new Binding(allParams));
        // Run
        return sh.evaluate(expression);

As you can see this is quite simple to implement. Please tell me if you have 
too much work already and you need a patch. In that case I guess I can create 
one.

Original issue reported on code.google.com by cama...@gmail.com on 15 May 2014 at 8:50

GoogleCodeExporter commented 9 years ago
It would be great if you could provide a patch or pull request for this.

Original comment by johan.ha...@gmail.com on 15 May 2014 at 9:05

GoogleCodeExporter commented 9 years ago
Sure :-) . I wrote you a pull request for this issue. How often do you release?

Original comment by cama...@gmail.com on 16 May 2014 at 11:19

GoogleCodeExporter commented 9 years ago
Great! :) It would be awesome if you could provide the same thing for XmlPath

I'll release as soon as there are enough new things :) Probably within a couple 
of weeks since hopefully I have some time to spend some more time with REST 
Assured soon.

Original comment by johan.ha...@gmail.com on 16 May 2014 at 11:25

GoogleCodeExporter commented 9 years ago
Ok, I will do that.
Btw.: Did I just break your build or was it already broken?

Original comment by cama...@gmail.com on 16 May 2014 at 12:35

GoogleCodeExporter commented 9 years ago
I don't think you broke anything. The problem is that some SSL related tests 
only works from Sweden (don't ask :)). I really need to fix or exclude them 
from the build. 

Original comment by johan.ha...@gmail.com on 16 May 2014 at 2:01