Skip to content

Commit 77f0d9a

Browse files
jeongsoolee09jeongsoolee09
committed
Checkpoint
1 parent 744be61 commit 77f0d9a

File tree

3 files changed

+73
-75
lines changed

3 files changed

+73
-75
lines changed

‎javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/UI5LogInjectionQuery.qll‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import javascript
22
import advanced_security.javascript.frameworks.ui5.dataflow.DataFlow
3-
import advanced_security.javascript.frameworks.ui5.dataflow.DataFlow::UI5PathGraph
3+
private import advanced_security.javascript.frameworks.ui5.dataflow.DataFlow::UI5PathGraph
44
import semmle.javascript.security.dataflow.LogInjectionQuery as LogInjection
55

66
class UI5LogInjectionConfiguration extends LogInjection::LogInjectionConfiguration {

‎javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/dataflow/FlowSteps.qll‎

Lines changed: 20 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -342,24 +342,23 @@ class ResourceBundleGetTextCallArgToReturnValueStep extends DataFlow::SharedFlow
342342
)
343343
}
344344
}
345-
346-
/**
347-
* A step from any argument of a SAP logging function to the `onLogEntry`
348-
* method of a custom log listener in the same application.
349-
*/
350-
class LogArgumentToListener extends DataFlow::SharedFlowStep {
351-
deprecated override predicate step(
352-
DataFlow::Node start, DataFlow::Node end, DataFlow::FlowLabel preLabel,
353-
DataFlow::FlowLabel postLabel
354-
) {
355-
inSameWebApp(start.getFile(), end.getFile()) and
356-
start =
357-
ModelOutput::getATypeNode("SapLogger")
358-
.getMember(["debug", "error", "fatal", "info", "trace", "warning"])
359-
.getACall()
360-
.getAnArgument() and
361-
end = ModelOutput::getATypeNode("SapLogEntries").asSource() and
362-
preLabel = "logged" and
363-
postLabel = "accessed"
364-
}
365-
}
345+
// /**
346+
// * A step from any argument of a SAP logging function to the `onLogEntry`
347+
// * method of a custom log listener in the same application.
348+
// */
349+
// class LogArgumentToListener extends DataFlow::SharedFlowStep {
350+
// deprecated override predicate step(
351+
// DataFlow::Node start, DataFlow::Node end, DataFlow::FlowLabel preLabel,
352+
// DataFlow::FlowLabel postLabel
353+
// ) {
354+
// inSameWebApp(start.getFile(), end.getFile()) and
355+
// start =
356+
// ModelOutput::getATypeNode("SapLogger")
357+
// .getMember(["debug", "error", "fatal", "info", "trace", "warning"])
358+
// .getACall()
359+
// .getAnArgument() and
360+
// end = ModelOutput::getATypeNode("SapLogEntries").asSource() and
361+
// preLabel = "logged" and
362+
// postLabel = "accessed"
363+
// }
364+
// }

‎javascript/frameworks/ui5/src/UI5LogInjection/UI5LogsToHttp.ql‎

Lines changed: 52 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,7 @@
1414
import javascript
1515
import advanced_security.javascript.frameworks.ui5.dataflow.DataFlow
1616
import semmle.javascript.frameworks.data.internal.ApiGraphModels
17-
import advanced_security.javascript.frameworks.ui5.dataflow.DataFlow::UI5PathGraph
1817
import advanced_security.javascript.frameworks.ui5.UI5LogInjectionQuery
19-
import semmle.javascript.security.dataflow.LogInjectionQuery as LogInjection
2018

