Skip to content

Commit 26c29cb

Browse files
author
Esben Sparre Andreasen
committed
JS: split TypeConfusionThroughParameterTampering.qll
1 parent 8225d99 commit 26c29cb

File tree

2 files changed

+103
-85
lines changed

2 files changed

+103
-85
lines changed

‎javascript/ql/src/semmle/javascript/security/dataflow/TypeConfusionThroughParameterTampering.qll‎

Lines changed: 8 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,18 @@
11
/**
2-
* Provides a tracking configuration for reasoning about type confusion for HTTP request inputs.
2+
* Provides a tracking configuration for reasoning about type
3+
* confusion for HTTP request inputs.
4+
*
5+
* Note, for performance reasons: only import this file if
6+
* `TypeConfusionThroughParameterTampering::Configuration` is needed,
7+
* otherwise `TypeConfusionThroughParameterTamperingCustomizations`
8+
* should be imported instead.
39
*/
410

511
import javascript
6-
import semmle.javascript.security.dataflow.RemoteFlowSources
712
private import semmle.javascript.dataflow.InferredTypes
813

914
module TypeConfusionThroughParameterTampering {
10-
/**
11-
* A data flow source for type confusion for HTTP request inputs.
12-
*/
13-
abstract class Source extends DataFlow::Node { }
14-
15-
/**
16-
* A data flow sink for type confusion for HTTP request inputs.
17-
*/
18-
abstract class Sink extends DataFlow::Node { }
19-
20-
/**
21-
* A barrier for type confusion for HTTP request inputs.
22-
*/
23-
abstract class Barrier extends DataFlow::Node { }
15+
import TypeConfusionThroughParameterTamperingCustomizations::TypeConfusionThroughParameterTampering
2416

2517
/**
2618
* A taint tracking configuration for type confusion for HTTP request inputs.
@@ -38,73 +30,4 @@ module TypeConfusionThroughParameterTampering {
3830

3931
override predicate isBarrier(DataFlow::Node node) { node instanceof Barrier }
4032
}
41-
42-
/**
43-
* An HTTP request parameter that the user controls the type of.
44-
*
45-
* Node.js-based HTTP servers turn request parameters into arrays if their names are repeated.
46-
*/
47-
private class TypeTamperableRequestParameter extends Source {
48-
TypeTamperableRequestParameter() { this.(HTTP::RequestInputAccess).isUserControlledObject() }
49-
}
50-
51-
/**
52-
* Methods calls that behave slightly different for arrays and strings receivers.
53-
*/
54-
private class StringArrayAmbiguousMethodCall extends Sink {
55-
StringArrayAmbiguousMethodCall() {
56-
exists(string name, DataFlow::MethodCallNode mc |
57-
name = "concat" or
58-
name = "includes" or
59-
name = "indexOf" or
60-
name = "lastIndexOf" or
61-
name = "slice"
62-
|
63-
mc.calls(this, name) and
64-
// ignore patterns that are innocent in practice
65-
not exists(EqualityTest cmp, Expr op | cmp.hasOperands(mc.asExpr(), op) |
66-
// prefix checking: `x.indexOf(prefix) === 0`
67-
name = "indexOf" and
68-
op.getIntValue() = 0
69-
or
70-
// suffix checking: `x.slice(-1) === '/'`
71-
name = "slice" and
72-
mc.getArgument(0).asExpr().getIntValue() = -1 and
73-
op.getStringValue().length() = 1
74-
)
75-
)
76-
}
77-
}
78-
79-
/**
80-
* An access to the `length` property of an object.
81-
*/
82-
private class LengthAccess extends Sink {
83-
LengthAccess() {
84-
exists(DataFlow::PropRead read |
85-
read.accesses(this, "length") and
86-
// an array/string confusion cannot control an emptiness check
87-
not (
88-
// `if (x.length) {...}`
89-
exists(ConditionGuardNode cond | read.asExpr() = cond.getTest())
90-
or
91-
// `x.length == 0`, `x.length > 0`
92-
exists(Comparison cmp, Expr zero |
93-
zero.getIntValue() = 0 and
94-
cmp.hasOperands(read.asExpr(), zero)
95-
)
96-
or
97-
// `x.length < 1`
98-
exists(RelationalComparison cmp |
99-
cmp.getLesserOperand() = read.asExpr() and
100-
cmp.getGreaterOperand().getIntValue() = 1 and
101-
not cmp.isInclusive()
102-
)
103-
or
104-
// `!x.length`
105-
exists(LogNotExpr neg | neg.getOperand() = read.asExpr())
106-
)
107-
)
108-
}
109-
}
11033
}

