Skip to content

Commit faa5554

Browse files
dilanbhalladilanbhalla
authored
Merge pull request #153 from microsoft/revert-152-dilan/rust-exclude-broken-queries
Revert "Remove Broken Rust Queries" + Stub DataflowStack required Interface
2 parents aec5d89 + 2d97d0f commit faa5554

File tree

9 files changed

+201
-26
lines changed

9 files changed

+201
-26
lines changed

‎csharp/ql/lib/semmle/code/csharp/security/dataflow/ZipSlipQuery.qll

Lines changed: 6 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
import csharp
66
private import semmle.code.csharp.controlflow.Guards
77
private import semmle.code.csharp.security.dataflow.flowsinks.FlowSinks
8-
private import semmle.code.csharp.dataflow.DataFlow::DataFlow::PathGraph
98

109
abstract private class AbstractSanitizerMethod extends Method { }
1110

@@ -453,26 +452,14 @@ private module ZipSlipConfig implements DataFlow::ConfigSig {
453452
predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
454453

455454
predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
456-
}
457-
458-
/**
459-
* A taint tracking module for Zip Slip.
460-
*/
461-
module ZipSlip = TaintTracking::Global<ZipSlipConfig>;
462-
463-
deprecated class TaintTrackingConfiguration extends TaintTracking::Configuration {
464-
TaintTrackingConfiguration() { this = "ZipSlipTaintTrackingConfiguration" }
465455

466-
override predicate isSource(DataFlow::Node node) { node instanceof Source }
467-
468-
override predicate isSink(DataFlow::Node node) { node instanceof Sink }
469-
470-
override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
471-
super.isAdditionalTaintStep(pred, succ)
472-
or
456+
predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) {
473457
// If the sink is a method call, and the source is an argument to that method call
474458
exists(MethodCall mc | succ.asExpr() = mc and pred.asExpr() = mc.getAnArgument())
475459
}
460+
}
476461

477-
override predicate isSanitizer(DataFlow::Node node) { node instanceof Sanitizer }
478-
}
462+
/**
463+
* A taint tracking module for Zip Slip.
464+
*/
465+
module ZipSlip = TaintTracking::Global<ZipSlipConfig>;

