Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 6486302

Browse filesBrowse files
hmachmac
committed
Ruby: Remove additional XSS taint steps
These are now covered by more general steps for render calls.
1 parent 458b556 commit 6486302
Copy full SHA for 6486302

File tree

Expand file treeCollapse file tree

3 files changed

+0
-123
lines changed
Filter options
Expand file treeCollapse file tree

3 files changed

+0
-123
lines changed

‎ruby/ql/lib/codeql/ruby/security/ReflectedXSSQuery.qll

Copy file name to clipboardExpand all lines: ruby/ql/lib/codeql/ruby/security/ReflectedXSSQuery.qll
-8Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,6 @@ deprecated module ReflectedXss {
2929
override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
3030

3131
override predicate isSanitizer(DataFlow::Node node) { node instanceof Sanitizer }
32-
33-
override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
34-
isAdditionalXssTaintStep(node1, node2)
35-
}
3632
}
3733
}
3834

@@ -44,10 +40,6 @@ private module ReflectedXssConfig implements DataFlow::ConfigSig {
4440
predicate isSink(DataFlow::Node sink) { sink instanceof RX::Sink }
4541

4642
predicate isBarrier(DataFlow::Node node) { node instanceof RX::Sanitizer }
47-
48-
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
49-
RX::isAdditionalXssTaintStep(node1, node2)
50-
}
5143
}
5244

5345
/**

‎ruby/ql/lib/codeql/ruby/security/StoredXSSQuery.qll

Copy file name to clipboardExpand all lines: ruby/ql/lib/codeql/ruby/security/StoredXSSQuery.qll
-8Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,6 @@ deprecated module StoredXss {
3434
super.isSanitizer(node) or
3535
node instanceof Sanitizer
3636
}
37-
38-
override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
39-
isAdditionalXssTaintStep(node1, node2)
40-
}
4137
}
4238

4339
import TaintTracking::Global<StoredXssConfig>
@@ -51,10 +47,6 @@ private module StoredXssConfig implements DataFlow::ConfigSig {
5147
predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
5248

5349
predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
54-
55-
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
56-
isAdditionalXssTaintStep(node1, node2)
57-
}
5850
}
5951

6052
/**

‎ruby/ql/lib/codeql/ruby/security/XSS.qll

Copy file name to clipboardExpand all lines: ruby/ql/lib/codeql/ruby/security/XSS.qll
-107Lines changed: 0 additions & 107 deletions
Original file line numberDiff line numberDiff line change
@@ -146,99 +146,6 @@ private module Shared {
146146

147147
AssignExpr getAnAssignExpr() { result.getLeftOperand() = this.getExpr() }
148148
}
149-
150-
/**
151-
* Holds if `call` is a method call in ERB file `erb`, targeting a method
152-
* named `name`.
153-
*/
154-
pragma[noinline]
155-
private predicate isMethodCall(MethodCall call, string name, ErbFile erb) {
156-
name = call.getMethodName() and
157-
erb = call.getLocation().getFile()
158-
}
159-
160-
/**
161-
* Holds if `action` contains an assignment of `value` to an instance
162-
* variable named `name`, in ERB file `erb`.
163-
*/
164-
pragma[noinline]
165-
private predicate actionAssigns(
166-
ActionControllerActionMethod action, string name, Expr value, ErbFile erb
167-
) {
168-
exists(AssignExpr ae, FinalInstanceVarWrite controllerVarWrite |
169-
action.getDefaultTemplateFile() = erb and
170-
ae.getParent+() = action and
171-
ae = controllerVarWrite.getAnAssignExpr() and
172-
name = controllerVarWrite.getVariable().getName() and
173-
value = ae.getRightOperand()
174-
)
175-
}
176-
177-
pragma[noinline]
178-
private predicate isVariableReadAccess(VariableReadAccess viewVarRead, string name, ErbFile erb) {
179-
erb = viewVarRead.getLocation().getFile() and
180-
viewVarRead.getVariable().getName() = name
181-
}
182-
183-
private predicate isFlowFromControllerInstanceVariable(DataFlow::Node node1, DataFlow::Node node2) {
184-
// instance variables in the controller
185-
exists(string name, ErbFile template |
186-
// match read to write on variable name
187-
actionAssigns(_, name, node1.asExpr().getExpr(), template) and
188-
// propagate taint from assignment RHS expr to variable read access in view
189-
isVariableReadAccess(node2.asExpr().getExpr(), name, template)
190-
)
191-
}
192-
193-
/**
194-
* Holds if `helperMethod` is a helper method named `name` that is associated
195-
* with ERB file `erb`.
196-
*/
197-
pragma[noinline]
198-
private predicate isHelperMethod(
199-
ActionControllerHelperMethod helperMethod, string name, ErbFile erb
200-
) {
201-
helperMethod.getName() = name and
202-
helperMethod.getControllerClass() = getAssociatedControllerClass(erb)
203-
}
204-
205-
private predicate isFlowIntoHelperMethod(DataFlow::Node node1, DataFlow::Node node2) {
206-
// flow from template into controller helper method
207-
exists(
208-
ErbFile template, ActionControllerHelperMethod helperMethod, string name,
209-
CfgNodes::ExprNodes::MethodCallCfgNode helperMethodCall, int argIdx
210-
|
211-
isHelperMethod(helperMethod, name, template) and
212-
isMethodCall(helperMethodCall.getExpr(), name, template) and
213-
helperMethodCall.getArgument(pragma[only_bind_into](argIdx)) = node1.asExpr() and
214-
helperMethod.getParameter(pragma[only_bind_into](argIdx)) = node2.asParameter()
215-
)
216-
}
217-
218-
private predicate isFlowFromHelperMethod(DataFlow::Node node1, DataFlow::Node node2) {
219-
// flow out of controller helper method into template
220-
exists(ErbFile template, ActionControllerHelperMethod helperMethod, string name |
221-
// `node1` is an expr node that may be returned by the helper method
222-
exprNodeReturnedFrom(node1, helperMethod) and
223-
// `node2` is a call to the helper method
224-
isHelperMethod(helperMethod, name, template) and
225-
isMethodCall(node2.asExpr().getExpr(), name, template)
226-
)
227-
}
228-
229-
/**
230-
* An additional step that is preserves dataflow in the context of XSS.
231-
*/
232-
predicate isAdditionalXssFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
233-
none() and
234-
(
235-
isFlowFromControllerInstanceVariable(node1, node2)
236-
or
237-
isFlowIntoHelperMethod(node1, node2)
238-
or
239-
isFlowFromHelperMethod(node1, node2)
240-
)
241-
}
242149
}
243150

