Skip to content

Commit 2b07759

Browse files
aibaarsaibaars
committed
Also track DTDLOAD and NONET
1 parent 4268d9c commit 2b07759

File tree

3 files changed

+98
-19
lines changed

3 files changed

+98
-19
lines changed

‎ql/lib/codeql/ruby/frameworks/XmlParsing.qll‎

Lines changed: 75 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@ private class NokogiriXmlParserCall extends XmlParserCall::Range, DataFlow::Call
2222
override DataFlow::Node getInput() { result = this.getArgument(0) }
2323

2424
override predicate externalEntitiesEnabled() {
25-
this.getArgument(3) = trackNoEnt()
25+
this.getArgument(3) =
26+
[trackEnableFeature(TNOENT()), trackEnableFeature(TDTDLOAD()), trackDisableFeature(TNONET())]
2627
or
2728
this.asExpr()
2829
.getExpr()
@@ -31,7 +32,7 @@ private class NokogiriXmlParserCall extends XmlParserCall::Range, DataFlow::Call
3132
.getAStmt()
3233
.getAChild*()
3334
.(MethodCall)
34-
.getMethodName() = "noent"
35+
.getMethodName() = ["noent", "nononet"]
3536
}
3637
}
3738

@@ -49,41 +50,95 @@ private class LibXmlRubyXmlParserCall extends XmlParserCall::Range, DataFlow::Ca
4950
exists(Pair pair |
5051
pair = this.getArgument(1).asExpr().getExpr().(HashLiteral).getAKeyValuePair() and
5152
pair.getKey().(Literal).getValueText() = "options" and
52-
trackNoEnt().asExpr().getExpr() = pair.getValue()
53+
pair.getValue() =
54+
[
55+
trackEnableFeature(TNOENT()), trackEnableFeature(TDTDLOAD()),
56+
trackDisableFeature(TNONET())
57+
].asExpr().getExpr()
5358
)
5459
}
5560
}
5661