‎csharp/ql/test/query-tests/Security Features/CWE-022/ZipSlip/ZipSlip.expected

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,30 +11,45 @@
1111
| ZipSlipBad.cs:9:59:9:72 | access to property FullName | ZipSlipBad.cs:9:59:9:72 | access to property FullName : String | ZipSlipBad.cs:10:29:10:40 | access to local variable destFileName | Unsanitized archive entry, which may contain '..', is used in a $@. | ZipSlipBad.cs:10:29:10:40 | access to local variable destFileName | file system operation |
1212
edges
1313
| ZipSlip.cs:15:24:15:40 | access to local variable fullPath_relative : String | ZipSlip.cs:30:71:30:87 | access to local variable fullPath_relative : String | provenance | |
14-
| ZipSlip.cs:15:24:15:40 | access to local variable fullPath_relative : String | ZipSlip.cs:35:28:35:56 | access to local variable destFilePath_notCanonicalized : String | provenance | |
1514
| ZipSlip.cs:15:44:15:75 | call to method GetFullPath : String | ZipSlip.cs:15:24:15:40 | access to local variable fullPath_relative : String | provenance | |
15+
| ZipSlip.cs:15:61:15:74 | access to property FullName : String | ZipSlip.cs:15:44:15:75 | call to method GetFullPath : String | provenance | Config |
1616
| ZipSlip.cs:15:61:15:74 | access to property FullName : String | ZipSlip.cs:15:44:15:75 | call to method GetFullPath : String | provenance | MaD:2 |
17-
| ZipSlip.cs:18:24:18:49 | access to local variable file_badDirectoryTraversal : String | ZipSlip.cs:22:71:22:96 | access to local variable file_badDirectoryTraversal : String | provenance | |
17+
| ZipSlip.cs:18:24:18:49 | access to local variable file_badDirectoryTraversal : String | ZipSlip.cs:19:43:19:68 | access to local variable file_badDirectoryTraversal : String | provenance | |
1818
| ZipSlip.cs:18:53:18:66 | access to property FullName : String | ZipSlip.cs:18:24:18:49 | access to local variable file_badDirectoryTraversal : String | provenance | |
19+
| ZipSlip.cs:19:43:19:68 | access to local variable file_badDirectoryTraversal : String | ZipSlip.cs:22:71:22:96 | access to local variable file_badDirectoryTraversal : String | provenance | |
1920
| ZipSlip.cs:22:28:22:39 | access to local variable destFileName : String | ZipSlip.cs:23:41:23:52 | access to local variable destFileName | provenance | |
2021
| ZipSlip.cs:22:43:22:97 | call to method Combine : String | ZipSlip.cs:22:28:22:39 | access to local variable destFileName : String | provenance | |
22+
| ZipSlip.cs:22:71:22:96 | access to local variable file_badDirectoryTraversal : String | ZipSlip.cs:22:43:22:97 | call to method Combine : String | provenance | Config |
2123
| ZipSlip.cs:22:71:22:96 | access to local variable file_badDirectoryTraversal : String | ZipSlip.cs:22:43:22:97 | call to method Combine : String | provenance | MaD:1 |
2224
| ZipSlip.cs:30:28:30:39 | access to local variable destFilePath : String | ZipSlip.cs:31:41:31:52 | access to local variable destFilePath | provenance | |
2325
| ZipSlip.cs:30:43:30:88 | call to method Combine : String | ZipSlip.cs:30:28:30:39 | access to local variable destFilePath : String | provenance | |
26+
| ZipSlip.cs:30:71:30:87 | access to local variable fullPath_relative : String | ZipSlip.cs:30:43:30:88 | call to method Combine : String | provenance | Config |
2427
| ZipSlip.cs:30:71:30:87 | access to local variable fullPath_relative : String | ZipSlip.cs:30:43:30:88 | call to method Combine : String | provenance | MaD:1 |
28+
| ZipSlip.cs:30:71:30:87 | access to local variable fullPath_relative : String | ZipSlip.cs:35:28:35:56 | access to local variable destFilePath_notCanonicalized : String | provenance | |
2529
| ZipSlip.cs:35:28:35:56 | access to local variable destFilePath_notCanonicalized : String | ZipSlip.cs:39:45:39:73 | access to local variable destFilePath_notCanonicalized | provenance | |
2630
| ZipSlip.cs:58:20:58:27 | access to local variable fullpath : String | ZipSlip.cs:62:33:62:40 | access to local variable fullpath | provenance | |
27-
| ZipSlip.cs:58:20:58:27 | access to local variable fullpath : String | ZipSlip.cs:71:37:71:44 | access to local variable fullpath | provenance | |
31+
| ZipSlip.cs:58:20:58:27 | access to local variable fullpath : String | ZipSlip.cs:62:33:62:40 | access to local variable fullpath : String | provenance | |
2832
| ZipSlip.cs:58:31:58:75 | call to method Combine : String | ZipSlip.cs:58:20:58:27 | access to local variable fullpath : String | provenance | |
33+
| ZipSlip.cs:58:61:58:74 | access to property FullName : String | ZipSlip.cs:58:31:58:75 | call to method Combine : String | provenance | Config |
2934
| ZipSlip.cs:58:61:58:74 | access to property FullName : String | ZipSlip.cs:58:31:58:75 | call to method Combine : String | provenance | MaD:1 |
30-
| ZipSlip.cs:105:32:105:43 | access to local variable destFilePath : String | ZipSlip.cs:112:74:112:85 | access to local variable destFilePath | provenance | |
31-
| ZipSlip.cs:105:32:105:43 | access to local variable destFilePath : String | ZipSlip.cs:119:71:119:82 | access to local variable destFilePath | provenance | |
32-
| ZipSlip.cs:105:32:105:43 | access to local variable destFilePath : String | ZipSlip.cs:126:57:126:68 | access to local variable destFilePath | provenance | |
33-
| ZipSlip.cs:105:32:105:43 | access to local variable destFilePath : String | ZipSlip.cs:134:58:134:69 | access to local variable destFilePath | provenance | |
35+
| ZipSlip.cs:62:33:62:40 | access to local variable fullpath : String | ZipSlip.cs:64:29:64:36 | access to local variable fullpath : String | provenance | |
36+
| ZipSlip.cs:64:29:64:36 | access to local variable fullpath : String | ZipSlip.cs:69:30:69:37 | access to local variable fullpath : String | provenance | |
37+
| ZipSlip.cs:69:30:69:37 | access to local variable fullpath : String | ZipSlip.cs:71:37:71:44 | access to local variable fullpath | provenance | |
38+
| ZipSlip.cs:105:32:105:43 | access to local variable destFilePath : String | ZipSlip.cs:107:73:107:84 | access to local variable destFilePath : String | provenance | |
3439
| ZipSlip.cs:105:47:105:86 | call to method Combine : String | ZipSlip.cs:105:32:105:43 | access to local variable destFilePath : String | provenance | |
40+
| ZipSlip.cs:105:72:105:85 | access to property FullName : String | ZipSlip.cs:105:47:105:86 | call to method Combine : String | provenance | Config |
3541
| ZipSlip.cs:105:72:105:85 | access to property FullName : String | ZipSlip.cs:105:47:105:86 | call to method Combine : String | provenance | MaD:1 |
42+
| ZipSlip.cs:107:73:107:84 | access to local variable destFilePath : String | ZipSlip.cs:112:74:112:85 | access to local variable destFilePath | provenance | |
43+
| ZipSlip.cs:107:73:107:84 | access to local variable destFilePath : String | ZipSlip.cs:114:71:114:82 | access to local variable destFilePath : String | provenance | |
44+
| ZipSlip.cs:114:71:114:82 | access to local variable destFilePath : String | ZipSlip.cs:119:71:119:82 | access to local variable destFilePath | provenance | |
45+
| ZipSlip.cs:114:71:114:82 | access to local variable destFilePath : String | ZipSlip.cs:119:71:119:82 | access to local variable destFilePath : String | provenance | |
46+
| ZipSlip.cs:119:71:119:82 | access to local variable destFilePath : String | ZipSlip.cs:121:71:121:82 | access to local variable destFilePath : String | provenance | |
47+
| ZipSlip.cs:121:71:121:82 | access to local variable destFilePath : String | ZipSlip.cs:126:57:126:68 | access to local variable destFilePath | provenance | |
48+
| ZipSlip.cs:121:71:121:82 | access to local variable destFilePath : String | ZipSlip.cs:129:71:129:82 | access to local variable destFilePath : String | provenance | |
49+
| ZipSlip.cs:129:71:129:82 | access to local variable destFilePath : String | ZipSlip.cs:134:58:134:69 | access to local variable destFilePath | provenance | |
3650
| ZipSlipBad.cs:9:16:9:27 | access to local variable destFileName : String | ZipSlipBad.cs:10:29:10:40 | access to local variable destFileName | provenance | |
3751
| ZipSlipBad.cs:9:31:9:73 | call to method Combine : String | ZipSlipBad.cs:9:16:9:27 | access to local variable destFileName : String | provenance | |
52+
| ZipSlipBad.cs:9:59:9:72 | access to property FullName : String | ZipSlipBad.cs:9:31:9:73 | call to method Combine : String | provenance | Config |
3853
| ZipSlipBad.cs:9:59:9:72 | access to property FullName : String | ZipSlipBad.cs:9:31:9:73 | call to method Combine : String | provenance | MaD:1 |
3954
models
4055
| 1 | Summary: System.IO; Path; false; Combine; (System.String,System.String); ; Argument[1]; ReturnValue; taint; manual |
@@ -45,6 +60,7 @@ nodes
4560
| ZipSlip.cs:15:61:15:74 | access to property FullName : String | semmle.label | access to property FullName : String |
4661
| ZipSlip.cs:18:24:18:49 | access to local variable file_badDirectoryTraversal : String | semmle.label | access to local variable file_badDirectoryTraversal : String |
4762
| ZipSlip.cs:18:53:18:66 | access to property FullName : String | semmle.label | access to property FullName : String |
63+
| ZipSlip.cs:19:43:19:68 | access to local variable file_badDirectoryTraversal : String | semmle.label | access to local variable file_badDirectoryTraversal : String |
4864
| ZipSlip.cs:22:28:22:39 | access to local variable destFileName : String | semmle.label | access to local variable destFileName : String |
4965
| ZipSlip.cs:22:43:22:97 | call to method Combine : String | semmle.label | call to method Combine : String |
5066
| ZipSlip.cs:22:71:22:96 | access to local variable file_badDirectoryTraversal : String | semmle.label | access to local variable file_badDirectoryTraversal : String |
@@ -59,13 +75,21 @@ nodes
5975
| ZipSlip.cs:58:31:58:75 | call to method Combine : String | semmle.label | call to method Combine : String |
6076
| ZipSlip.cs:58:61:58:74 | access to property FullName : String | semmle.label | access to property FullName : String |
6177
| ZipSlip.cs:62:33:62:40 | access to local variable fullpath | semmle.label | access to local variable fullpath |
78+
| ZipSlip.cs:62:33:62:40 | access to local variable fullpath : String | semmle.label | access to local variable fullpath : String |
79+
| ZipSlip.cs:64:29:64:36 | access to local variable fullpath : String | semmle.label | access to local variable fullpath : String |
80+
| ZipSlip.cs:69:30:69:37 | access to local variable fullpath : String | semmle.label | access to local variable fullpath : String |
6281
| ZipSlip.cs:71:37:71:44 | access to local variable fullpath | semmle.label | access to local variable fullpath |
6382
| ZipSlip.cs:105:32:105:43 | access to local variable destFilePath : String | semmle.label | access to local variable destFilePath : String |
6483
| ZipSlip.cs:105:47:105:86 | call to method Combine : String | semmle.label | call to method Combine : String |
6584
| ZipSlip.cs:105:72:105:85 | access to property FullName : String | semmle.label | access to property FullName : String |
85+
| ZipSlip.cs:107:73:107:84 | access to local variable destFilePath : String | semmle.label | access to local variable destFilePath : String |
6686
| ZipSlip.cs:112:74:112:85 | access to local variable destFilePath | semmle.label | access to local variable destFilePath |
87+
| ZipSlip.cs:114:71:114:82 | access to local variable destFilePath : String | semmle.label | access to local variable destFilePath : String |
6788
| ZipSlip.cs:119:71:119:82 | access to local variable destFilePath | semmle.label | access to local variable destFilePath |
89+
| ZipSlip.cs:119:71:119:82 | access to local variable destFilePath : String | semmle.label | access to local variable destFilePath : String |
90+
| ZipSlip.cs:121:71:121:82 | access to local variable destFilePath : String | semmle.label | access to local variable destFilePath : String |
6891
| ZipSlip.cs:126:57:126:68 | access to local variable destFilePath | semmle.label | access to local variable destFilePath |
92+
| ZipSlip.cs:129:71:129:82 | access to local variable destFilePath : String | semmle.label | access to local variable destFilePath : String |
6993
| ZipSlip.cs:134:58:134:69 | access to local variable destFilePath | semmle.label | access to local variable destFilePath |
7094
| ZipSlipBad.cs:9:16:9:27 | access to local variable destFileName : String | semmle.label | access to local variable destFileName : String |
7195
| ZipSlipBad.cs:9:31:9:73 | call to method Combine : String | semmle.label | call to method Combine : String |

