0%

GitHub Security Lab CTF: CodeQL and Chill

发表于 更新于 阅读次数： Valine：

这是GitHub Security Lab CTF的一道题目，利用CodeQL挖掘Netflix Titus服务端模板注入漏洞。

漏洞详情

JSR是Java Specification Requests的缩写，意思是Java 规范提案。JSR380是关于数据校验这块的，也就是JSR第380号标准。Netflix Titus使用Java Bean Validation (JSR 380) 规范自定义了约束验证器，如com.netflix.titus.api.jobmanager.model.job.sanitizer.SchedulingConstraintSetValidator

SchedulingConstraintSetValidator.java

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
public class SchedulingConstraintSetValidator implements ConstraintValidator<SchedulingConstraintSetValidator.SchedulingConstraintSet, Container> {
		...
    @Override
    public boolean isValid(Container container, ConstraintValidatorContext context) {
        if (container == null) {
            return true;
        }
        Set<String> common = new HashSet<>(container.getSoftConstraints().keySet());
        common.retainAll(container.getHardConstraints().keySet());

        if (common.isEmpty()) {
            return true;
        }

        context.buildConstraintViolationWithTemplate(
                "Soft and hard constraints not unique. Shared constraints: " + common
        ).addConstraintViolation().disableDefaultConstraintViolation();
        return false;
    }
}

通过Bean Validation 2.0 规范可知在构建违反约束的错误信息时，可以插入多种类型的值，包括Java EL 表达式。因此如果ConstraintValidatorContext.buildConstraintViolationWithTemplate()的第一个参数即错误信息模板被攻击者可控，就有可能导致任意代码执行，即CVE-2020-9297。这些错误信息模板就是注入漏洞的sink。经过验证的bean属性通常会流入自定义错误信息，这些就是source。

数据流和污点跟踪分析

首先下载存在漏洞的版本8a8bd4c对应的CodeQL数据库。

Source

题目已经提示source是上文代码中isValid方法的第一个参数，我们要找的isValid方法其实是对ConstraintValidator接口的具体实现，首先抽象出该接口：

1
2
3
4
5
class ConstraintValidator extends Interface{
    ConstraintValidator(){
        this.hasQualifiedName("javax.validation", "ConstraintValidator")
    }
}

抽象出isValid方法：

1
2
3
4
5
6
7
8
9
10
11
12
class AbstractIsValid extends Method{
    AbstractIsValid(){
        this.getDeclaringType() instanceof ConstraintValidator and
        this.hasName("isValid")
    }
}

class IsValid extends Method{
    IsValid(){
        exists(AbstractIsValid abstractIsValid | this.overridesOrInstantiates*(abstractIsValid) )
    }
}

确定source：

1
2
3
predicate isSource(DataFlow::Node source) { 
    exists(IsValid isValid | source.asParameter() = isValid.getParameter(0))
}

Sink

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
class ConstraintValidatorContext extends RefType {
    ConstraintValidatorContext(){
        this.hasQualifiedName("javax.validation","ConstraintValidatorContext")
    }
}

class BuildConstraintViolationWithTemplate extends Method {
    BuildConstraintViolationWithTemplate(){
        this.getDeclaringType().getASupertype*() instanceof  ConstraintValidatorContext and
        this.hasName("buildConstraintViolationWithTemplate")
    }
}

predicate isSink(DataFlow::Node sink) { 
    exists(MethodAccess methodAccess | 
        methodAccess.getCallee() instanceof  BuildConstraintViolationWithTemplate and
        methodAccess.getArgument(0) = sink.asExpr()
    )
}

污点跟踪

完整代码：

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
import java
import semmle.code.java.dataflow.DataFlow
import semmle.code.java.dataflow.TaintTracking
import DataFlow::PathGraph

class ConstraintValidator extends Interface{
    ConstraintValidator(){
        this.hasQualifiedName("javax.validation", "ConstraintValidator")
    }
}

class AbstractIsValid extends Method{
    AbstractIsValid(){
        this.getDeclaringType() instanceof ConstraintValidator and
        this.hasName("isValid")
    }
}

class IsValid extends Method{
    IsValid(){
        exists(AbstractIsValid abstractIsValid | this.overridesOrInstantiates*(abstractIsValid) )
    }
}


