Skip to content

Add sensitive information exposure query #126

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Jul 30, 2024
Merged
Show file tree
Hide file tree
Changes from 2 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
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,35 @@ class CdlAttribute extends JsonObject {
int getLength() { result = this.getPropValue("length").(JsonPrimitiveValue).getIntValue() }
}

/**
* any `JsonValue` that has a `PersonalData` like annotation above it
*/
class SensitiveAnnotatedElement extends JsonValue {
string annotationName;
string fieldOrEntityName;

SensitiveAnnotatedElement() {
exists(JsonValue annotationval, JsonValue entityOrField |
annotationval = this.getPropValue(annotationName) and
annotationName.matches("@PersonalData%") and
this = entityOrField.getPropValue(fieldOrEntityName)
)
}

/**
* Gets the name of this annotation, without the leading `@` character.
*/
string getAnnotationName() { "@" + result = annotationName }

/**
* Gets the name of this field or entity that is annotated
*/
string getEntityOrFieldName() { result = fieldOrEntityName }
}

/**
* CDL annotations specifically associated to `CdlElement`s
*/
abstract class CdlAnnotation extends JsonValue {
string annotationName;
CdlElement element;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
# CAP Insertion of Sensitive Information into Log File

If sensitive information is written to a log entry using the CAP Node.js logging API, a malicious user may be able to gain access to user data.

Data annotated as `@PersonalData` should not be logged.

## Recommendation

CAP applications should not log sensitive information. Check CDS declarations for annotations before logging certain data types or fields.

## Examples

This CAP service directly logs the sensitive information.

```cds
namespace advanced_security.log_exposure.sample_entities;

entity Sample {
name : String(111);
}

// annotations for Data Privacy
annotate Sample with
@PersonalData : { DataSubjectRole : 'Sample', EntitySemantics : 'DataSubject' }
{
name @PersonalData.IsPotentiallySensitive;
}
```

``` javascript
import cds from '@sap/cds'
const LOG = cds.log("logger");

const { Sample } = cds.entities('advanced_security.log_exposure.sample_entities')

class SampleVulnService extends cds.ApplicationService {
init() {
LOG.info("Received: ", Sample.name); // CAP log exposure alert
}

}
```

## References

- OWASP 2021: [Security Logging and Monitoring Failures](https://owasp.org/Top10/A09_2021-Security_Logging_and_Monitoring_Failures/).
- SAP CAPire Documentation: [PersonalData Annotations](https://cap.cloud.sap/docs/guides/data-privacy/annotations).
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/**
* @name Insertion of sensitive information into log files
* @description Writing sensitive information to log files can allow that
* information to be leaked to an attacker more easily.
* @kind path-problem
* @problem.severity warning
* @security-severity 7.5
* @precision medium
* @id javascript/sensitive-log
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

* @tags security
* external/cwe/cwe-532
*/

import javascript
import advanced_security.javascript.frameworks.cap.CDS
import advanced_security.javascript.frameworks.cap.CAPLogInjectionQuery
import DataFlow::PathGraph

class SensitiveExposureSource extends DataFlow::Node {
SensitiveExposureSource() {
exists(PropRead p, SensitiveAnnotatedElement c |
Copy link
Contributor

@mbaluda mbaluda Jul 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see a problem here as you are only comparing on the property name (name in the test vs Sample.name)
There can be multiple entities with the same names as well as multiple applications in the same repo...
I think you can use getCdsDeclaration()

p.getPropertyName() = c.getEntityOrFieldName() and
this = p
)
}
}

class SensitiveLogExposureConfig extends TaintTracking::Configuration {
SensitiveLogExposureConfig() { this = "SensitiveLogExposure" }

override predicate isSource(DataFlow::Node source) { source instanceof SensitiveExposureSource }

override predicate isSink(DataFlow::Node sink) { sink instanceof CdsLogSink }
}

from SensitiveLogExposureConfig config, DataFlow::PathNode source, DataFlow::PathNode sink
where config.hasFlowPath(source, sink)
select sink, source, sink, "Log entry depends on a potentially sensitive piece of information."
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
namespace advanced_security.log_exposure.sample_entities;

entity Sample {
name : String(111);
dateOfBirth : Date;
}

// annotations for Data Privacy
annotate Sample with
@PersonalData : { DataSubjectRole : 'Sample', EntitySemantics : 'DataSubject' }
{
name @PersonalData.IsPotentiallySensitive;
dateOfBirth @PersonalData.IsPotentiallyPersonal;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
nodes
| sensitive-exposure.js:10:32:10:42 | Sample.name |
| sensitive-exposure.js:10:32:10:42 | Sample.name |
| sensitive-exposure.js:10:32:10:42 | Sample.name |
edges
| sensitive-exposure.js:10:32:10:42 | Sample.name | sensitive-exposure.js:10:32:10:42 | Sample.name |
#select
| sensitive-exposure.js:10:32:10:42 | Sample.name | sensitive-exposure.js:10:32:10:42 | Sample.name | sensitive-exposure.js:10:32:10:42 | Sample.name | Log entry depends on a potentially sensitive piece of information. |
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import cds from '@sap/cds'
const LOG = cds.log("logger");

const { Sample } = cds.entities('advanced_security.log_exposure.sample_entities')

class SampleVulnService extends cds.ApplicationService {
init() {
/* A sensitive info log sink. */

LOG.info("Received: ", Sample.name); // CAP log exposure alert

Check failure

Code scanning / CodeQL

Insertion of sensitive information into log files High test

Log entry depends on a potentially sensitive piece of information.
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
sensitive-exposure/SensitiveExposure.ql
Empty file modified scripts/compile-all-cds-in-directory.sh
100644 → 100755
Empty file.
Empty file modified scripts/create-db-with-cds.sh
100644 → 100755
Empty file.
Loading