From 1c40a3b30dac00202e601f5236db18eb1c61b14a Mon Sep 17 00:00:00 2001 From: Remco Vermeulen Date: Fri, 6 Oct 2023 15:48:12 -0700 Subject: [PATCH 1/3] Add diagnostic queries These list: - Remote flow source. - UI5 specific sinks for XSS. - UI5 specific sinks for log injection. These help with determining if we are missing any for a given codebase. --- .../ui5/src/Diagnostics/ListHtmlSinks.ql | 21 +++++++++++++++++++ .../src/Diagnostics/ListLogInjectionSinks.ql | 17 +++++++++++++++ .../src/Diagnostics/ListRemoteFlowSources.ql | 15 +++++++++++++ 3 files changed, 53 insertions(+) create mode 100644 javascript/frameworks/ui5/src/Diagnostics/ListHtmlSinks.ql create mode 100644 javascript/frameworks/ui5/src/Diagnostics/ListLogInjectionSinks.ql create mode 100644 javascript/frameworks/ui5/src/Diagnostics/ListRemoteFlowSources.ql diff --git a/javascript/frameworks/ui5/src/Diagnostics/ListHtmlSinks.ql b/javascript/frameworks/ui5/src/Diagnostics/ListHtmlSinks.ql new file mode 100644 index 000000000..f4de0c51b --- /dev/null +++ b/javascript/frameworks/ui5/src/Diagnostics/ListHtmlSinks.ql @@ -0,0 +1,21 @@ +/** + * @name SAP UI5 Html injection sinks + * @description List all SAP UI5 Html injection sinks + * @kind problem + * @problem.severity info + * @precision high + * @id js/ui5-list-html-injection-sinks + * @tags diagnostics + */ + + import javascript + import advanced_security.javascript.frameworks.ui5.UI5DataFlow + + from DataFlow::Node sink, string kind + where + sink = ModelOutput::getASinkNode(kind).asSink() and + kind = "ui5-html-injection" + or + sink instanceof UI5DataFlow::UI5ModelHtmlISink and + kind = "ui5-model-sink" +select sink, "SAP UI5 Html injection sink with kind: " + kind \ No newline at end of file diff --git a/javascript/frameworks/ui5/src/Diagnostics/ListLogInjectionSinks.ql b/javascript/frameworks/ui5/src/Diagnostics/ListLogInjectionSinks.ql new file mode 100644 index 000000000..1fbb4fcab --- /dev/null +++ b/javascript/frameworks/ui5/src/Diagnostics/ListLogInjectionSinks.ql @@ -0,0 +1,17 @@ +/** + * @name SAP UI5 log injection sinks + * @description List all SAP UI5 log injection sinks + * @kind problem + * @problem.severity info + * @precision high + * @id js/ui5-list-log-injection-sinks + * @tags diagnostics + */ + +import javascript + +from DataFlow::Node sink, string kind +where + sink = ModelOutput::getASinkNode(kind).asSink() and + kind = "ui5-log-injection" +select sink, "SAP UI5 log injection sink with kind: " + kind diff --git a/javascript/frameworks/ui5/src/Diagnostics/ListRemoteFlowSources.ql b/javascript/frameworks/ui5/src/Diagnostics/ListRemoteFlowSources.ql new file mode 100644 index 000000000..90432ac82 --- /dev/null +++ b/javascript/frameworks/ui5/src/Diagnostics/ListRemoteFlowSources.ql @@ -0,0 +1,15 @@ +/** + * @name List all remote sources + * @description List all remote sources + * @kind problem + * @problem.severity info + * @precision high + * @id js/ui5-list-remote-flow-sources + * @tags diagnostics + */ + +import javascript + +from RemoteFlowSource source, string type +where type = source.getSourceType() +select source, "Remote flow source of type: " + type \ No newline at end of file From 7bb909e9639ec9d15b21ab17cb4b6d9c22afa53f Mon Sep 17 00:00:00 2001 From: Remco Vermeulen Date: Fri, 6 Oct 2023 15:50:10 -0700 Subject: [PATCH 2/3] Add codeql suites and set default suite Because the pack now contains diagnostic queries we want to by default only execute the security queries. The suites follow the standard library convention where the default suite is suited for running in code scanning. This means only security queries with precision high or very high. Currently this excludes the log injection query because its precision is medium. The security extended suite includes the log injection query. --- .../src/codeql-suites/sap-ui5-code-scanning.qls | 13 +++++++++++++ .../ui5/src/codeql-suites/sap-ui5-diagnostics.qls | 4 ++++ .../codeql-suites/sap-ui5-security-extended.qls | 14 ++++++++++++++ javascript/frameworks/ui5/src/qlpack.yml | 1 + 4 files changed, 32 insertions(+) create mode 100644 javascript/frameworks/ui5/src/codeql-suites/sap-ui5-code-scanning.qls create mode 100644 javascript/frameworks/ui5/src/codeql-suites/sap-ui5-diagnostics.qls create mode 100644 javascript/frameworks/ui5/src/codeql-suites/sap-ui5-security-extended.qls diff --git a/javascript/frameworks/ui5/src/codeql-suites/sap-ui5-code-scanning.qls b/javascript/frameworks/ui5/src/codeql-suites/sap-ui5-code-scanning.qls new file mode 100644 index 000000000..b7cd75559 --- /dev/null +++ b/javascript/frameworks/ui5/src/codeql-suites/sap-ui5-code-scanning.qls @@ -0,0 +1,13 @@ +- description: SAP UI5 Code Scanning Suite +- queries: . +- include: + tags contain: security + kind: + - problem + - path-problem + precision: + - high + - very-high + problem.severity: + - warning + - error \ No newline at end of file diff --git a/javascript/frameworks/ui5/src/codeql-suites/sap-ui5-diagnostics.qls b/javascript/frameworks/ui5/src/codeql-suites/sap-ui5-diagnostics.qls new file mode 100644 index 000000000..a28a11e35 --- /dev/null +++ b/javascript/frameworks/ui5/src/codeql-suites/sap-ui5-diagnostics.qls @@ -0,0 +1,4 @@ +- description: SAP UI5 Code Scanning Suite +- queries: . +- include: + tags contain: diagnostics \ No newline at end of file diff --git a/javascript/frameworks/ui5/src/codeql-suites/sap-ui5-security-extended.qls b/javascript/frameworks/ui5/src/codeql-suites/sap-ui5-security-extended.qls new file mode 100644 index 000000000..d51295c2d --- /dev/null +++ b/javascript/frameworks/ui5/src/codeql-suites/sap-ui5-security-extended.qls @@ -0,0 +1,14 @@ +- description: SAP UI5 Security Extended Suite +- queries: . +- include: + tags contain: security + kind: + - problem + - path-problem + precision: + - medium + - high + - very-high + problem.severity: + - warning + - error \ No newline at end of file diff --git a/javascript/frameworks/ui5/src/qlpack.yml b/javascript/frameworks/ui5/src/qlpack.yml index c4c8c4ab7..1fc56c383 100644 --- a/javascript/frameworks/ui5/src/qlpack.yml +++ b/javascript/frameworks/ui5/src/qlpack.yml @@ -8,3 +8,4 @@ dependencies: codeql/javascript-all: "^0.6.3" advanced-security/javascript-sap-ui5-models: "^0.2.0" advanced-security/javascript-sap-ui5-all: "^0.2.0" +default-suite-file: codeql-suites/sap-ui5-code-scanning.qls From 53855a004d5ea12b187b9078aeb54444dbdb7366 Mon Sep 17 00:00:00 2001 From: Remco Vermeulen Date: Fri, 6 Oct 2023 16:25:22 -0700 Subject: [PATCH 3/3] Replace log sink test with diagnostic query --- .../src/Diagnostics/ListRemoteFlowSources.ql | 1 + .../ui5/test/models/sink/logSinkTest.expected | 130 ++++++++---------- .../ui5/test/models/sink/logSinkTest.ql | 17 --- .../ui5/test/models/sink/logSinkTest.qlref | 1 + 4 files changed, 58 insertions(+), 91 deletions(-) delete mode 100644 javascript/frameworks/ui5/test/models/sink/logSinkTest.ql create mode 100644 javascript/frameworks/ui5/test/models/sink/logSinkTest.qlref diff --git a/javascript/frameworks/ui5/src/Diagnostics/ListRemoteFlowSources.ql b/javascript/frameworks/ui5/src/Diagnostics/ListRemoteFlowSources.ql index 90432ac82..a433d30cd 100644 --- a/javascript/frameworks/ui5/src/Diagnostics/ListRemoteFlowSources.ql +++ b/javascript/frameworks/ui5/src/Diagnostics/ListRemoteFlowSources.ql @@ -9,6 +9,7 @@ */ import javascript +import advanced_security.javascript.frameworks.ui5.UI5DataFlow from RemoteFlowSource source, string type where type = source.getSourceType() diff --git a/javascript/frameworks/ui5/test/models/sink/logSinkTest.expected b/javascript/frameworks/ui5/test/models/sink/logSinkTest.expected index a5f81aeb6..d96f4a032 100644 --- a/javascript/frameworks/ui5/test/models/sink/logSinkTest.expected +++ b/javascript/frameworks/ui5/test/models/sink/logSinkTest.expected @@ -1,74 +1,56 @@ -| sink.js:20:38:20:42 | code0 | code0 | -| sink.js:20:45:20:49 | code1 | code1 | -| sink.js:20:52:20:56 | code2 | code2 | -| sink.js:21:38:21:42 | code0 | code0 | -| sink.js:21:45:21:49 | code1 | code1 | -| sink.js:21:52:21:56 | code2 | code2 | -| sink.js:23:40:23:44 | code0 | code0 | -| sink.js:23:47:23:51 | code1 | code1 | -| sink.js:23:54:23:58 | code2 | code2 | -| sink.js:25:37:25:41 | code0 | code0 | -| sink.js:25:44:25:48 | code1 | code1 | -| sink.js:25:51:25:55 | code2 | code2 | -| sink.js:27:38:27:42 | code0 | code0 | -| sink.js:27:45:27:49 | code1 | code1 | -| sink.js:27:52:27:56 | code2 | code2 | -| sink.js:29:38:29:42 | code0 | code0 | -| sink.js:29:45:29:49 | code1 | code1 | -| sink.js:29:52:29:56 | code2 | code2 | -| sink.js:33:27:33:31 | code0 | code0 | -| sink.js:33:34:33:38 | code1 | code1 | -| sink.js:33:41:33:45 | code2 | code2 | -| sink.js:35:27:35:31 | code0 | code0 | -| sink.js:35:34:35:38 | code1 | code1 | -| sink.js:35:41:35:45 | code2 | code2 | -| sink.js:37:29:37:33 | code0 | code0 | -| sink.js:37:36:37:40 | code1 | code1 | -| sink.js:37:43:37:47 | code2 | code2 | -| sink.js:39:26:39:30 | code0 | code0 | -| sink.js:39:33:39:37 | code1 | code1 | -| sink.js:39:40:39:44 | code2 | code2 | -| sink.js:41:27:41:31 | code0 | code0 | -| sink.js:41:34:41:38 | code1 | code1 | -| sink.js:41:41:41:45 | code2 | code2 | -| sink.js:43:27:43:31 | code0 | code0 | -| sink.js:43:34:43:38 | code1 | code1 | -| sink.js:43:41:43:45 | code2 | code2 | -| sink.js:45:42:45:46 | code1 | code1 | -| sink.js:74:36:74:40 | code0 | code0 | -| sink.js:74:43:74:47 | code1 | code1 | -| sink.js:74:50:74:54 | code2 | code2 | -| sink.js:75:36:75:40 | code0 | code0 | -| sink.js:75:43:75:47 | code1 | code1 | -| sink.js:75:50:75:54 | code2 | code2 | -| sink.js:76:38:76:42 | code0 | code0 | -| sink.js:76:45:76:49 | code1 | code1 | -| sink.js:76:52:76:56 | code2 | code2 | -| sink.js:77:35:77:39 | code0 | code0 | -| sink.js:77:42:77:46 | code1 | code1 | -| sink.js:77:49:77:53 | code2 | code2 | -| sink.js:78:36:78:40 | code0 | code0 | -| sink.js:78:43:78:47 | code1 | code1 | -| sink.js:78:50:78:54 | code2 | code2 | -| sink.js:79:36:79:40 | code0 | code0 | -| sink.js:79:43:79:47 | code1 | code1 | -| sink.js:79:50:79:54 | code2 | code2 | -| sink.js:80:27:80:31 | code0 | code0 | -| sink.js:80:34:80:38 | code1 | code1 | -| sink.js:80:41:80:45 | code2 | code2 | -| sink.js:81:27:81:31 | code0 | code0 | -| sink.js:81:34:81:38 | code1 | code1 | -| sink.js:81:41:81:45 | code2 | code2 | -| sink.js:82:29:82:33 | code0 | code0 | -| sink.js:82:36:82:40 | code1 | code1 | -| sink.js:82:43:82:47 | code2 | code2 | -| sink.js:83:26:83:30 | code0 | code0 | -| sink.js:83:33:83:37 | code1 | code1 | -| sink.js:83:40:83:44 | code2 | code2 | -| sink.js:84:27:84:31 | code0 | code0 | -| sink.js:84:34:84:38 | code1 | code1 | -| sink.js:84:41:84:45 | code2 | code2 | -| sink.js:85:27:85:31 | code0 | code0 | -| sink.js:85:34:85:38 | code1 | code1 | -| sink.js:85:41:85:45 | code2 | code2 | -| sink.js:86:40:86:44 | code1 | code1 | +| sink.js:20:38:20:42 | code0 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:20:45:20:49 | code1 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:20:52:20:56 | code2 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:21:38:21:42 | code0 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:21:45:21:49 | code1 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:21:52:21:56 | code2 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:23:40:23:44 | code0 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:23:47:23:51 | code1 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:23:54:23:58 | code2 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:25:37:25:41 | code0 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:25:44:25:48 | code1 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:25:51:25:55 | code2 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:27:38:27:42 | code0 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:27:45:27:49 | code1 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:27:52:27:56 | code2 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:29:38:29:42 | code0 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:29:45:29:49 | code1 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:29:52:29:56 | code2 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:33:27:33:31 | code0 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:33:34:33:38 | code1 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:33:41:33:45 | code2 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:35:27:35:31 | code0 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:35:34:35:38 | code1 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:35:41:35:45 | code2 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:37:29:37:33 | code0 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:37:36:37:40 | code1 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:37:43:37:47 | code2 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:39:26:39:30 | code0 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:39:33:39:37 | code1 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:39:40:39:44 | code2 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:41:27:41:31 | code0 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:41:34:41:38 | code1 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:41:41:41:45 | code2 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:43:27:43:31 | code0 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:43:34:43:38 | code1 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:43:41:43:45 | code2 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:45:42:45:46 | code1 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:74:36:74:40 | code0 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:74:43:74:47 | code1 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:74:50:74:54 | code2 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:75:36:75:40 | code0 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:75:43:75:47 | code1 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:75:50:75:54 | code2 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:76:38:76:42 | code0 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:76:45:76:49 | code1 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:76:52:76:56 | code2 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:77:35:77:39 | code0 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:77:42:77:46 | code1 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:77:49:77:53 | code2 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:78:36:78:40 | code0 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:78:43:78:47 | code1 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:78:50:78:54 | code2 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:79:36:79:40 | code0 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:79:43:79:47 | code1 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:79:50:79:54 | code2 | SAP UI5 log injection sink with kind: ui5-log-injection | +| sink.js:86:40:86:44 | code1 | SAP UI5 log injection sink with kind: ui5-log-injection | diff --git a/javascript/frameworks/ui5/test/models/sink/logSinkTest.ql b/javascript/frameworks/ui5/test/models/sink/logSinkTest.ql deleted file mode 100644 index bb43a4779..000000000 --- a/javascript/frameworks/ui5/test/models/sink/logSinkTest.ql +++ /dev/null @@ -1,17 +0,0 @@ -/** - * @id log-sinks - * @name Log injection sinks - * @kind problem - * @problem.severity error - */ - -import javascript -import advanced_security.javascript.frameworks.ui5.UI5DataFlow -import semmle.javascript.security.dataflow.LogInjectionQuery as LogInjectionQuery - -class UI5ExtLogISink extends LogInjectionQuery::Sink { - UI5ExtLogISink() { this = ModelOutput::getASinkNode("ui5-log-injection").asSink() } -} - -from UI5ExtLogISink sink -select sink, sink.toString() diff --git a/javascript/frameworks/ui5/test/models/sink/logSinkTest.qlref b/javascript/frameworks/ui5/test/models/sink/logSinkTest.qlref new file mode 100644 index 000000000..286209e7e --- /dev/null +++ b/javascript/frameworks/ui5/test/models/sink/logSinkTest.qlref @@ -0,0 +1 @@ +Diagnostics/ListLogInjectionSinks.ql \ No newline at end of file