class ConstraintValidatorContext extends RefType {
    ConstraintValidatorContext(){
        this.hasQualifiedName("javax.validation","ConstraintValidatorContext")
    }
}

class BuildConstraintViolationWithTemplate extends Method {
    BuildConstraintViolationWithTemplate(){
        this.getDeclaringType().getASupertype*() instanceof  ConstraintValidatorContext and
        this.hasName("buildConstraintViolationWithTemplate")
    }
}

class MyTaintTrackingConfig extends TaintTracking::Configuration {
    MyTaintTrackingConfig() { this = "MyTaintTrackingConfig" }

    override predicate isSource(DataFlow::Node source) {
        exists(IsValid isValid | source.asParameter() = isValid.getParameter(0))
    }

    override predicate isSink(DataFlow::Node sink) {
        exists(MethodAccess methodAccess | 
            methodAccess.getCallee() instanceof  BuildConstraintViolationWithTemplate and
            methodAccess.getArgument(0) = sink.asExpr()
        )
    }
}

from MyTaintTrackingConfig cfg, DataFlow::PathNode source, DataFlow::PathNode sink
where cfg.hasFlowPath(source, sink)
select sink, source, sink, "Custom constraint error message contains unsanitized user data"

查询结果为0。

问题分析

目前source和sink点都已经明确，结果为0则说明从source到sink的路径上缺少了一步。CodeQL提供了partial data flow来进行Debug，这个功能可以查找从给定的source到任何可能的sink的流，让sink不受限制，同时限制从source到sink的搜索步骤的数量。因此可以使用这个功能来跟踪污点从source到所有可能的sink的流向，并查看流在哪一步不再被进一步跟踪。

predicate hasPartialFlow(PartialPathNode source, PartialPathNode node, int dist)

如果存在从source到node的部分数据流路径，则成立。node与最近的source之间的近似距离是dist并且被限制为小于或等于explorationLimit()。该谓词完全无视sink的定义。

此谓词旨在用于数据流探索和调试，如果sourece数量太多或者explorationLimit设置得太高则可能会表现不佳。

默认情况下禁用此谓词（没有结果）。用合适的数字覆盖 explorationLimit()以启用此谓词。

要在path-problem查询中使用它，请导入模块PartialPathGraph。

使用模板如下：

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
/** @kind path-problem */
import java
import semmle.code.java.dataflow.TaintTracking
import DataFlow::PartialPathGraph // this is different!

class MyTaintTrackingConfig extends TaintTracking::Configuration {
    MyTaintTrackingConfig() { ... } // same as before
    override predicate isSource(DataFlow::Node source) { ... } // same as before
    override predicate isSink(DataFlow::Node sink) { ... } // same as before
    override int explorationLimit() { result =  10} // this is different!
}
from MyTaintTrackingConfig cfg, DataFlow::PartialPathNode source, DataFlow::PartialPathNode sink
where
  cfg.hasPartialFlow(source, sink, _) and
  source.getNode() = ... // TODO restrict to the one source we are interested in, for ease of debugging
select sink, source, sink, "Partial flow from unsanitized user data"

predicate partial_flow(PartialPathNode n, Node src, int dist) {
  exists(MyTaintTrackingConfig conf, PartialPathNode source |
    conf.hasPartialFlow(source, n, dist) and
    src = source.getNode() and
    source =  // TODO - restrict to THE source we are interested in
  )
}

完整代码（注意DataFlow::PathGraph和DataFlow::PartialPathGraph不能同时导入）：

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
import java
import semmle.code.java.dataflow.DataFlow
import semmle.code.java.dataflow.TaintTracking
//import DataFlow::PathGraph
import DataFlow::PartialPathGraph

class ConstraintValidator extends Interface{
    ConstraintValidator(){
        this.hasQualifiedName("javax.validation", "ConstraintValidator")
    }
}

class AbstractIsValid extends Method{
    AbstractIsValid(){
        this.getDeclaringType() instanceof ConstraintValidator and
        this.hasName("isValid")
    }
}

class IsValid extends Method{
    IsValid(){
        exists(AbstractIsValid abstractIsValid | this.overridesOrInstantiates*(abstractIsValid) )
    }
}


class ConstraintValidatorContext extends RefType {
    ConstraintValidatorContext(){
        this.hasQualifiedName("javax.validation","ConstraintValidatorContext")
    }
}