‎rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ private import codeql.rust.dataflow.Ssa
1414
private import codeql.rust.dataflow.FlowSummary
1515
private import FlowSummaryImpl as FlowSummaryImpl
1616

17+
1718
/**
1819
* A return kind. A return kind describes how a value can be returned from a
1920
* callable.
@@ -45,6 +46,10 @@ final class DataFlowCallable extends TDataFlowCallable {
4546

4647
/** Gets the location of this callable. */
4748
Location getLocation() { result = this.asCfgScope().getLocation() }
49+
50+
//** TODO JB1: Move to subclass, monkey patching for #153 */
51+
int totalorder(){ none() }
52+
//** TODO JB1: end stubs for #153 */
4853
}
4954

5055
final class DataFlowCall extends TDataFlowCall {
@@ -86,6 +91,12 @@ final class DataFlowCall extends TDataFlowCall {
8691
}
8792

8893
Location getLocation() { result = this.asCallBaseExprCfgNode().getLocation() }
94+
95+
//** TODO JB1: Move to subclass, monkey patching for #153 */
96+
DataFlowCallable getARuntimeTarget(){ none() }
97+
Node::ArgumentNode getAnArgumentNode(){ none() }
98+
int totalorder(){ none() }
99+
//** TODO JB1: end stubs for #153 */
89100
}
90101