244151
/**
@@ -256,11 +163,6 @@ module ReflectedXss {
256163
/** A sanitizer for stored XSS vulnerabilities. */
257164
class Sanitizer = Shared::Sanitizer;
258165

259-
/**
260-
* An additional step that is preserves dataflow in the context of reflected XSS.
261-
*/
262-
predicate isAdditionalXssTaintStep = Shared::isAdditionalXssFlowStep/2;
263-
264166
/**
265167
* A HTTP request input, considered as a flow source.
266168
*/
@@ -285,10 +187,6 @@ private module OrmTracking {
285187
// Select any call receiver and narrow down later
286188
predicate isSink(DataFlow::Node sink) { sink = any(DataFlow::CallNode c).getReceiver() }
287189

288-
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
289-
Shared::isAdditionalXssFlowStep(node1, node2)
290-
}
291-
292190
predicate isBarrierIn(DataFlow::Node node) {
293191
node instanceof DataFlow::SelfParameterNode and
294192
not node.getLocation().getFile() instanceof ErbFile
@@ -309,11 +207,6 @@ module StoredXss {
309207
/** A sanitizer for stored XSS vulnerabilities. */
310208
class Sanitizer = Shared::Sanitizer;
311209

312-
/**
313-
* An additional step that preserves dataflow in the context of stored XSS.
314-
*/
315-
predicate isAdditionalXssTaintStep = Shared::isAdditionalXssFlowStep/2;
316-
317210
private class OrmFieldAsSource extends Source instanceof DataFlow::CallNode {
318211
OrmFieldAsSource() {
319212
exists(DataFlow::CallNode subSrc |

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.