‎javascript/ql/src/semmle/javascript/security/dataflow/TypeConfusionThroughParameterTamperingCustomizations.qll‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
/**
2+
* Provides default sources, sinks and sanitisers for reasoning about
3+
* type confusion for HTTP request inputs, as well as extension points
4+
* for adding your own.
5+
*/
6+
7+
import javascript
8+
import semmle.javascript.security.dataflow.RemoteFlowSources
9+
private import semmle.javascript.dataflow.InferredTypes
10+
11+
module TypeConfusionThroughParameterTampering {
12+
/**
13+
* A data flow source for type confusion for HTTP request inputs.
14+
*/
15+
abstract class Source extends DataFlow::Node { }
16+
17+
/**
18+
* A data flow sink for type confusion for HTTP request inputs.
19+
*/
20+
abstract class Sink extends DataFlow::Node { }
21+
22+
/**
23+
* A barrier for type confusion for HTTP request inputs.
24+
*/
25+
abstract class Barrier extends DataFlow::Node { }
26+
27+
/**
28+
* An HTTP request parameter that the user controls the type of.
29+
*
30+
* Node.js-based HTTP servers turn request parameters into arrays if their names are repeated.
31+
*/
32+
private class TypeTamperableRequestParameter extends Source {
33+
TypeTamperableRequestParameter() { this.(HTTP::RequestInputAccess).isUserControlledObject() }
34+
}
35+
36+
/**
37+
* Methods calls that behave slightly different for arrays and strings receivers.
38+
*/
39+
private class StringArrayAmbiguousMethodCall extends Sink {
40+
StringArrayAmbiguousMethodCall() {
41+
exists(string name, DataFlow::MethodCallNode mc |
42+
name = "concat" or
43+
name = "includes" or
44+
name = "indexOf" or
45+
name = "lastIndexOf" or
46+
name = "slice"
47+
|
48+
mc.calls(this, name) and
49+
// ignore patterns that are innocent in practice
50+
not exists(EqualityTest cmp, Expr op | cmp.hasOperands(mc.asExpr(), op) |
51+
// prefix checking: `x.indexOf(prefix) === 0`
52+
name = "indexOf" and
53+
op.getIntValue() = 0
54+
or
55+
// suffix checking: `x.slice(-1) === '/'`
56+
name = "slice" and
57+
mc.getArgument(0).asExpr().getIntValue() = -1 and
58+
op.getStringValue().length() = 1
59+
)
60+
)
61+
}
62+
}
63+
64+
/**
65+
* An access to the `length` property of an object.
66+
*/
67+
private class LengthAccess extends Sink {
68+
LengthAccess() {
69+
exists(DataFlow::PropRead read |
70+
read.accesses(this, "length") and
71+
// an array/string confusion cannot control an emptiness check
72+
not (
73+
// `if (x.length) {...}`
74+
exists(ConditionGuardNode cond | read.asExpr() = cond.getTest())
75+
or
76+
// `x.length == 0`, `x.length > 0`
77+
exists(Comparison cmp, Expr zero |
78+
zero.getIntValue() = 0 and
79+
cmp.hasOperands(read.asExpr(), zero)
80+
)
81+
or
82+
// `x.length < 1`
83+
exists(RelationalComparison cmp |
84+
cmp.getLesserOperand() = read.asExpr() and
85+
cmp.getGreaterOperand().getIntValue() = 1 and
86+
not cmp.isInclusive()
87+
)
88+
or
89+
// `!x.length`
90+
exists(LogNotExpr neg | neg.getOperand() = read.asExpr())
91+
)
92+
)
93+
}
94+
}
95+
}

0 commit comments

Comments
 (0)