91102
/**
@@ -989,6 +1000,10 @@ module RustDataFlow implements InputSig<Location> {
9891000
string toString() { result = "NodeRegion" }
9901001

9911002
predicate contains(Node n) { none() }
1003+
1004+
//** TODO JB1: Move to subclass, monkey patching for #153 */
1005+
int totalOrder(){ none() }
1006+
//** TODO JB1: end stubs for #153 */
9921007
}
9931008

9941009
/**

‎rust/ql/src/queries/diagnostics/DataFlowConsistencyCounts.ql

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
/**
2+
* @name Data flow inconsistency counts
3+
* @description Counts the number of data flow inconsistencies of each type. This query is intended for internal use.
4+
* @kind diagnostic
5+
* @id rust/diagnostics/data-flow-consistency-counts
6+
*/
7+
8+
import codeql.rust.dataflow.internal.DataFlowConsistency as Consistency
9+
10+
// see also `rust/diagnostics/data-flow-consistency`, which lists the
11+
// individual inconsistency results.
12+
from string type, int num
13+
where num = Consistency::getInconsistencyCounts(type)
14+
select type, num

‎rust/ql/src/queries/security/CWE-089/SqlInjection.ql

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
/**
2+
* @name Database query built from user-controlled sources
3+
* @description Building a database query from user-controlled sources is vulnerable to insertion of malicious code by attackers.
4+
* @kind path-problem
5+
* @problem.severity error
6+
* @security-severity 8.8
7+
* @precision high
8+
* @id rust/sql-injection
9+
* @tags security
10+
* external/cwe/cwe-089
11+
*/
12+
13+
import rust
14+
import codeql.rust.dataflow.DataFlow
15+
import codeql.rust.dataflow.TaintTracking
16+
import codeql.rust.security.SqlInjectionExtensions
17+
import SqlInjectionFlow::PathGraph
18+
19+
/**
20+
* A taint configuration for tainted data that reaches a SQL sink.
21+
*/
22+
module SqlInjectionConfig implements DataFlow::ConfigSig {
23+
predicate isSource(DataFlow::Node node) { node instanceof SqlInjection::Source }
24+
25+
predicate isSink(DataFlow::Node node) { node instanceof SqlInjection::Sink }
26+
27+
predicate isBarrier(DataFlow::Node barrier) { barrier instanceof SqlInjection::Barrier }
28+
}
29+
30+
module SqlInjectionFlow = TaintTracking::Global<SqlInjectionConfig>;
31+
32+
from SqlInjectionFlow::PathNode sourceNode, SqlInjectionFlow::PathNode sinkNode
33+
where SqlInjectionFlow::flowPath(sourceNode, sinkNode)
34+
select sinkNode.getNode(), sourceNode, sinkNode, "This query depends on a $@.",
35+
sourceNode.getNode(), "user-provided value"