class BuildConstraintViolationWithTemplate extends Method {
    BuildConstraintViolationWithTemplate(){
        this.getDeclaringType().getASupertype*() instanceof  ConstraintValidatorContext and
        this.hasName("buildConstraintViolationWithTemplate")
    }
}

class MyTaintTrackingConfig extends TaintTracking::Configuration {
    MyTaintTrackingConfig() { this = "MyTaintTrackingConfig" }

    override predicate isSource(DataFlow::Node source) {
        exists(IsValid isValid | source.asParameter() = isValid.getParameter(0))
    }

    override predicate isSink(DataFlow::Node sink) {
        exists(MethodAccess methodAccess | 
            methodAccess.getCallee() instanceof  BuildConstraintViolationWithTemplate and
            methodAccess.getArgument(0) = sink.asExpr()
        )
    }

    override int explorationLimit() { result =  3}
}


from MyTaintTrackingConfig cfg, DataFlow::PartialPathNode source, DataFlow::PartialPathNode sink
where
    cfg.hasPartialFlow(source, sink, _) and
    source.getNode().asParameter().getName() = "container"
select sink, source, sink, "Partial flow from unsanitized user data"

predicate partial_flow(DataFlow::PartialPathNode n, DataFlow::Node src, int dist) {
    exists(MyTaintTrackingConfig conf, DataFlow::PartialPathNode source |
        conf.hasPartialFlow(source, n, dist) and
        src = source.getNode() and
        source.getNode().asParameter().getName() = "container"
    )
}

测试发现污点确实是到了预期的地方的

image-20220515222307815

这与题目中CodeQL不会通过getter传播污染的描述不符，猜测可能是我当前用的最新版引擎（v2.9.1）已经支持这类方法的污点自动传播了。

You must have found that CodeQL does not propagate taint through getters like container.getHardConstraints and container.getSoftConstraints. Can you guess why this default behaviour was implemented?

但如上所述，最新版CodeQL理应直接得到查询结果才对。修改一下污点跟踪处的查询：

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
class MyTaintTrackingConfig extends TaintTracking::Configuration {
    MyTaintTrackingConfig() { this = "MyTaintTrackingConfig" }

    override predicate isSource(DataFlow::Node source) {
        exists(IsValid isValid | source.asParameter() = isValid.getParameter(0)) 
    }

    override predicate isSink(DataFlow::Node sink) {
        exists(MethodAccess methodAccess | 
            //methodAccess.getArgument(0) = sink.asExpr() and
            methodAccess.getCallee() instanceof  BuildConstraintViolationWithTemplate and
            sink.getLocation().getFile().getBaseName() = "SchedulingConstraintSetValidator.java"
        )
        
    }
}

from MyTaintTrackingConfig cfg, DataFlow::Node source, DataFlow::Node sink,MethodAccess methodAccess
where cfg.hasFlow(source, sink) and
    methodAccess.getArgument(0) = sink.asExpr() and
    methodAccess.getCallee() instanceof  BuildConstraintViolationWithTemplate and
    sink.getLocation().getFile().getBaseName() = "SchedulingConstraintSetValidator.java"
select sink, source, sink, "Custom constraint error message contains unsanitized user data"

如果取消上面代码//methodAccess.getArgument(0) = sink.asExpr() and的注释，那么在isSink中和最后的查询条件中，对sink的筛选条件是一致的，所以如果上面的查询能查出结果，那么将那一行取消注释之后，也应该能查出结果，但是实际测试取消注释后是0个结果，感觉这里可能有什么bug，已经提了Issue。

image-20220516214726813

image-20220516214817080

为了顺着原题目的思路继续学习，这里暂时把CodeQL引擎版本换成了v2.2.3，库换成了v1.25.0。

重新执行上方Debug的查询，发现不会通过getters传播污点，比如 container.getHardConstraints 和 container.getSoftConstraints。

image-20220518222225797

添加额外的污点步骤

可以通过继承类 TaintTracking::AdditionalTaintStep 并实现 step 谓词。 当受污染的数据从 node1 流向 node2.时， step 谓词应该成立。添加如下查询以让污点在getters传播。

1
2
3
4
5
6
7
8
9
class GetterTaintStep extends TaintTracking::AdditionalTaintStep {
    override predicate step(DataFlow::Node node1, DataFlow::Node node2) {
        exists(MethodAccess ma |
            ma.getQualifier() = node1.asExpr() and
            ma = node2.asExpr() and
            ma.getMethod() instanceof GetterMethod
        )
    }
}

