Skip to content

Commit 4792d8f

Browse files
committed
Checkpoint
1 parent 4002b70 commit 4792d8f

File tree

2 files changed

+63
-10
lines changed

2 files changed

+63
-10
lines changed

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -348,13 +348,18 @@ class ResourceBundleGetTextCallArgToReturnValueStep extends DataFlow::SharedFlow
348348
* method of a custom log listener in the same application.
349349
*/
350350
class LogArgumentToListener extends DataFlow::SharedFlowStep {
351-
override predicate step(DataFlow::Node start, DataFlow::Node end) {
351+
deprecated override predicate step(
352+
DataFlow::Node start, DataFlow::Node end, DataFlow::FlowLabel preLabel,
353+
DataFlow::FlowLabel postLabel
354+
) {
352355
inSameWebApp(start.getFile(), end.getFile()) and
353356
start =
354357
ModelOutput::getATypeNode("SapLogger")
355358
.getMember(["debug", "error", "fatal", "info", "trace", "warning"])
356359
.getACall()
357360
.getAnArgument() and
358-
end = ModelOutput::getATypeNode("SapLogEntries").asSource()
361+
end = ModelOutput::getATypeNode("SapLogEntries").asSource() and
362+
preLabel = "logged" and
363+
postLabel = "accessed"
359364
}
360365
}

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

Lines changed: 56 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,16 +13,16 @@
1313

1414
import javascript
1515
import advanced_security.javascript.frameworks.ui5.dataflow.DataFlow
16+
import semmle.javascript.frameworks.data.internal.ApiGraphModels
1617
import advanced_security.javascript.frameworks.ui5.dataflow.DataFlow::UI5PathGraph
18+
import advanced_security.javascript.frameworks.ui5.UI5LogInjectionQuery
1719
import semmle.javascript.security.dataflow.LogInjectionQuery as LogInjection
1820

19-
class UI5LogInjectionConfiguration extends LogInjection::LogInjectionConfiguration {
20-
override predicate isSource(DataFlow::Node node) { node instanceof RemoteFlowSource }
21-
22-
override predicate isSink(DataFlow::Node node) {
21+
class ClientRequestInjectionVector extends DataFlow::Node {
22+
ClientRequestInjectionVector() {
2323
exists(ClientRequest req |
24-
node = req.getUrl() or
25-
node = req.getADataNode()
24+
this = req.getUrl() or
25+
this = req.getADataNode()
2626
)
2727
}
2828
}
@@ -49,6 +49,14 @@ private predicate test(MethodCallNode call, Node receiver, SourceNode receiverSo
4949
receiverSource = receiver.getALocalSource()
5050
}
5151

52+
class SapLogger extends DataFlow::Node {
53+
SapLogger() { this = ModelOutput::getATypeNode("SapLogger").getInducingNode() }
54+
}
55+
56+
class SapLogEntries extends SourceNode {
57+
SapLogEntries() { this = ModelOutput::getATypeNode("SapLogEntries").asSource() }
58+
}
59+
5260
SourceNode isLogListener(TypeBackTracker t) {
5361
t.start() and
5462
exists(UI5Logger log | result = log.getALogListener())
@@ -68,8 +76,48 @@ class LogListener extends DataFlow::Node {
6876
}
6977
}
7078

71-
from
72-
UI5LogInjectionConfiguration cfg, UI5PathNode source, UI5PathNode sink, UI5PathNode primarySource
79+
class UI5LogEntryToHttp extends LogInjection::LogInjectionConfiguration {
80+
override predicate isSource(DataFlow::Node node) { node instanceof RemoteFlowSource }
81+
82+
/*
83+
* !!!!!!!!!! NOTE !!!!!!!!!!
84+
*
85+
* The `DataFlow::FlowLabel` class became deprecated along with the deprecation
86+
* of `DataFlow::Configuration` and `TaintTracking::Configuration`.
87+
*
88+
* There is now no standard library taking advantage of `DataFlow::FlowLabel`
89+
* specifically, so we shouldn't expect our pre-labels and post-labels to
90+
* be propagated along with `LogInjection::Configuration.isAdditionalFlowStep`!
91+
*/
92+
93+
override predicate isAdditionalFlowStep(
94+
DataFlow::Node start, DataFlow::Node end, DataFlow::FlowLabel preLabel,
95+
DataFlow::FlowLabel postLabel
96+
) {
97+
/* 1. From a remote flow source to a logging function. */
98+
exists(UI5LogInjectionConfiguration config |
99+
config.isAdditionalFlowStep(start, end) and
100+
preLabel = "not-logged" and
101+
postLabel = "logged"
102+
)
103+
or
104+
/*
105+
* 2. From a logging function to a log entry: a shared flow step
106+
* `LogArgumentToListener` in FlowSteps.qll, implemented as a
107+
* `DataFlow::SharedFlowStep`.
108+
*/
109+
110+
/*
111+
* 3. From a log entry to an HTTP sending function.
112+
*/
113+
114+
exists() // TODO
115+
}
116+
117+
override predicate isSink(DataFlow::Node node) { node instanceof ClientRequestInjectionVector }
118+
}
119+
120+
from UI5LogEntryToHttp cfg, UI5PathNode source, UI5PathNode sink, UI5PathNode primarySource
73121
where
74122
cfg.hasFlowPath(source.getPathNode(), sink.getPathNode()) and
75123
primarySource = source.getAPrimarySource()

0 commit comments

Comments
 (0)