Skip to content

CTF 4: CodeQL and Chill- Step 1.1 #132

Answered
80d ago
· 8 replies
Answered
Copy link

@Zast996 Zast996

Hi everyone, I am trying to resolve the step 1.1 of the challenge, reported below:

The sources of tainted data are the bean properties that go through constraint validation. In the code, these can be found as the first parameter of ConstraintValidator.isValid(...).
Write a CodeQL predicate that identifies these call arguments:
predicate isSource(DataFlow::Node source) { /
TODO describe source / }
To test your predicate, use the Quick Evaluation command (Right-click > CodeQL: Quick Evaluation). You should get 6 results.

I get the following solution:
code

but the result isn't the expected:
solution

I tried different solutions, but all of them provide the same result.
May someone please tell me where is the problem with my solution?

Replies

Answer selected by Zast996

Read carefully again:

The sources of tainted data are the bean properties that go through constraint validation. In the code, these can be found as the first parameter of ConstraintValidator.isValid(...).

If my hint is not helpful, I can give further hints or completely explain the problem in your solution.

@Zast996

The first parameter should be get (or not?) through the following statement in the where clause:
source.asExpr() = call.getArgument(0)

@intrigus-lgtm

There is a big difference between a parameter and an argument.
source.asParameter() = ...

@Zast996
80d

Zast996
Author

Thanks for the hint.
I tried with:
source.asParameter() = call.getMethod().getParameter(0)
The result is different but I only got 2 results when I was supposed to get 6.
Capture
In addition, i don't know why the parameter "Object value" isn't get as a result, although it is the first parameter of a "isValid" method inside a class which implements "ConstraintValidator<>"

@intrigus-lgtm

For the future, please post your current query as text :)
I suggest using Quick Evaluation on both parts of the and and also on the whole.

method.getName() = "isValid" and
method.getDeclaringType().getAnANcestor().getASupertype().getName() = " ConstraintValidator<>"

You're trying to find the overrides of a method, but I think that this will not work this way.
There is a overrides predicate that you can use.

After some time, you will probably come to the conclusion that overrides does not work for our use-case.
This is due to generics.
if so, have a look at github/codeql#3490.

And as always, the greatest tool in debugging is Quick Evaluation.

@Zast996

I did some tests using Quick Evaluation tool as you suggested.
I figured out that the problem was due to the method access call, so I tried with the following query:

predicate isSource(DataFlow::Node source) {
  exists( Method method |
    source.asParameter() = method.getParameter(0) and
    method.getName() = "isValid" and
    method.getDeclaringType().getAnAncestor().getASupertype().getName()
    = "ConstraintValidator<>" and
    )
}

This time the result seems almost the right one.
I got 8 results because there are 2 isValid method not overridden too, so I had a look to the link you shared.
I tried with a new query:

predicate isSource(DataFlow::Node source) {
  exists( Method method, Method overridden |
    source.asParameter() = method.getParameter(0) and
    method.getName() = "isValid" and
    method.getDeclaringType().getAnAncestor().getASupertype().getName()
    = "ConstraintValidator<>" and
    overridden.getSourceDeclaration() = method and
    method.overrides*(overridden) 
    )
}

However, the result isn't change (the same 8 results as before), but i think that the use of the overrides* method should be right.

@intrigus-lgtm

One big problem is this:

method.getName() = "isValid" and 
method.getDeclaringType().getAnAncestor().getASupertype().getName()
= "ConstraintValidator<>"

This will select all method that have the name isValid.
What you are trying to do here is selecting only methods that are derived from ConstraintValidator.
But consider these (pseudo-code-ish) classes:

class Foo implements Bar {
public void isValid(String clearly, String not, String a, String constraintValidatorMethod);
}

class Bar implements ConstraintValidator {}

Your query would flag the above isValid method because it is declared in Foo. And Bar is an ancestor of Foo, but also ConstraintValidator is an ancestor of Foo.

Instead I'd suggest using something like this:

isValidMethod.hasName("isValid") and
  isValidMethod.getDeclaringType().getName() = "ConstraintValidator"

this will get you all isValid method that are defined in ConstraintValidator.
This will give you two results. I don't know why there are two results, but it is probably related to Generics...
Then I'd use m.overridenOrInstantiates(isValidMethod) to find all methods m that override the isValid method from ConstraintValidator.

@Zast996

Finally I managed to resolve the Step.
I used your suggestion, but keeping the getAncestor method.

predicate isSource(DataFlow::Node source) {
  exists( Method method, Method m |
    source.asParameter() = m.getParameter(0) and
    method.getName() = "isValid" and
    method.getDeclaringType().getAnAncestor().getASupertype().getName()
    = "ConstraintValidator<>" and
    m.overridesOrInstantiates(method)
    )
}

I used the method m in the getParameter function instead of isValidMethod and used your override instruction.
The query gets 6 isValidMethod overridden.
Thanks a lot :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
Beta
You can’t perform that action at this time.