Expr getQualifier() : 获取此方法访问的限定表达式（如果有）。

image-20220518225203065

在keySet中断了，接着添加：

1
2
3
4
5
6
7
8
9
class KeySetTaintStep extends TaintTracking::AdditionalTaintStep {
    override predicate step(DataFlow::Node node1, DataFlow::Node node2) {
        exists(MethodAccess ma |
            ma.getQualifier() = node1.asExpr() and
            ma = node2.asExpr() and
            ma.getMethod().getName() = "keySet"
        )
    }
}

image-20220518225526091

在new HashSet<>中断了，添加：

1
2
3
4
5
6
7
8
9
class HashSetStep extends TaintTracking::AdditionalTaintStep {
    override predicate step(DataFlow::Node node1, DataFlow::Node node2) {
        exists(ConstructorCall cc |
            cc.getAnArgument() = node1.asExpr() and
            cc = node2.asExpr() and
            cc.getConstructor().getSourceDeclaration().getName() = "HashSet"
        )
    }
}

image-20220518230306550

已经流到预期的sink点了

最终查询

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
import java
import semmle.code.java.dataflow.DataFlow
import semmle.code.java.dataflow.TaintTracking
import DataFlow::PathGraph

class ConstraintValidator extends Interface{
    ConstraintValidator(){
        this.hasQualifiedName("javax.validation", "ConstraintValidator")
    }
}

class AbstractIsValid extends Method{
    AbstractIsValid(){
        this.getDeclaringType() instanceof ConstraintValidator and
        this.hasName("isValid")
    }
}

class IsValid extends Method{
    IsValid(){
        exists(AbstractIsValid abstractIsValid | this.overridesOrInstantiates*(abstractIsValid) )
    }
}


class ConstraintValidatorContext extends RefType {
    ConstraintValidatorContext(){
        this.hasQualifiedName("javax.validation","ConstraintValidatorContext")
    }
}

class BuildConstraintViolationWithTemplate extends Method {
    BuildConstraintViolationWithTemplate(){
        this.getDeclaringType().getASupertype*() instanceof  ConstraintValidatorContext and
        this.hasName("buildConstraintViolationWithTemplate")
    }
}

class MyTaintTrackingConfig extends TaintTracking::Configuration {
    MyTaintTrackingConfig() { this = "MyTaintTrackingConfig" }

    override predicate isSource(DataFlow::Node source) {
        exists(IsValid isValid | source.asParameter() = isValid.getParameter(0))
    }

    override predicate isSink(DataFlow::Node sink) {
        exists(MethodAccess methodAccess | 
            methodAccess.getCallee() instanceof  BuildConstraintViolationWithTemplate and
            methodAccess.getArgument(0) = sink.asExpr()
        )
    }
}

class GetterTaintStep extends TaintTracking::AdditionalTaintStep {
    override predicate step(DataFlow::Node node1, DataFlow::Node node2) {
        exists(MethodAccess ma |
            ma.getQualifier() = node1.asExpr() and
            ma = node2.asExpr() and
            ma.getMethod() instanceof GetterMethod
        )
    }
}

class KeySetTaintStep extends TaintTracking::AdditionalTaintStep {
    override predicate step(DataFlow::Node node1, DataFlow::Node node2) {
        exists(MethodAccess ma |
            ma.getQualifier() = node1.asExpr() and
            ma = node2.asExpr() and
            ma.getMethod().getName() = "keySet"
        )
    }
}

class HashSetStep extends TaintTracking::AdditionalTaintStep {
    override predicate step(DataFlow::Node node1, DataFlow::Node node2) {
        exists(ConstructorCall cc |
            cc.getAnArgument() = node1.asExpr() and
            cc = node2.asExpr() and
            cc.getConstructor().getSourceDeclaration().getName() = "HashSet"
        )
    }
}

from MyTaintTrackingConfig cfg, DataFlow::PathNode source, DataFlow::PathNode sink
where cfg.hasFlowPath(source, sink)
select sink, source, sink, "Custom constraint error message contains unsanitized user data"

image-20220518230617304

之前问题解决

之前提的Issue有了答复。

