Skip to content

Commit c6ad123

Browse files
knewbury01knewbury01
committed
Address review comments
1 parent ad9e4fe commit c6ad123

File tree

4 files changed

+50
-23
lines changed

4 files changed

+50
-23
lines changed

‎javascript/frameworks/cap/src/sensitive-exposure/SensitiveExposureHeuristicSource.md‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,11 @@ Data that may expose system information such as full path names, system informat
66

77
## Recommendation
88

9-
CAP applications should not log sensitive information.
9+
CAP applications should not log sensitive information. Sensitive information can include: full path names, system information, usernames, passwords or any personally identifiable information. Make sure to log only information that is not sensitive, or obfuscate/encrypt sensitive information any time that it is logged.
1010

1111
## Examples
1212

13-
This CAP service directly logs the sensitive information.
13+
This CAP service directly logs the sensitive information. Potential attackers may gain access to this sensitive information when the log output is displayed or when the attacker gains access to the log, and the info is not obfuscated or encrypted.
1414

1515
``` javascript
1616
import cds from '@sap/cds'
@@ -19,6 +19,7 @@ const LOG = cds.log("logger");
1919
class SampleVulnService extends cds.ApplicationService {
2020
init() {
2121
LOG.info(`[INFO] Environment: ${JSON.stringify(process.env)}`); // CAP log exposure alert
22+
LOG.info(`[INFO] Environment: ${JSON.stringify(process.env)}`); // CAP log exposure alert
2223
}
2324
}
2425
```

‎javascript/frameworks/cap/src/sensitive-exposure/SensitiveExposureHeuristicSource.ql‎

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,17 +15,30 @@ import javascript
1515
import advanced_security.javascript.frameworks.cap.CDS
1616
import advanced_security.javascript.frameworks.cap.CAPLogInjectionQuery
1717
private import semmle.javascript.security.dataflow.CleartextLoggingCustomizations::CleartextLogging as CleartextLogging
18-
import DataFlow::PathGraph
1918

20-
class SensitiveLogExposureConfig extends TaintTracking::Configuration {
21-
SensitiveLogExposureConfig() { this = "SensitiveLogExposure" }
19+
module SensitiveLogExposureConfig implements DataFlow::ConfigSig {
20+
predicate isSource(DataFlow::Node source) { source instanceof CleartextLogging::Source }
2221

23-
override predicate isSource(DataFlow::Node source) { source instanceof CleartextLogging::Source }
22+
predicate isSink(DataFlow::Node sink) { sink instanceof CdsLogSink }
2423

25-
override predicate isSink(DataFlow::Node sink) { sink instanceof CdsLogSink }
24+
predicate isAdditionalFlowStep(DataFlow::Node src, DataFlow::Node trg) {
25+
CleartextLogging::isAdditionalTaintStep(src, trg)
26+
}
27+
28+
predicate isBarrier(DataFlow::Node sink) { sink instanceof CleartextLogging::Barrier }
29+
30+
predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet contents) {
31+
// Assume all properties of a logged object are themselves logged.
32+
contents = DataFlow::ContentSet::anyProperty() and
33+
isSink(node)
34+
}
2635
}
2736

28-
from SensitiveLogExposureConfig config, DataFlow::PathNode source, DataFlow::PathNode sink
29-
where config.hasFlowPath(source, sink)
37+
module SensitiveLogExposureFlow = TaintTracking::Global<SensitiveLogExposureConfig>;
38+
39+
import SensitiveLogExposureFlow::PathGraph
40+
41+
from SensitiveLogExposureFlow::PathNode source, SensitiveLogExposureFlow::PathNode sink
42+
where SensitiveLogExposureFlow::flowPath(source, sink)
3043
select sink, source, sink, "This logs sensitive data returned by $@ as clear text.",
3144
source.getNode(), source.getNode().(CleartextLogging::Source).describe()

