search

awslabs / ar-go-tools

ar-go-tools (Argot) is a collection of analysis tools for Go
Apache License 2.0
5 stars 1 forks source link

Support uncalled functions as analysis entrypoints #34

Closed samarth-aws closed 7 months ago

samarth-aws commented 8 months ago

Description of changes:

This PR fixes a soundness issue where a source function supplied as an actual parameter did not get tracked as an analysis entrypoint.

Before the fix:

❯ ./bin/taint -config ./testdata/src/taint/closures_paper/config.yaml ./testdata/src/taint/closures_paper/main.go ./testdata/src/taint/closures_paper/helpers.go
2023/10/09 17:24:19 Argot taint tool - build unknown
2023/10/09 17:24:19 Reading sources
[INFO]  Gathering global variable declaration in the program...
[INFO]  Computing information about types and functions for analysis...
[INFO]  Global gathering terminated, added 1 items (0.00 s)
[INFO]  Gathering values and starting pointer analysis...
[INFO]  Pointer analysis state computed, added 0 items (0.00 s)
[INFO]  Pointer analysis terminated (0.00 s)
[INFO]  Starting intra-procedural analysis ...
[INFO]  Intra-procedural pass done (0.00 s).
[INFO]  Starting inter-procedural pass...
[INFO]  --- # of analysis entrypoints: 4 ---
[INFO]  
****************************** NEW SOURCE ******************************
[INFO]  ==> Source: "[#7.2] (SA)call: source() in example1$1"
[INFO]  Found at /Users/samarkis/workplace/argot/testdata/src/taint/closures_paper/main.go:27:39
[INFO]  
****************************** NEW SOURCE ******************************
[INFO]  ==> Source: "[#7.2] (SA)call: source() in example1$1"
[INFO]  Found at /Users/samarkis/workplace/argot/testdata/src/taint/closures_paper/main.go:27:39
[INFO]  
****************************** NEW SOURCE ******************************
[INFO]  ==> Source: "[#7.2] (SA)call: source() in example1$1"
[INFO]  Found at /Users/samarkis/workplace/argot/testdata/src/taint/closures_paper/main.go:27:39
[INFO]  
****************************** NEW SOURCE ******************************
[INFO]  ==> Source: "[#7.2] (SA)call: source() in example1$1"
[INFO]  Found at /Users/samarkis/workplace/argot/testdata/src/taint/closures_paper/main.go:27:39
[INFO]  inter-procedural pass done (0.00 s).
[INFO]  
[INFO]  -********************************************************************************
[INFO]  Analysis took 0.0008 s
[INFO]  
[INFO]  RESULT:
        No taint flows detected ✓

Notice that there are only 4 entrypoints: the source function in example2 was undetected leading to a soundness issue.

After the fix:

❯ ./bin/taint -config ./testdata/src/taint/closures_paper/config.yaml ./testdata/src/taint/closures_paper/main.go ./testdata/src/taint/closures_paper/helpers.go
2023/10/09 17:25:51 Argot taint tool - build unknown
2023/10/09 17:25:51 Reading sources
[INFO]  Gathering global variable declaration in the program...
[INFO]  Global gathering terminated, added 1 items (0.00 s)
[INFO]  Computing information about types and functions for analysis...
[INFO]  Gathering values and starting pointer analysis...
[INFO]  Pointer analysis state computed, added 0 items (0.00 s)
[INFO]  Pointer analysis terminated (0.00 s)
[INFO]  Starting intra-procedural analysis ...
[INFO]  Intra-procedural pass done (0.00 s).
[INFO]  Starting inter-procedural pass...
[INFO]  --- # of analysis entrypoints: 5 ---
[INFO]  
****************************** NEW SOURCE ******************************
[INFO]  ==> Source: "[#7.1] (SA)call: process(source, sink) in example2"
[INFO]  Found at /Users/samarkis/workplace/argot/testdata/src/taint/closures_paper/main.go:35:9
[INFO]   💀 Sink reached at /Users/samarkis/workplace/argot/testdata/src/taint/closures_paper/main.go:21:8
[INFO]   Add new path from "[#7.1] (SA)call: process(source, sink) in example2" to "[#1.8] @arg 0:t3 in [#1.7] (CG)call: handle(t3...) in process " <== 
[INFO]  
****************************** NEW SOURCE ******************************
[INFO]  ==> Source: "[#2.2] (SA)call: source() in example1$1"
[INFO]  Found at /Users/samarkis/workplace/argot/testdata/src/taint/closures_paper/main.go:27:39
[INFO]   💀 Sink reached at /Users/samarkis/workplace/argot/testdata/src/taint/closures_paper/main.go:21:8
[INFO]   Add new path from "[#2.2] (SA)call: source() in example1$1" to "[#1.8] @arg 0:t3 in [#1.7] (CG)call: handle(t3...) in process " <== 
[INFO]  
****************************** NEW SOURCE ******************************
[INFO]  ==> Source: "[#2.2] (SA)call: source() in example1$1"
[INFO]  Found at /Users/samarkis/workplace/argot/testdata/src/taint/closures_paper/main.go:27:39
[INFO]   💀 Sink reached at /Users/samarkis/workplace/argot/testdata/src/taint/closures_paper/main.go:21:8
[INFO]   Add new path from "[#2.2] (SA)call: source() in example1$1" to "[#1.8] @arg 0:t3 in [#1.7] (CG)call: handle(t3...) in process " <== 
[INFO]  
****************************** NEW SOURCE ******************************
[INFO]  ==> Source: "[#2.2] (SA)call: source() in example1$1"
[INFO]  Found at /Users/samarkis/workplace/argot/testdata/src/taint/closures_paper/main.go:27:39
[INFO]   💀 Sink reached at /Users/samarkis/workplace/argot/testdata/src/taint/closures_paper/main.go:21:8
[INFO]   Add new path from "[#2.2] (SA)call: source() in example1$1" to "[#1.8] @arg 0:t3 in [#1.7] (CG)call: handle(t3...) in process " <== 
[INFO]  
****************************** NEW SOURCE ******************************
[INFO]  ==> Source: "[#2.2] (SA)call: source() in example1$1"
[INFO]  Found at /Users/samarkis/workplace/argot/testdata/src/taint/closures_paper/main.go:27:39
[INFO]   💀 Sink reached at /Users/samarkis/workplace/argot/testdata/src/taint/closures_paper/main.go:21:8
[INFO]   Add new path from "[#2.2] (SA)call: source() in example1$1" to "[#1.8] @arg 0:t3 in [#1.7] (CG)call: handle(t3...) in process " <== 
[INFO]  inter-procedural pass done (0.00 s).
[INFO]  
[INFO]  -********************************************************************************
[INFO]  Analysis took 0.0010 s
[INFO]  
[ERROR] RESULT:
        Taint flows detected!
[WARN]  A source has reached a sink in function process:
    Sink: [SSA] handle(t3...)
        /Users/samarkis/workplace/argot/testdata/src/taint/closures_paper/main.go:21:8
    Source: [SSA] source()
        /Users/samarkis/workplace/argot/testdata/src/taint/closures_paper/main.go:27:39
[WARN]  A source has reached a sink in function process:
    Sink: [SSA] handle(t3...)
        /Users/samarkis/workplace/argot/testdata/src/taint/closures_paper/main.go:21:8
    Source: [SSA] process(source, sink)
        /Users/samarkis/workplace/argot/testdata/src/taint/closures_paper/main.go:35:9

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

samarth-aws commented 8 months ago

Good catch! However I don't think this is the solution: for function calls, we need to track the return value of the function call. This solution changes how the sources are identified to match function values. I would prefer a solution were function calls are accurately resolved so that matching sources doesn't miss calls to source functions because they have a different name when they're called.

Agreed, that would be the better approach. For example, this PR doesn't handle the case when a source function is not an argument:

func example3() {
    handler := sink
    str := "hello"
    s := func() func() string {
        str += " world"
        return source
    }
    process(s(), handler)
    process(func() string { return "ok" }, handler)
    handler(str)
}
victornicolet commented 7 months ago

I think we need to bump go versions to 1.20.12 and 1.21.5