而且之前的数据流其实没流向预期的地方，是有中断的。如下查询：

codeql-cli 及库版本：

image-20220521115653697

Java代码：

1
2
3
4
5
6
7
8
9
10
11
12
13
import java.util.Arrays;
import java.util.HashSet;
import java.util.Set;

public class Demo {
    public static void main(String[] args) throws Exception {
        test("test");
    }
    public static void test(String source){
        Set<String> common = new HashSet<>(Arrays.asList(source));
        System.out.println("" + common);
    }
}

将test方法的参数作为source，标准输出的内容作为sink的话，可以看到实际运行结果是source可以流向sink。

image-20220521125052495

CodeQL查询：

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
/**
 * @kind path-problem
 */
import java
import semmle.code.java.dataflow.DataFlow
import semmle.code.java.dataflow.TaintTracking
//import DataFlow::PathGraph
import DataFlow::PartialPathGraph


class MyTaintTrackingConfig extends TaintTracking::Configuration {
    MyTaintTrackingConfig() { this = "MyTaintTrackingConfig" }

    override predicate isSource(DataFlow::Node source) {
        exists(Method m | 
            m.hasQualifiedName("", "Demo", "test") and
            m.getAParameter() = source.asParameter()
        )
    }

    override predicate isSink(DataFlow::Node sink) {
        exists(MethodAccess ma | 
            ma.getCallee().getDeclaringType().hasQualifiedName("java.io", "PrintStream") and
            sink.asExpr() = ma.getAnArgument()
        )
    }

    override int explorationLimit() { result = 10}
}
  
from MyTaintTrackingConfig cfg, DataFlow::PartialPathNode source, DataFlow::PartialPathNode sink
where cfg.hasPartialFlow(source, sink, _)
select sink, source, sink, "Partial flow from unsanitized user data"

image-20220521125744518

查询结果是污点只流到了common [<element>]，"" + common对于common中的污点数据来说是一种隐式读取，所以并没有让污点继续传播，也就没有流到预期的sink点即System.out.println的参数。

解决方法是在字符串连接处引入这些隐式读取，即在污点跟踪配置中覆盖以下方法：

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
    pred.asExpr() = succ.asExpr().(AddExpr).getAnOperand()
}

override predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet c) {
    super.allowImplicitRead(node, c)
    or
    this.isAdditionalTaintStep(node, _) and
    (
        c instanceof DataFlow::ArrayContent
        or
        c instanceof DataFlow::CollectionContent
        or
        c instanceof DataFlow::MapValueContent
    )
}

最终查询：

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
/**
 * @kind path-problem
 */
import java
import semmle.code.java.dataflow.DataFlow
import semmle.code.java.dataflow.TaintTracking
import DataFlow::PathGraph
//import DataFlow::PartialPathGraph

class MyTaintTrackingConfig extends TaintTracking::Configuration {
    MyTaintTrackingConfig() { this = "MyTaintTrackingConfig" }

    override predicate isSource(DataFlow::Node source) {
        exists(Method m | 
            m.hasQualifiedName("", "Demo", "test") and
            m.getAParameter() = source.asParameter()
        )
    }

    override predicate isSink(DataFlow::Node sink) {
        exists(MethodAccess ma | 
            ma.getCallee().getDeclaringType().hasQualifiedName("java.io", "PrintStream") and
            sink.asExpr() = ma.getAnArgument()
        )
    }

    override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
        pred.asExpr() = succ.asExpr().(AddExpr).getAnOperand()
    }
    
    override predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet c) {
        super.allowImplicitRead(node, c)
        or
        this.isAdditionalTaintStep(node, _) and
        (
            c instanceof DataFlow::ArrayContent
            or
            c instanceof DataFlow::CollectionContent
            or
            c instanceof DataFlow::MapValueContent
        )
    }
}
  
from MyTaintTrackingConfig cfg, DataFlow::PathNode source, DataFlow::PathNode sink
where cfg.hasFlowPath(source, sink)
select sink, source, sink, "Partial flow from unsanitized user data"

image-20220521131153845

参考文档

GitHub Security Lab CTF 4: CodeQL and Chill - The Java Edition

Answers & Feedback - GitHub Security Lab CTF 4: CodeQL and chill - The Java edition

GitHub Java CodeQL CTF

使用 CodeQL 挖掘 CVE-2020-9297