‎javascript/frameworks/cap/test/queries/sensitive-exposure/sensitive-exposure-js-all-sinks/sensitive-exposure-heuristic-source.expected‎

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,18 @@
1-
WARNING: module 'PathGraph' has been deprecated and may be removed in future (SensitiveExposureHeuristicSource.ql:18,8-27)
2-
WARNING: type 'Configuration' has been deprecated and may be removed in future (SensitiveExposureHeuristicSource.ql:20,42-70)
3-
WARNING: type 'PathNode' has been deprecated and may be removed in future (SensitiveExposureHeuristicSource.ql:28,41-59)
4-
WARNING: type 'PathNode' has been deprecated and may be removed in future (SensitiveExposureHeuristicSource.ql:28,68-86)
5-
nodes
6-
| sensitive-exposure-heuristic-source.js:6:18:6:69 | `[INFO] ... .env)}` |
7-
| sensitive-exposure-heuristic-source.js:6:18:6:69 | `[INFO] ... .env)}` |
8-
| sensitive-exposure-heuristic-source.js:6:41:6:67 | JSON.st ... ss.env) |
9-
| sensitive-exposure-heuristic-source.js:6:56:6:66 | process.env |
10-
| sensitive-exposure-heuristic-source.js:6:56:6:66 | process.env |
111
edges
12-
| sensitive-exposure-heuristic-source.js:6:41:6:67 | JSON.st ... ss.env) | sensitive-exposure-heuristic-source.js:6:18:6:69 | `[INFO] ... .env)}` |
13-
| sensitive-exposure-heuristic-source.js:6:41:6:67 | JSON.st ... ss.env) | sensitive-exposure-heuristic-source.js:6:18:6:69 | `[INFO] ... .env)}` |
14-
| sensitive-exposure-heuristic-source.js:6:56:6:66 | process.env | sensitive-exposure-heuristic-source.js:6:41:6:67 | JSON.st ... ss.env) |
15-
| sensitive-exposure-heuristic-source.js:6:56:6:66 | process.env | sensitive-exposure-heuristic-source.js:6:41:6:67 | JSON.st ... ss.env) |
2+
| sensitive-exposure-heuristic-source.js:6:41:6:67 | JSON.st ... ss.env) | sensitive-exposure-heuristic-source.js:6:18:6:69 | `[INFO] ... .env)}` | provenance | |
3+
| sensitive-exposure-heuristic-source.js:6:56:6:66 | process.env | sensitive-exposure-heuristic-source.js:6:41:6:67 | JSON.st ... ss.env) | provenance | |
4+
| sensitive-exposure-heuristic-source.js:8:13:10:9 | obj [x] | sensitive-exposure-heuristic-source.js:11:18:11:20 | obj | provenance | |
5+
| sensitive-exposure-heuristic-source.js:8:19:10:9 | {\\n ... } [x] | sensitive-exposure-heuristic-source.js:8:13:10:9 | obj [x] | provenance | |
6+
| sensitive-exposure-heuristic-source.js:9:16:9:23 | password | sensitive-exposure-heuristic-source.js:8:19:10:9 | {\\n ... } [x] | provenance | |
7+
nodes
8+
| sensitive-exposure-heuristic-source.js:6:18:6:69 | `[INFO] ... .env)}` | semmle.label | `[INFO] ... .env)}` |
9+
| sensitive-exposure-heuristic-source.js:6:41:6:67 | JSON.st ... ss.env) | semmle.label | JSON.st ... ss.env) |
10+
| sensitive-exposure-heuristic-source.js:6:56:6:66 | process.env | semmle.label | process.env |
11+
| sensitive-exposure-heuristic-source.js:8:13:10:9 | obj [x] | semmle.label | obj [x] |
12+
| sensitive-exposure-heuristic-source.js:8:19:10:9 | {\\n ... } [x] | semmle.label | {\\n ... } [x] |
13+
| sensitive-exposure-heuristic-source.js:9:16:9:23 | password | semmle.label | password |
14+
| sensitive-exposure-heuristic-source.js:11:18:11:20 | obj | semmle.label | obj |
15+
subpaths
1616
#select
1717
| sensitive-exposure-heuristic-source.js:6:18:6:69 | `[INFO] ... .env)}` | sensitive-exposure-heuristic-source.js:6:56:6:66 | process.env | sensitive-exposure-heuristic-source.js:6:18:6:69 | `[INFO] ... .env)}` | This logs sensitive data returned by $@ as clear text. | sensitive-exposure-heuristic-source.js:6:56:6:66 | process.env | process environment |
18+
| sensitive-exposure-heuristic-source.js:11:18:11:20 | obj | sensitive-exposure-heuristic-source.js:9:16:9:23 | password | sensitive-exposure-heuristic-source.js:11:18:11:20 | obj | This logs sensitive data returned by $@ as clear text. | sensitive-exposure-heuristic-source.js:9:16:9:23 | password | an access to password |

‎javascript/frameworks/cap/test/queries/sensitive-exposure/sensitive-exposure-js-all-sinks/sensitive-exposure-heuristic-source.js‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,5 +4,17 @@ const LOG = cds.log("logger");
44
class SampleVulnService extends cds.ApplicationService {
55
init() {
66
LOG.info(`[INFO] Environment: ${JSON.stringify(process.env)}`); // CAP log exposure alert
7+
8+
var obj = {
9+
x: password
10+
};
11+
LOG.info(obj); // CAP log exposure alert
12+
13+
LOG.info(obj.x.replace(/./g, "*")); // NO CAP log exposure alert - replace as sanitizer
14+
15+
var user = {
16+
password: encryptLib.encryptPassword(password)
17+
};
18+
LOG.info(user); // NO CAP log exposure alert - encrypted data is fine
719
}
820
}

0 commit comments

Comments
 (0)