‎rust/ql/src/queries/summary/LinesOfCode.ql

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
/**
2+
* @name Total lines of Rust code in the database
3+
* @description The total number of lines of Rust code across all files, including any libraries and auto-generated files that the extractor sees. This is a useful metric of the size of a database. For all files that were seen during the build, this query counts the lines of code, excluding whitespace or comments.
4+
* @kind metric
5+
* @id rust/summary/lines-of-code
6+
* @tags summary
7+
* lines-of-code
8+
* telemetry
9+
*/
10+
11+
import rust
12+
import Stats
13+
14+
select getLinesOfCode()

‎rust/ql/src/queries/summary/LinesOfUserCode.ql

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
/**
2+
* @name Total lines of user written Rust code in the database
3+
* @description The total number of lines of Rust code from the source code directory. This query counts the lines of code, excluding whitespace or comments.
4+
* @kind metric
5+
* @id rust/summary/lines-of-user-code
6+
* @tags summary
7+
* lines-of-code
8+
* debug
9+
*/
10+
11+
import rust
12+
import Stats
13+
14+
select getLinesOfUserCode()

‎rust/ql/src/queries/summary/SummaryStats.ql

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
/**
2+
* @name Summary Statistics
3+
* @description A table of summary statistics about a database.
4+
* @kind metric
5+
* @id rust/summary/summary-statistics
6+
* @tags summary
7+
*/
8+
9+
import rust
10+
import codeql.rust.Concepts
11+
import codeql.rust.Diagnostics
12+
import Stats
13+
14+
from string key, int value
15+
where
16+
key = "Elements extracted" and value = count(Element e | not e instanceof Unextracted)
17+
or
18+
key = "Elements unextracted" and value = count(Unextracted e)
19+
or
20+
key = "Extraction errors" and value = count(ExtractionError e)
21+
or
22+
key = "Extraction warnings" and value = count(ExtractionWarning w)
23+
or
24+
key = "Files extracted - total" and value = count(ExtractedFile f | exists(f.getRelativePath()))
25+
or
26+
key = "Files extracted - with errors" and
27+
value =
28+
count(ExtractedFile f |
29+
exists(f.getRelativePath()) and not f instanceof SuccessfullyExtractedFile
30+
)
31+
or
32+
key = "Files extracted - without errors" and
33+
value = count(SuccessfullyExtractedFile f | exists(f.getRelativePath()))
34+
or
35+
key = "Lines of code extracted" and value = getLinesOfCode()
36+
or
37+
key = "Lines of user code extracted" and value = getLinesOfUserCode()
38+
or
39+
key = "Inconsistencies - AST" and value = getTotalAstInconsistencies()
40+
or
41+
key = "Inconsistencies - CFG" and value = getTotalCfgInconsistencies()
42+
or
43+
key = "Inconsistencies - data flow" and value = getTotalDataFlowInconsistencies()
44+
or
45+
key = "Macro calls - total" and value = count(MacroCall mc)
46+
or
47+
key = "Macro calls - resolved" and value = count(MacroCall mc | mc.hasExpanded())
48+
or
49+
key = "Macro calls - unresolved" and value = count(MacroCall mc | not mc.hasExpanded())
50+
or
51+
key = "Taint sources - total" and value = count(ThreatModelSource s)
52+
or
53+
key = "Taint sources - active" and value = count(ActiveThreatModelSource s)
54+
select key, value order by key

‎rust/ql/src/queries/summary/TaintSources.ql

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
/**
2+
* @name Taint Sources
3+
* @description List all sources of untrusted input that have been idenfitied
4+
* in the database.
5+
* @kind problem
6+
* @problem.severity info
7+
* @id rust/summary/taint-sources
8+
* @tags summary
9+
*/
10+
11+
import rust
12+
import codeql.rust.Concepts
13+
14+
from ThreatModelSource s, string defaultString
15+
where
16+
if s instanceof ActiveThreatModelSource then defaultString = " (DEFAULT)" else defaultString = ""
17+
select s,
18+
"Flow source '" + s.getSourceType() + "' of type " + s.getThreatModel() + defaultString + "."

0 commit comments

Comments
 (0)