2119
class ClientRequestInjectionVector extends DataFlow::Node {
2220
ClientRequestInjectionVector() {
@@ -43,12 +41,6 @@ class UI5Logger extends RequiredObject {
4341
}
4442
}
4543

46-
private predicate test(MethodCallNode call, Node receiver, SourceNode receiverSource) {
47-
call.getMethodName() = "getLogEntries" and
48-
receiver = call.getReceiver() and
49-
receiverSource = receiver.getALocalSource()
50-
}
51-
5244
class SapLogger extends DataFlow::Node {
5345
SapLogger() { this = ModelOutput::getATypeNode("SapLogger").getInducingNode() }
5446
}
@@ -70,67 +62,74 @@ class LogListener extends DataFlow::Node {
7062
LogListener() { this = isLogListener() }
7163

7264
FunctionNode getOnLogEntryMethod() {
73-
exists(DataFlow::PropWrite propWrite | propWrite.getPropertyName() = "onLogEntry" |
74-
result = propWrite.getRhs()
65+
exists(DataFlow::PropWrite onLogEntryProp | onLogEntryProp.getPropertyName() = "onLogEntry" |
66+
result = onLogEntryProp.getRhs()
7567
)
7668
}
7769
}
7870

71+
class UI5LogEntryFlowState extends DataFlow::FlowLabel {
72+
UI5LogEntryFlowState() { this = ["not-logged-not-accessed", "logged-and-accessed"] }
73+
}
74+
7975
class UI5LogEntryToHttp extends TaintTracking::Configuration {
80-
UI5LogEntryToHttp() { this = "UI5 log entries being passed to outbound HTTP requests" }
76+
UI5LogEntryToHttp() { this = "UI5 Log Entry included in an outbound HTTP request" }
8177

82-
override predicate isSource(DataFlow::Node node, DataFlow::FlowLabel label) {
78+
override predicate isSource(DataFlow::Node node, DataFlow::FlowLabel state) {
8379
node instanceof RemoteFlowSource and
84-
label = "not-logged"
80+
state = "not-logged-not-accessed"
8581
}
8682

87-
/*
88-
* !!!!!!!!!! NOTE !!!!!!!!!!
89-
*
90-
* The `DataFlow::FlowLabel` class became deprecated together with
91-
* `DataFlow::Configuration` and `TaintTracking::Configuration`.
92-
*
93-
* There is now no standard library taking advantage of `DataFlow::FlowLabel`
94-
* specifically, so we shouldn't expect our pre-labels and post-labels to
95-
* be propagated along with steps in `LogInjection::Configuration.isAdditionalFlowStep`!
96-
*/
97-
9883
override predicate isAdditionalFlowStep(
99-
DataFlow::Node start, DataFlow::Node end, DataFlow::FlowLabel preLabel,
100-
DataFlow::FlowLabel postLabel
84+
DataFlow::Node start, DataFlow::Node end, DataFlow::FlowLabel preState,
85+
DataFlow::FlowLabel postState
10186
) {
102-
/* 1. From a remote flow source to a logging function. */
103-
exists(UI5LogInjectionConfiguration config |
104-
config.isAdditionalFlowStep(start, end) and
105-
preLabel = "not-logged" and
106-
postLabel = "logged"
107-
)
108-
/*
109-
* 2. From a logging function to a log entry: a shared flow step
110-
* `LogArgumentToListener` in FlowSteps.qll, implemented as a
111-
* `DataFlow::SharedFlowStep`.
112-
*/
113-
114-
/*
115-
* 3. From a log entry to an HTTP sending function.
116-
*/
117-
118-
}
87+
inSameWebApp(start.getFile(), end.getFile()) and
88+
start =
89+
ModelOutput::getATypeNode("SapLogger")
90+
.getMember(["debug", "error", "fatal", "info", "trace", "warning"])
91+
.getACall()
92+
.getAnArgument() and
93+
end = ModelOutput::getATypeNode("SapLogEntries").asSource() and
94+
preState = "not-logged-not-accessed" and
95+
postState = "logged-and-accessed"
96+
}
11997

120-
override predicate isSink(DataFlow::Node node, DataFlow::FlowLabel label) {
98+
override predicate isSink(DataFlow::Node node, DataFlow::FlowLabel state) {
12199
node instanceof ClientRequestInjectionVector and
122-
label = "accessed"
100+
state = "logged-and-accessed"
123101
}
124102
}
125103

126-
from UI5LogEntryToHttp cfg, UI5PathNode source, UI5PathNode sink, UI5PathNode primarySource
127-
where
128-
cfg.hasFlowPath(source.getPathNode(), sink.getPathNode()) and
129-
primarySource = source.getAPrimarySource()
130-
select sink, primarySource, sink, "Outbound network request depends on $@ log data.", primarySource,
104+
/**
105+
* Config without states for sanity check
106+
*/
107+
module UI5LogEntryToHttp implements DataFlow::ConfigSig {
108+
predicate isSource(DataFlow::Node node) { node instanceof RemoteFlowSource }
109+
110+
predicate isAdditionalFlowStep(DataFlow::Node start, DataFlow::Node end) {
111+
inSameWebApp(start.getFile(), end.getFile()) and
112+
start =
113+
ModelOutput::getATypeNode("SapLogger")
114+
.getMember(["debug", "error", "fatal", "info", "trace", "warning"])
115+
.getACall()
116+
.getAnArgument() and
117+
end = ModelOutput::getATypeNode("SapLogEntries").asSource()
118+
}
119+
120+
predicate isSink(DataFlow::Node node) { node instanceof ClientRequestInjectionVector }
121+
}
122+
123+
import DataFlow::PathGraph
124+
125+
// import UI5LogEntryToHttpFlow::PathGraph
126+
module UI5LogEntryToHttpFlow = TaintTracking::Global<UI5LogEntryToHttp>;
127+
128+
from UI5LogEntryToHttp cfg, DataFlow::PathNode source, DataFlow::PathNode sink
129+
where cfg.hasFlowPath(source, sink)
130+
select sink, source, sink, "Outbound network request depends on $@ log data.", source,
131131
"user-provided"
132-
// import DataFlow::PathGraph
133-
// from UI5LogEntryToHttp cfg, DataFlow::PathNode source, DataFlow::PathNode sink
134-
// where cfg.hasFlowPath(source, sink)
132+
// from UI5LogEntryToHttpFlow::PathNode source, UI5LogEntryToHttpFlow::PathNode sink
133+
// where UI5LogEntryToHttpFlow::flowPath(source, sink)
135134
// select sink, source, sink, "Outbound network request depends on $@ log data.", source,
136135
// "user-provided"

0 commit comments

Comments
 (0)