From 768e57f12c4045d013c4f76df03873b7e8f2d2b7 Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Thu, 2 Oct 2025 15:46:28 -0400 Subject: [PATCH 1/3] Fix shared flow step LogArgumentToListener over sharing of flow steps was occuring unintentionally --- .../frameworks/ui5/UI5LogsToHttpQuery.qll | 14 +++++++++----- .../frameworks/ui5/dataflow/FlowSteps.qll | 16 ---------------- 2 files changed, 9 insertions(+), 21 deletions(-) diff --git a/javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/UI5LogsToHttpQuery.qll b/javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/UI5LogsToHttpQuery.qll index 8a6b6452c..0e291687a 100644 --- a/javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/UI5LogsToHttpQuery.qll +++ b/javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/UI5LogsToHttpQuery.qll @@ -29,11 +29,15 @@ module UI5LogEntryToHttp implements DataFlow::StateConfigSig { UI5LogInjection::isAdditionalFlowStep(start, end) and preState = postState or - exists(LogArgumentToListener logArgumentToListener | - logArgumentToListener.step(start, end) and - preState = "not-logged-not-accessed" and - postState = "logged-and-accessed" - ) + inSameWebApp(start.getFile(), end.getFile()) and + start = + ModelOutput::getATypeNode("SapLogger") + .getMember(["debug", "error", "fatal", "info", "trace", "warning"]) + .getACall() + .getAnArgument() and + end = ModelOutput::getATypeNode("SapLogEntries").asSource() and + preState = "not-logged-not-accessed" and + postState = "logged-and-accessed" } predicate isSink(DataFlow::Node node, FlowState state) { diff --git a/javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/dataflow/FlowSteps.qll b/javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/dataflow/FlowSteps.qll index 3fde64d21..4588c3572 100644 --- a/javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/dataflow/FlowSteps.qll +++ b/javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/dataflow/FlowSteps.qll @@ -342,19 +342,3 @@ class ResourceBundleGetTextCallArgToReturnValueStep extends DataFlow::SharedFlow ) } } - -/** - * A step from any argument of a SAP logging function to the `onLogEntry` - * method of a custom log listener in the same application. - */ -class LogArgumentToListener extends DataFlow::SharedFlowStep { - override predicate step(DataFlow::Node start, DataFlow::Node end) { - inSameWebApp(start.getFile(), end.getFile()) and - start = - ModelOutput::getATypeNode("SapLogger") - .getMember(["debug", "error", "fatal", "info", "trace", "warning"]) - .getACall() - .getAnArgument() and - end = ModelOutput::getATypeNode("SapLogEntries").asSource() - } -} From fb52bfef02fbc43fddd546262f06d53cfea67ccf Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Thu, 2 Oct 2025 16:49:40 -0400 Subject: [PATCH 2/3] Refactor fix logger step previously lost some intentional sharing --- .../frameworks/ui5/UI5LogsToHttpQuery.qll | 8 +------- .../frameworks/ui5/dataflow/FlowSteps.qll | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/UI5LogsToHttpQuery.qll b/javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/UI5LogsToHttpQuery.qll index 0e291687a..75c8ab2e1 100644 --- a/javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/UI5LogsToHttpQuery.qll +++ b/javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/UI5LogsToHttpQuery.qll @@ -29,13 +29,7 @@ module UI5LogEntryToHttp implements DataFlow::StateConfigSig { UI5LogInjection::isAdditionalFlowStep(start, end) and preState = postState or - inSameWebApp(start.getFile(), end.getFile()) and - start = - ModelOutput::getATypeNode("SapLogger") - .getMember(["debug", "error", "fatal", "info", "trace", "warning"]) - .getACall() - .getAnArgument() and - end = ModelOutput::getATypeNode("SapLogEntries").asSource() and + stepLogger(start, end) and preState = "not-logged-not-accessed" and postState = "logged-and-accessed" } diff --git a/javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/dataflow/FlowSteps.qll b/javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/dataflow/FlowSteps.qll index 4588c3572..54d3581ed 100644 --- a/javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/dataflow/FlowSteps.qll +++ b/javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/dataflow/FlowSteps.qll @@ -342,3 +342,21 @@ class ResourceBundleGetTextCallArgToReturnValueStep extends DataFlow::SharedFlow ) } } + +/** + * A step from any argument of a SAP logging function to the `onLogEntry` + * method of a custom log listener in the same application. + */ +predicate stepLogger(DataFlow::Node start, DataFlow::Node end) { + inSameWebApp(start.getFile(), end.getFile()) and + start = + ModelOutput::getATypeNode("SapLogger") + .getMember(["debug", "error", "fatal", "info", "trace", "warning"]) + .getACall() + .getAnArgument() and + end = ModelOutput::getATypeNode("SapLogEntries").asSource() +} + +class LogArgumentToListener extends DataFlow::SharedFlowStep { + override predicate step(DataFlow::Node start, DataFlow::Node end) { stepLogger(start, end) } +} From 0c4e3b15eec1f0314d89c7f2b8259811f83748bc Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Fri, 3 Oct 2025 13:01:31 -0400 Subject: [PATCH 3/3] Address review comments --- .../javascript/frameworks/ui5/UI5LogsToHttpQuery.qll | 2 +- .../javascript/frameworks/ui5/dataflow/FlowSteps.qll | 10 ++++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/UI5LogsToHttpQuery.qll b/javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/UI5LogsToHttpQuery.qll index 75c8ab2e1..7b847f8ad 100644 --- a/javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/UI5LogsToHttpQuery.qll +++ b/javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/UI5LogsToHttpQuery.qll @@ -29,7 +29,7 @@ module UI5LogEntryToHttp implements DataFlow::StateConfigSig { UI5LogInjection::isAdditionalFlowStep(start, end) and preState = postState or - stepLogger(start, end) and + logArgumentToListener(start, end) and preState = "not-logged-not-accessed" and postState = "logged-and-accessed" } diff --git a/javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/dataflow/FlowSteps.qll b/javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/dataflow/FlowSteps.qll index 54d3581ed..a3463e079 100644 --- a/javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/dataflow/FlowSteps.qll +++ b/javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/dataflow/FlowSteps.qll @@ -347,7 +347,7 @@ class ResourceBundleGetTextCallArgToReturnValueStep extends DataFlow::SharedFlow * A step from any argument of a SAP logging function to the `onLogEntry` * method of a custom log listener in the same application. */ -predicate stepLogger(DataFlow::Node start, DataFlow::Node end) { +predicate logArgumentToListener(DataFlow::Node start, DataFlow::Node end) { inSameWebApp(start.getFile(), end.getFile()) and start = ModelOutput::getATypeNode("SapLogger") @@ -357,6 +357,12 @@ predicate stepLogger(DataFlow::Node start, DataFlow::Node end) { end = ModelOutput::getATypeNode("SapLogEntries").asSource() } +/** + * A step from any argument of a SAP logging function to the `onLogEntry` + * method of a custom log listener in the same application. + */ class LogArgumentToListener extends DataFlow::SharedFlowStep { - override predicate step(DataFlow::Node start, DataFlow::Node end) { stepLogger(start, end) } + override predicate step(DataFlow::Node start, DataFlow::Node end) { + logArgumentToListener(start, end) + } }