Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions javascript/frameworks/ui5/ext/ui5.model.yml
Original file line number Diff line number Diff line change
Expand Up @@ -62,14 +62,14 @@ extensions:
pack: codeql/javascript-all
extensible: "sourceModel"
data:
- ["UI5Control", "Member[value]", "ui5-remote"]
- ["UI5Control", "Member[getValue].ReturnValue", "ui5-remote"]
- ["UI5CodeEditor", "Member[value]", "ui5-remote"]
- ["UI5CodeEditor", "Member[getCurrentValue].ReturnValue", "ui5-remote"]
- ["global", "Member[jQuery].Member[sap].Member[getUriParameters].ReturnValue.Member[get].ReturnValue", "ui5-remote"]
- ["global", "Member[jQuery].Member[sap].Member[syncHead,syncGet,syncGetText,syncPost,syncPostText].ReturnValue", "ui5-remote"]
- ["UI5URIParameters", "Member[get].ReturnValue", "ui5-remote"]
- ["UI5URIParameters", "Member[getAll].ReturnValue", "ui5-remote"]
- ["UI5Control", "Member[value]", "remote"]
- ["UI5Control", "Member[getValue].ReturnValue", "remote"]
- ["UI5CodeEditor", "Member[value]", "remote"]
- ["UI5CodeEditor", "Member[getCurrentValue].ReturnValue", "remote"]
- ["global", "Member[jQuery].Member[sap].Member[getUriParameters].ReturnValue.Member[get].ReturnValue", "remote"]
- ["global", "Member[jQuery].Member[sap].Member[syncHead,syncGet,syncGetText,syncPost,syncPostText].ReturnValue", "remote"]
- ["UI5URIParameters", "Member[get].ReturnValue", "remote"]
- ["UI5URIParameters", "Member[getAll].ReturnValue", "remote"]

