From b46bb5e5d504014b137296377be5527b18cf7237 Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Mon, 22 Jul 2024 15:09:19 -0400 Subject: [PATCH 1/2] Adjust cap log sink model to exclude only constant only template literals --- .../javascript/frameworks/cap/CAPLogInjectionQuery.qll | 8 +++++++- .../frameworks/cap/test/models/cds/logger/logger.expected | 1 + .../frameworks/cap/test/models/cds/logger/logger.js | 3 +++ 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/javascript/frameworks/cap/lib/advanced_security/javascript/frameworks/cap/CAPLogInjectionQuery.qll b/javascript/frameworks/cap/lib/advanced_security/javascript/frameworks/cap/CAPLogInjectionQuery.qll index 10f206b9f..05d13e3c5 100644 --- a/javascript/frameworks/cap/lib/advanced_security/javascript/frameworks/cap/CAPLogInjectionQuery.qll +++ b/javascript/frameworks/cap/lib/advanced_security/javascript/frameworks/cap/CAPLogInjectionQuery.qll @@ -22,6 +22,12 @@ class CdsLogger extends MethodCallNode { string getName() { result = name } } +class ConstantOnlyTemplateLiteral extends TemplateLiteral { + ConstantOnlyTemplateLiteral() { + forall(Expr e | e = this.getAnElement() | e instanceof TemplateElement) + } +} + /** * Arguments of calls to `cds.log.{trace, debug, info, log, warn, error}` */ @@ -31,7 +37,7 @@ class CdsLogSink extends DataFlow::Node { this = loggingMethod.getAnArgument() and loggingMethod.getMethodName() = ["trace", "debug", "info", "log", "warn", "error"] and not this.asExpr() instanceof Literal and - not this.asExpr() instanceof TemplateLiteral and + not exists(ConstantOnlyTemplateLiteral t | this.asExpr() = t) and loggingMethod.getReceiver().getALocalSource() = log ) } diff --git a/javascript/frameworks/cap/test/models/cds/logger/logger.expected b/javascript/frameworks/cap/test/models/cds/logger/logger.expected index 6f2c1aa2c..b25f427ce 100644 --- a/javascript/frameworks/cap/test/models/cds/logger/logger.expected +++ b/javascript/frameworks/cap/test/models/cds/logger/logger.expected @@ -5,3 +5,4 @@ | logger.js:7:24:7:28 | code0 | | logger.js:8:25:8:29 | code0 | | logger.js:12:10:12:14 | code1 | +| logger.js:14:10:14:28 | `logging: ${code1}` | diff --git a/javascript/frameworks/cap/test/models/cds/logger/logger.js b/javascript/frameworks/cap/test/models/cds/logger/logger.js index bead5c040..fb9c2f2aa 100644 --- a/javascript/frameworks/cap/test/models/cds/logger/logger.js +++ b/javascript/frameworks/cap/test/models/cds/logger/logger.js @@ -10,3 +10,6 @@ cds.log('nodejs').error(code0); const code0 = "some-name"; const LOG = cds.log(code0); LOG.info(code1); + +LOG.info(`logging: ${code1}`); +LOG.info(`not actually logging`); \ No newline at end of file From f286b5faee2596da56ba2681bbc4713fff7df2eb Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Mon, 22 Jul 2024 19:38:40 -0400 Subject: [PATCH 2/2] Refactor exclusion for readability --- .../javascript/frameworks/cap/CAPLogInjectionQuery.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/javascript/frameworks/cap/lib/advanced_security/javascript/frameworks/cap/CAPLogInjectionQuery.qll b/javascript/frameworks/cap/lib/advanced_security/javascript/frameworks/cap/CAPLogInjectionQuery.qll index 05d13e3c5..bb3cdf690 100644 --- a/javascript/frameworks/cap/lib/advanced_security/javascript/frameworks/cap/CAPLogInjectionQuery.qll +++ b/javascript/frameworks/cap/lib/advanced_security/javascript/frameworks/cap/CAPLogInjectionQuery.qll @@ -37,7 +37,7 @@ class CdsLogSink extends DataFlow::Node { this = loggingMethod.getAnArgument() and loggingMethod.getMethodName() = ["trace", "debug", "info", "log", "warn", "error"] and not this.asExpr() instanceof Literal and - not exists(ConstantOnlyTemplateLiteral t | this.asExpr() = t) and + not this.asExpr() instanceof ConstantOnlyTemplateLiteral and loggingMethod.getReceiver().getALocalSource() = log ) }