57-
private DataFlow::LocalSourceNode trackNoEnt(TypeTracker t) {
62+
private newtype TFeature =
63+
TNOENT() or
64+
TNONET() or
65+
TDTDLOAD()
66+
67+
class Feature extends TFeature {
68+
abstract int getValue();
69+
70+
string toString() { result = getConstantName() }
71+
72+
abstract string getConstantName();
73+
}
74+
75+
private class FeatureNOENT extends Feature, TNOENT {
76+
override int getValue() { result = 2 }
77+
78+
override string getConstantName() { result = "NOENT" }
79+
}
80+
81+
private class FeatureNONET extends Feature, TNONET {
82+
override int getValue() { result = 2048 }
83+
84+
override string getConstantName() { result = "NONET" }
85+
}
86+
87+
private class FeatureDTDLOAD extends Feature, TDTDLOAD {
88+
override int getValue() { result = 4 }
89+
90+
override string getConstantName() { result = "DTDLOAD" }
91+
}
92+
93+
private DataFlow::LocalSourceNode trackFeature(Feature f, boolean enable, TypeTracker t) {
5894
t.start() and
5995
(
60-
result.asExpr().getExpr().(IntegerLiteral).getValue().bitAnd(2) = 2
96+
result.asExpr().getExpr().(IntegerLiteral).getValue().bitAnd(f.getValue()) = f.getValue() and
97+
enable = true
6198
or
99+
enable = true and
62100
result =
63101
API::getTopLevelMember("Nokogiri")
64102
.getMember("XML")
65103
.getMember("ParseOptions")
66-
.getMember("NOENT")
104+
.getMember(f.getConstantName())
67105
.getAUse()
68106
or
107+
enable = true and
69108
result =
70109
[API::getTopLevelMember("LibXML").getMember("XML"), API::getTopLevelMember("XML")]
71110
.getMember("Options")
72-
.getMember("NOENT")
111+
.getMember(f.getConstantName())
73112
.getAUse()
74113
or
75-
result.asExpr().getExpr() instanceof BitwiseOrExpr and
76-
result.asExpr().(CfgNodes::ExprNodes::OperationCfgNode).getAnOperand() = trackNoEnt().asExpr()
114+
(
115+
result.asExpr().getExpr() instanceof BitwiseOrExpr or
116+
result.asExpr().getExpr() instanceof AssignBitwiseOrExpr or
117+
result.asExpr().getExpr() instanceof BitwiseAndExpr or
118+
result.asExpr().getExpr() instanceof AssignBitwiseAndExpr
119+
) and
120+
result.asExpr().(CfgNodes::ExprNodes::OperationCfgNode).getAnOperand() =
121+
trackFeature(f, enable).asExpr()
122+
or
123+
result.asExpr().getExpr() instanceof ComplementExpr and
124+
result.asExpr().(CfgNodes::ExprNodes::OperationCfgNode).getAnOperand() =
125+
trackFeature(f, enable.booleanNot()).asExpr()
77126
or
78127
result =
79128
API::getTopLevelMember("Nokogiri")
80129
.getMember("XML")
81130
.getMember("ParseOptions")
82131
.getAnInstantiation() and
83-
result.asExpr().(CfgNodes::ExprNodes::CallCfgNode).getArgument(0) = trackNoEnt().asExpr()
132+
result.asExpr().(CfgNodes::ExprNodes::CallCfgNode).getArgument(0) =
133+
trackFeature(f, enable).asExpr()
84134
or
85135
exists(CfgNodes::ExprNodes::CallCfgNode call |
86-
call.getExpr().(MethodCall).getMethodName() = "noent" and
136+
enable = true and
137+
call.getExpr().(MethodCall).getMethodName() = f.getConstantName().toLowerCase()
138+
or
139+
enable = false and
140+
call.getExpr().(MethodCall).getMethodName() = "no" + f.getConstantName().toLowerCase()
141+
|
87142
(
88143
result.asExpr() = call
89144
or
@@ -92,12 +147,18 @@ private DataFlow::LocalSourceNode trackNoEnt(TypeTracker t) {
92147
)
93148
or
94149
exists(CfgNodes::ExprNodes::CallCfgNode call |
95-
trackNoEnt().asExpr() = call.getReceiver() and
150+
trackFeature(f, enable).asExpr() = call.getReceiver() and
96151
result.asExpr() = call
97152
)
98153
)
99154
or
100-
exists(TypeTracker t2 | result = trackNoEnt(t2).track(t2, t))
155+
exists(TypeTracker t2 | result = trackFeature(f, enable, t2).track(t2, t))
156+
}
157+
158+
private DataFlow::Node trackFeature(Feature f, boolean enable) {
159+
trackFeature(f, enable, TypeTracker::end()).flowsTo(result)
101160
}
102161

103-
private DataFlow::Node trackNoEnt() { trackNoEnt(TypeTracker::end()).flowsTo(result) }
162+
private DataFlow::Node trackEnableFeature(Feature f) { result = trackFeature(f, true) }
163+
164+
private DataFlow::Node trackDisableFeature(Feature f) { result = trackFeature(f, false) }

‎ql/test/query-tests/security/cwe-611/Nokogiri.rb‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,19 +4,25 @@ class NokogiriXXE < ApplicationController
44

55
Nokogiri::XML::parse(content, nil, nil, 2)
66
Nokogiri::XML::parse(content, nil, nil, 1 | 2)
7+
Nokogiri::XML::parse(content, nil, nil, 1 & ~Nokogiri::XML::ParseOptions::NONET)
78
Nokogiri::XML::parse(content, nil, nil, Nokogiri::XML::ParseOptions::NOENT)
9+
Nokogiri::XML::parse(content, nil, nil, Nokogiri::XML::ParseOptions::DTDLOAD)
10+
Nokogiri::XML::parse(content, nil, nil, ~Nokogiri::XML::ParseOptions::NOENT) #OK
11+
Nokogiri::XML::parse(content, nil, nil, ~Nokogiri::XML::ParseOptions::NONET)
812
Nokogiri::XML::parse(content, nil, nil, Nokogiri::XML::ParseOptions.new 2)
913
options = Nokogiri::XML::ParseOptions.new 0
1014
options.noent
1115
Nokogiri::XML::parse(content, nil, nil, options)
1216
Nokogiri::XML::parse(content, nil, nil, (Nokogiri::XML::ParseOptions.new 0).noent)
1317

1418
Nokogiri::XML::parse(content) { |x| x.noent }
19+
Nokogiri::XML::parse(content) { |x| x.nononet }
20+
Nokogiri::XML::parse(content) { |x| x.nodtdload } # OK
1521

16-
Nokogiri::XML::parse(content) { |x| x.nonet.noent.dtdload }
22+
Nokogiri::XML::parse(content) { |x| x.nonet.noent.nodtdload }
1723

1824
Nokogiri::XML::parse(content, nil, nil, 1) # OK
1925
Nokogiri::XML::parse(content, nil, nil, 3)
20-
Nokogiri::XML::parse(content) { |x| x.nonet.dtdload } # OK
26+
Nokogiri::XML::parse(content) { |x| x.nonet.nodtdload } # OK
2127

2228
end

‎ql/test/query-tests/security/cwe-611/Xxe.expected‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,15 @@ edges
1111
| Nokogiri.rb:3:15:3:20 | call to params : | Nokogiri.rb:6:26:6:32 | content |
1212
| Nokogiri.rb:3:15:3:20 | call to params : | Nokogiri.rb:7:26:7:32 | content |
1313
| Nokogiri.rb:3:15:3:20 | call to params : | Nokogiri.rb:8:26:8:32 | content |
14+
| Nokogiri.rb:3:15:3:20 | call to params : | Nokogiri.rb:9:26:9:32 | content |
1415
| Nokogiri.rb:3:15:3:20 | call to params : | Nokogiri.rb:11:26:11:32 | content |
1516
| Nokogiri.rb:3:15:3:20 | call to params : | Nokogiri.rb:12:26:12:32 | content |
16-
| Nokogiri.rb:3:15:3:20 | call to params : | Nokogiri.rb:14:26:14:32 | content |
17+
| Nokogiri.rb:3:15:3:20 | call to params : | Nokogiri.rb:15:26:15:32 | content |
1718
| Nokogiri.rb:3:15:3:20 | call to params : | Nokogiri.rb:16:26:16:32 | content |
19+
| Nokogiri.rb:3:15:3:20 | call to params : | Nokogiri.rb:18:26:18:32 | content |
1820
| Nokogiri.rb:3:15:3:20 | call to params : | Nokogiri.rb:19:26:19:32 | content |
21+
| Nokogiri.rb:3:15:3:20 | call to params : | Nokogiri.rb:22:26:22:32 | content |
22+
| Nokogiri.rb:3:15:3:20 | call to params : | Nokogiri.rb:25:26:25:32 | content |
1923
nodes
2024
| LibXmlRuby.rb:3:15:3:20 | call to params : | semmle.label | call to params : |
2125
| LibXmlRuby.rb:4:34:4:40 | content | semmle.label | content |
@@ -31,11 +35,15 @@ nodes
3135
| Nokogiri.rb:6:26:6:32 | content | semmle.label | content |
3236
| Nokogiri.rb:7:26:7:32 | content | semmle.label | content |
3337
| Nokogiri.rb:8:26:8:32 | content | semmle.label | content |
38+
| Nokogiri.rb:9:26:9:32 | content | semmle.label | content |
3439
| Nokogiri.rb:11:26:11:32 | content | semmle.label | content |
3540
| Nokogiri.rb:12:26:12:32 | content | semmle.label | content |
36-
| Nokogiri.rb:14:26:14:32 | content | semmle.label | content |
41+
| Nokogiri.rb:15:26:15:32 | content | semmle.label | content |
3742
| Nokogiri.rb:16:26:16:32 | content | semmle.label | content |
43+
| Nokogiri.rb:18:26:18:32 | content | semmle.label | content |
3844
| Nokogiri.rb:19:26:19:32 | content | semmle.label | content |
45+
| Nokogiri.rb:22:26:22:32 | content | semmle.label | content |
46+
| Nokogiri.rb:25:26:25:32 | content | semmle.label | content |
3947
subpaths
4048
#select
4149
| LibXmlRuby.rb:4:34:4:40 | content | LibXmlRuby.rb:3:15:3:20 | call to params : | LibXmlRuby.rb:4:34:4:40 | content | Unsafe parsing of XML file from $@. | LibXmlRuby.rb:3:15:3:20 | call to params | user input |
@@ -50,8 +58,12 @@ subpaths
5058
| Nokogiri.rb:6:26:6:32 | content | Nokogiri.rb:3:15:3:20 | call to params : | Nokogiri.rb:6:26:6:32 | content | Unsafe parsing of XML file from $@. | Nokogiri.rb:3:15:3:20 | call to params | user input |
5159
| Nokogiri.rb:7:26:7:32 | content | Nokogiri.rb:3:15:3:20 | call to params : | Nokogiri.rb:7:26:7:32 | content | Unsafe parsing of XML file from $@. | Nokogiri.rb:3:15:3:20 | call to params | user input |
5260
| Nokogiri.rb:8:26:8:32 | content | Nokogiri.rb:3:15:3:20 | call to params : | Nokogiri.rb:8:26:8:32 | content | Unsafe parsing of XML file from $@. | Nokogiri.rb:3:15:3:20 | call to params | user input |
61+
| Nokogiri.rb:9:26:9:32 | content | Nokogiri.rb:3:15:3:20 | call to params : | Nokogiri.rb:9:26:9:32 | content | Unsafe parsing of XML file from $@. | Nokogiri.rb:3:15:3:20 | call to params | user input |
5362
| Nokogiri.rb:11:26:11:32 | content | Nokogiri.rb:3:15:3:20 | call to params : | Nokogiri.rb:11:26:11:32 | content | Unsafe parsing of XML file from $@. | Nokogiri.rb:3:15:3:20 | call to params | user input |
5463
| Nokogiri.rb:12:26:12:32 | content | Nokogiri.rb:3:15:3:20 | call to params : | Nokogiri.rb:12:26:12:32 | content | Unsafe parsing of XML file from $@. | Nokogiri.rb:3:15:3:20 | call to params | user input |
55-
| Nokogiri.rb:14:26:14:32 | content | Nokogiri.rb:3:15:3:20 | call to params : | Nokogiri.rb:14:26:14:32 | content | Unsafe parsing of XML file from $@. | Nokogiri.rb:3:15:3:20 | call to params | user input |
64+
| Nokogiri.rb:15:26:15:32 | content | Nokogiri.rb:3:15:3:20 | call to params : | Nokogiri.rb:15:26:15:32 | content | Unsafe parsing of XML file from $@. | Nokogiri.rb:3:15:3:20 | call to params | user input |
5665
| Nokogiri.rb:16:26:16:32 | content | Nokogiri.rb:3:15:3:20 | call to params : | Nokogiri.rb:16:26:16:32 | content | Unsafe parsing of XML file from $@. | Nokogiri.rb:3:15:3:20 | call to params | user input |
66+
| Nokogiri.rb:18:26:18:32 | content | Nokogiri.rb:3:15:3:20 | call to params : | Nokogiri.rb:18:26:18:32 | content | Unsafe parsing of XML file from $@. | Nokogiri.rb:3:15:3:20 | call to params | user input |
5767
| Nokogiri.rb:19:26:19:32 | content | Nokogiri.rb:3:15:3:20 | call to params : | Nokogiri.rb:19:26:19:32 | content | Unsafe parsing of XML file from $@. | Nokogiri.rb:3:15:3:20 | call to params | user input |
68+
| Nokogiri.rb:22:26:22:32 | content | Nokogiri.rb:3:15:3:20 | call to params : | Nokogiri.rb:22:26:22:32 | content | Unsafe parsing of XML file from $@. | Nokogiri.rb:3:15:3:20 | call to params | user input |
69+
| Nokogiri.rb:25:26:25:32 | content | Nokogiri.rb:3:15:3:20 | call to params : | Nokogiri.rb:25:26:25:32 | content | Unsafe parsing of XML file from $@. | Nokogiri.rb:3:15:3:20 | call to params | user input |

0 commit comments

Comments
 (0)