- addsTo:
pack: codeql/javascript-all
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,12 @@ module UI5DataFlow {
/**
* An remote source associated with a `UI5BoundNode`
*/
class UI5ModelSource extends UI5DataFlow::UI5BoundNode {
class UI5ModelSource extends UI5DataFlow::UI5BoundNode, RemoteFlowSource {
UI5ModelSource() { bindingPath = any(UI5View view).getASource() }

override string getSourceType() {
result = "UI5 model remote flow source"
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ class JsView extends UI5View {
exists(ObjectExpr control, string type, string path, string property |
this = control.getFile() and
type = result.getControlTypeName() and
ApiGraphModelsExtensions::sourceModel(getASuperType(type), path, "ui5-remote") and
ApiGraphModelsExtensions::sourceModel(getASuperType(type), path, "remote") and
property = path.replaceAll(" ", "").regexpCapture("Member\\[([^\\]]+)\\]", 1) and
result = control.getPropertyByName(property)
)
Expand Down Expand Up @@ -245,7 +245,7 @@ class JsonView extends UI5View {
exists(JsonObject control, string type, string path, string property |
root = control.getParent+() and
type = result.getControlTypeName() and
ApiGraphModelsExtensions::sourceModel(getASuperType(type), path, "ui5-remote") and
ApiGraphModelsExtensions::sourceModel(getASuperType(type), path, "remote") and
property = path.replaceAll(" ", "").regexpCapture("Member\\[([^\\]]+)\\]", 1) and
result = control.getPropValue(property)
)
Expand Down Expand Up @@ -341,7 +341,7 @@ class HtmlView extends UI5View, HTML::HtmlFile {
exists(HTML::Element control, string type, string path, string property |
this = control.getFile() and
type = result.getControlTypeName() and
ApiGraphModelsExtensions::sourceModel(getASuperType(type), path, "ui5-remote") and
ApiGraphModelsExtensions::sourceModel(getASuperType(type), path, "remote") and
property = path.replaceAll(" ", "").regexpCapture("Member\\[([^\\]]+)\\]", 1) and
result = control.getAttributeByName("data-" + property)
)
Expand Down Expand Up @@ -417,7 +417,7 @@ class XmlView extends UI5View, XmlFile {
exists(XmlElement control, string type, string path, string property |
this = control.getFile() and
type = result.getControlTypeName() and
ApiGraphModelsExtensions::sourceModel(getASuperType(type), path, "ui5-remote") and
ApiGraphModelsExtensions::sourceModel(getASuperType(type), path, "remote") and
property = path.replaceAll(" ", "").regexpCapture("Member\\[([^\\]]+)\\]", 1) and
result = control.getAttribute(property)
)
Expand Down
18 changes: 3 additions & 15 deletions javascript/frameworks/ui5/src/UI5Xss/UI5Xss.ql
Original file line number Diff line number Diff line change
Expand Up @@ -50,29 +50,17 @@ class UI5XssConfiguration extends DomBasedXss::Configuration {
}
}

/**
* An remote source associated with a `UI5BoundNode`
*/
class UI5ModelSource extends UI5DataFlow::UI5ModelSource, DomBasedXss::Source { }

/**
* An html injection sink associated with a `UI5BoundNode`
*/
class UI5ModelHtmlISink extends UI5DataFlow::UI5ModelHtmlISink, DomBasedXss::Sink { }

// Sources and Sinks from data-extensions
class UI5ExtSource extends DomBasedXss::Source {
UI5ExtSource() { this = ModelOutput::getASourceNode("ui5-remote").asSource() }
}

class UI5ExtHtmlISink extends DomBasedXss::Sink {
UI5ExtHtmlISink() { this = ModelOutput::getASinkNode("ui5-html-injection").asSink() }
}

// XSS source or sinks that are ui5-specific
private predicate isUI5Specific(UI5PathGraph::UI5PathNode source, UI5PathGraph::UI5PathNode sink) {
source.asDataFlowPathNode().getNode() instanceof UI5ExtSource or
source.asDataFlowPathNode().getNode() instanceof UI5ModelSource or
private predicate isUI5Specific(UI5PathGraph::UI5PathNode sink) {
sink.asDataFlowPathNode().getNode() instanceof UI5ModelHtmlISink or
sink.asDataFlowPathNode().getNode() instanceof UI5ExtHtmlISink
}
Expand All @@ -84,7 +72,7 @@ where
cfg.hasFlowPath(source.asDataFlowPathNode(), sink.asDataFlowPathNode()) and
primarySource = source.getAPrimarySource() and
primarySink = sink.getAPrimaryHtmlISink() and
// source or sink are ui5-specific
isUI5Specific(source, sink)
// sink are ui5-specific
isUI5Specific(sink)
select primarySink, primarySource, primarySink, "XSS vulnerability due to $@.", primarySource,
"user-provided value"
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
category: majorAnalysis
---

* Turn UI5 specific sources into generic sources (i.e., they are considered remote flow sources as modelled by `RemoteFlowSource`).
56 changes: 28 additions & 28 deletions javascript/frameworks/ui5/test/models/source/sourceTest.expected
Original file line number Diff line number Diff line change
@@ -1,28 +1,28 @@
| source.js:25:17:25:25 | obj.value | obj.value |
| source.js:27:17:27:30 | obj.getValue() | obj.getValue() |
| source.js:29:17:29:25 | obj.value | obj.value |
| source.js:31:17:31:30 | obj.getValue() | obj.getValue() |
| source.js:33:17:33:25 | obj.value | obj.value |
| source.js:35:17:35:30 | obj.getValue() | obj.getValue() |
| source.js:37:17:37:25 | obj.value | obj.value |
| source.js:39:17:39:30 | obj.getValue() | obj.getValue() |
| source.js:41:17:41:25 | obj.value | obj.value |
| source.js:43:17:43:30 | obj.getValue() | obj.getValue() |
| source.js:45:17:45:25 | obj.value | obj.value |
| source.js:47:17:47:30 | obj.getValue() | obj.getValue() |
| source.js:49:17:49:25 | obj.value | obj.value |
| source.js:51:17:51:37 | obj.get ... Value() | obj.get ... Value() |
| source.js:53:17:53:25 | obj.value | obj.value |
| source.js:55:17:55:30 | obj.getValue() | obj.getValue() |
| source.js:57:17:57:51 | jQuery. ... ).get() | jQuery. ... ).get() |
| source.js:59:17:59:37 | jQuery. ... cHead() | jQuery. ... cHead() |
| source.js:61:17:61:36 | jQuery.sap.syncGet() | jQuery.sap.syncGet() |
| source.js:63:17:63:40 | jQuery. ... tText() | jQuery. ... tText() |
| source.js:65:17:65:37 | jQuery. ... cPost() | jQuery. ... cPost() |
| source.js:67:17:67:41 | jQuery. ... tText() | jQuery. ... tText() |
| source.js:69:17:69:52 | UriPara ... ).get() | UriPara ... ).get() |
| source.js:70:17:70:55 | UriPara ... etAll() | UriPara ... etAll() |
| source.js:73:17:73:25 | obj.get() | obj.get() |
| source.js:74:17:74:28 | obj.getAll() | obj.getAll() |
| source.js:76:17:76:28 | obj.getAll() | obj.getAll() |
| source.js:78:17:78:25 | obj.get() | obj.get() |
| source.js:25:17:25:25 | obj.value | Remote flow source of type: Remote flow |
| source.js:27:17:27:30 | obj.getValue() | Remote flow source of type: Remote flow |
| source.js:29:17:29:25 | obj.value | Remote flow source of type: Remote flow |
| source.js:31:17:31:30 | obj.getValue() | Remote flow source of type: Remote flow |
| source.js:33:17:33:25 | obj.value | Remote flow source of type: Remote flow |
| source.js:35:17:35:30 | obj.getValue() | Remote flow source of type: Remote flow |
| source.js:37:17:37:25 | obj.value | Remote flow source of type: Remote flow |
| source.js:39:17:39:30 | obj.getValue() | Remote flow source of type: Remote flow |
| source.js:41:17:41:25 | obj.value | Remote flow source of type: Remote flow |
| source.js:43:17:43:30 | obj.getValue() | Remote flow source of type: Remote flow |
| source.js:45:17:45:25 | obj.value | Remote flow source of type: Remote flow |
| source.js:47:17:47:30 | obj.getValue() | Remote flow source of type: Remote flow |
| source.js:49:17:49:25 | obj.value | Remote flow source of type: Remote flow |
| source.js:51:17:51:37 | obj.get ... Value() | Remote flow source of type: Remote flow |
| source.js:53:17:53:25 | obj.value | Remote flow source of type: Remote flow |
| source.js:55:17:55:30 | obj.getValue() | Remote flow source of type: Remote flow |
| source.js:57:17:57:51 | jQuery. ... ).get() | Remote flow source of type: Remote flow |
| source.js:59:17:59:37 | jQuery. ... cHead() | Remote flow source of type: Remote flow |
| source.js:61:17:61:36 | jQuery.sap.syncGet() | Remote flow source of type: Remote flow |
| source.js:63:17:63:40 | jQuery. ... tText() | Remote flow source of type: Remote flow |
| source.js:65:17:65:37 | jQuery. ... cPost() | Remote flow source of type: Remote flow |
| source.js:67:17:67:41 | jQuery. ... tText() | Remote flow source of type: Remote flow |
| source.js:69:17:69:52 | UriPara ... ).get() | Remote flow source of type: Remote flow |
| source.js:70:17:70:55 | UriPara ... etAll() | Remote flow source of type: Remote flow |
| source.js:73:17:73:25 | obj.get() | Remote flow source of type: Remote flow |
| source.js:74:17:74:28 | obj.getAll() | Remote flow source of type: Remote flow |
| source.js:76:17:76:28 | obj.getAll() | Remote flow source of type: Remote flow |
| source.js:78:17:78:25 | obj.get() | Remote flow source of type: Remote flow |
17 changes: 0 additions & 17 deletions javascript/frameworks/ui5/test/models/source/sourceTest.ql

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Diagnostics/ListRemoteFlowSources.ql
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
nodes
| LogInjectionTest.js:6:9:6:50 | value |
| LogInjectionTest.js:6:17:6:50 | jQuery. ... param") |
| LogInjectionTest.js:6:17:6:50 | jQuery. ... param") |
| LogInjectionTest.js:7:18:7:40 | `[INFO] ... value}` |
| LogInjectionTest.js:7:18:7:40 | `[INFO] ... value}` |
| LogInjectionTest.js:7:34:7:38 | value |
| LogInjectionTest.js:13:9:13:36 | q |
| LogInjectionTest.js:13:13:13:36 | url.par ... , true) |
| LogInjectionTest.js:13:23:13:29 | req.url |
Expand All @@ -25,6 +31,11 @@ nodes
| LogInjectionTest.js:24:18:24:41 | `[INFO] ... alue1}` |
| LogInjectionTest.js:24:34:24:39 | value1 |
edges
| LogInjectionTest.js:6:9:6:50 | value | LogInjectionTest.js:7:34:7:38 | value |
| LogInjectionTest.js:6:17:6:50 | jQuery. ... param") | LogInjectionTest.js:6:9:6:50 | value |
| LogInjectionTest.js:6:17:6:50 | jQuery. ... param") | LogInjectionTest.js:6:9:6:50 | value |
| LogInjectionTest.js:7:34:7:38 | value | LogInjectionTest.js:7:18:7:40 | `[INFO] ... value}` |
| LogInjectionTest.js:7:34:7:38 | value | LogInjectionTest.js:7:18:7:40 | `[INFO] ... value}` |
| LogInjectionTest.js:13:9:13:36 | q | LogInjectionTest.js:14:17:14:17 | q |
| LogInjectionTest.js:13:13:13:36 | url.par ... , true) | LogInjectionTest.js:13:9:13:36 | q |
| LogInjectionTest.js:13:23:13:29 | req.url | LogInjectionTest.js:13:13:13:36 | url.par ... , true) |
Expand All @@ -49,5 +60,6 @@ edges
| LogInjectionTest.js:24:34:24:39 | value1 | LogInjectionTest.js:24:18:24:41 | `[INFO] ... alue1}` |
| LogInjectionTest.js:24:34:24:39 | value1 | LogInjectionTest.js:24:18:24:41 | `[INFO] ... alue1}` |
#select
| LogInjectionTest.js:7:18:7:40 | `[INFO] ... value}` | LogInjectionTest.js:6:17:6:50 | jQuery. ... param") | LogInjectionTest.js:7:18:7:40 | `[INFO] ... value}` | Log entry depends on a $@. | LogInjectionTest.js:6:17:6:50 | jQuery. ... param") | user-provided value |
| LogInjectionTest.js:15:18:15:40 | `[INFO] ... value}` | LogInjectionTest.js:13:23:13:29 | req.url | LogInjectionTest.js:15:18:15:40 | `[INFO] ... value}` | Log entry depends on a $@. | LogInjectionTest.js:13:23:13:29 | req.url | user-provided value |
| LogInjectionTest.js:24:18:24:41 | `[INFO] ... alue1}` | LogInjectionTest.js:21:23:21:29 | req.url | LogInjectionTest.js:24:18:24:41 | `[INFO] ... alue1}` | Log entry depends on a $@. | LogInjectionTest.js:21:23:21:29 | req.url | user-provided value |
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const url = require('url');
// source is ui5-specific
function ui5loginjectionXss() {
let value = jQuery.sap.syncGet("url", "param")
console.info(`[INFO] User: ${value}`); //UI5 log-injection
console.info(`[INFO] User: ${value}`); // log-injection

Check failure

Code scanning / CodeQL

Log injection

Log entry depends on a [user-provided value](1).
jQuery.sap.log.debug(value); //UI5 log-injection
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ edges
| LogInjectionTest.js:23:39:23:43 | value | LogInjectionTest.js:23:18:23:44 | jQuery. ... (value) |
| LogInjectionTest.js:24:34:24:39 | value1 | LogInjectionTest.js:24:18:24:41 | `[INFO] ... alue1}` |
#select
| LogInjectionTest.js:7:18:7:40 | `[INFO] ... value}` | LogInjectionTest.js:6:17:6:50 | jQuery. ... param") | LogInjectionTest.js:7:18:7:40 | `[INFO] ... value}` | Log entry depends on a $@. | LogInjectionTest.js:6:17:6:50 | jQuery. ... param") | user-provided value |
| LogInjectionTest.js:8:26:8:30 | value | LogInjectionTest.js:6:17:6:50 | jQuery. ... param") | LogInjectionTest.js:8:26:8:30 | value | Log entry depends on a $@. | LogInjectionTest.js:6:17:6:50 | jQuery. ... param") | user-provided value |
| LogInjectionTest.js:16:26:16:30 | value | LogInjectionTest.js:13:23:13:29 | req.url | LogInjectionTest.js:16:26:16:30 | value | Log entry depends on a $@. | LogInjectionTest.js:13:23:13:29 | req.url | user-provided value |
| LogInjectionTest.js:25:26:25:31 | value1 | LogInjectionTest.js:21:23:21:29 | req.url | LogInjectionTest.js:25:26:25:31 | value1 | Log entry depends on a $@. | LogInjectionTest.js:21:23:21:29 | req.url | user-provided value |
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ edges
| XssTest.js:18:18:18:44 | jQuery. ... (value) | XssTest.js:18:9:18:44 | value1 |
| XssTest.js:18:39:18:43 | value | XssTest.js:18:18:18:44 | jQuery. ... (value) |
#select
| XssTest.js:4:20:4:24 | value | XssTest.js:3:17:3:50 | jQuery. ... param") | XssTest.js:4:20:4:24 | value | XSS vulnerability due to $@. | XssTest.js:3:17:3:50 | jQuery. ... param") | user-provided value |
| XssTest.js:5:27:5:31 | value | XssTest.js:3:17:3:50 | jQuery. ... param") | XssTest.js:5:27:5:31 | value | XSS vulnerability due to $@. | XssTest.js:3:17:3:50 | jQuery. ... param") | user-provided value |
| XssTest.js:12:27:12:31 | value | XssTest.js:10:17:10:40 | documen ... .search | XssTest.js:12:27:12:31 | value | XSS vulnerability due to $@. | XssTest.js:10:17:10:40 | documen ... .search | user-provided value |
| XssTest.js:20:27:20:32 | value1 | XssTest.js:17:17:17:40 | documen ... .search | XssTest.js:20:27:20:32 | value1 | XSS vulnerability due to $@. | XssTest.js:17:17:17:40 | documen ... .search | user-provided value |
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
nodes
| XssTest.js:3:9:3:50 | value |
| XssTest.js:3:9:3:50 | value |
| XssTest.js:3:17:3:50 | jQuery. ... param") |
| XssTest.js:3:17:3:50 | jQuery. ... param") |
| XssTest.js:3:17:3:50 | jQuery. ... param") |
| XssTest.js:4:20:4:24 | value |
| XssTest.js:4:20:4:24 | value |
| XssTest.js:4:20:4:24 | value |
| XssTest.js:10:9:10:40 | value |
| XssTest.js:10:17:10:40 | documen ... .search |
| XssTest.js:10:17:10:40 | documen ... .search |
Expand All @@ -21,6 +29,14 @@ nodes
| XssTest.js:27:20:27:25 | value1 |
| XssTest.js:27:20:27:25 | value1 |
edges
| XssTest.js:3:9:3:50 | value | XssTest.js:4:20:4:24 | value |
| XssTest.js:3:9:3:50 | value | XssTest.js:4:20:4:24 | value |
| XssTest.js:3:9:3:50 | value | XssTest.js:4:20:4:24 | value |
| XssTest.js:3:9:3:50 | value | XssTest.js:4:20:4:24 | value |
| XssTest.js:3:17:3:50 | jQuery. ... param") | XssTest.js:3:9:3:50 | value |
| XssTest.js:3:17:3:50 | jQuery. ... param") | XssTest.js:3:9:3:50 | value |
| XssTest.js:3:17:3:50 | jQuery. ... param") | XssTest.js:3:9:3:50 | value |
| XssTest.js:3:17:3:50 | jQuery. ... param") | XssTest.js:3:9:3:50 | value |
| XssTest.js:10:9:10:40 | value | XssTest.js:11:20:11:24 | value |
| XssTest.js:10:9:10:40 | value | XssTest.js:11:20:11:24 | value |
| XssTest.js:10:17:10:40 | documen ... .search | XssTest.js:10:9:10:40 | value |
Expand All @@ -40,6 +56,7 @@ edges
| XssTest.js:26:18:26:44 | jQuery. ... (value) | XssTest.js:26:9:26:44 | value1 |
| XssTest.js:26:39:26:43 | value | XssTest.js:26:18:26:44 | jQuery. ... (value) |
#select
| XssTest.js:4:20:4:24 | value | XssTest.js:3:17:3:50 | jQuery. ... param") | XssTest.js:4:20:4:24 | value | Cross-site scripting vulnerability due to $@. | XssTest.js:3:17:3:50 | jQuery. ... param") | user-provided value |
| XssTest.js:11:20:11:24 | value | XssTest.js:10:17:10:40 | documen ... .search | XssTest.js:11:20:11:24 | value | Cross-site scripting vulnerability due to $@. | XssTest.js:10:17:10:40 | documen ... .search | user-provided value |
| XssTest.js:19:20:19:25 | value1 | XssTest.js:17:17:17:40 | documen ... .search | XssTest.js:19:20:19:25 | value1 | Cross-site scripting vulnerability due to $@. | XssTest.js:17:17:17:40 | documen ... .search | user-provided value |
| XssTest.js:27:20:27:25 | value1 | XssTest.js:25:17:25:40 | documen ... .search | XssTest.js:27:20:27:25 | value1 | Cross-site scripting vulnerability due to $@. | XssTest.js:25:17:25:40 | documen ... .search | user-provided value |
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// source is ui5-specific
function testXss1() {
var value = jQuery.sap.syncGet("url", "param")
$('myId').html(value) //UI5 Xss
$('myId').html(value) // Xss

Check warning

Code scanning / CodeQL

Client-side cross-site scripting

Cross-site scripting vulnerability due to [user-provided value](1).
jQuery.sap.globalEval(value); //UI5 Xss
};

Expand Down