Skip to content

Commit 621a319

Browse files
committed
Fix regression in SensitiveExposure
1 parent fd0d13a commit 621a319

File tree

3 files changed

+40
-44
lines changed

3 files changed

+40
-44
lines changed

javascript/frameworks/cap/lib/advanced_security/javascript/frameworks/cap/CDS.qll

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -665,7 +665,7 @@ abstract class EntityReference extends CdsReference {
665665
* SELECT.from`Books`.where(`ID=${id}`)
666666
* ```
667667
*/
668-
class EntityReferenceFromEntities extends EntityReference instanceof PropRead {
668+
class EntityReferenceFromEntities extends EntityReference, SourceNode instanceof PropRead {
669669
/**
670670
* A read from property `entities` or a method call to `entities`.
671671
*/
@@ -711,6 +711,12 @@ class EntityReferenceFromEntities extends EntityReference instanceof PropRead {
711711

712712
string getEntityName() { result = entityName }
713713

714+
predicate isFromEntitiesCall() { entitiesAccess instanceof MethodCallNode }
715+
716+
string getEntitiesCallNamespace() {
717+
result = entitiesAccess.(MethodCallNode).getArgument(0).getStringValue()
718+
}
719+
714720
abstract override CdlEntity getCqlDefinition();
715721

716722
abstract UserDefinedApplicationService getServiceDefinition();

javascript/frameworks/cap/src/sensitive-exposure/SensitiveExposure.ql

Lines changed: 29 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -16,54 +16,44 @@ import advanced_security.javascript.frameworks.cap.CDS
1616
import advanced_security.javascript.frameworks.cap.CAPLogInjectionQuery
1717
import DataFlow::PathGraph
1818

19-
/**
20-
* An entity instance obtained by the entity's namespace,
21-
* via `cds.entities`
22-
* ```javascript
23-
* // Obtained through `cds.entities`
24-
* const Service1 = cds.entities("sample.application.namespace");
25-
* ```
26-
*/
27-
class EntityEntry extends DataFlow::CallNode {
28-
EntityEntry() { exists(CdsEntitiesCall c | c.getACall() = this) }
29-
30-
/**
31-
* Gets the namespace that this entity belongs to.
32-
*/
33-
string getNamespace() {
34-
result = this.getArgument(0).getALocalSource().asExpr().(StringLiteral).getValue()
35-
}
19+
// /**
20+
// * An entity instance obtained by the entity's namespace via `cds.entities`. e.g.
21+
// *
22+
// * ```javascript
23+
// * const Service1 = cds.entities("sample.application.namespace");
24+
// * ```
25+
// */
26+
// class EntityEntry extends DataFlow::CallNode {
27+
// EntityEntry() { exists(CdsEntitiesCall c | c.getACall() = this) }
28+
// /**
29+
// * Gets the namespace that this entity belongs to.
30+
// */
31+
// string getNamespace() {
32+
// result = this.getArgument(0).getALocalSource().asExpr().(StringLiteral).getValue()
33+
// }
34+
// }
35+
EntityReferenceFromEntities entityAccesses(string entityNamespace) {
36+
entityNamespace = result.getEntitiesCallNamespace()
3637
}
3738

38-
SourceNode entityAccesses(TypeTracker t, string entityNamespace) {
39-
t.start() and
40-
exists(EntityEntry e | result = e and entityNamespace = e.getNamespace())
41-
or
42-
exists(TypeTracker t2 | result = entityAccesses(t2, entityNamespace).track(t2, t))
43-
}
44-
45-
SourceNode entityAccesses(string entityNamespace) {
46-
result = entityAccesses(TypeTracker::end(), entityNamespace)
47-
}
48-
49-
class SensitiveExposureFieldSource extends DataFlow::Node {
50-
SensitiveAnnotatedAttribute cdsField;
51-
SensitiveAnnotatedEntity entity;
39+
class SensitiveExposureFieldSource instanceof PropRead {
40+
SensitiveAnnotatedAttribute cdlAttribute;
41+
SensitiveAnnotatedEntity cdlEntity;
5242
string namespace;
5343

5444
SensitiveExposureFieldSource() {
55-
this = entityAccesses(namespace).getAPropertyRead().getAPropertyRead() and
45+
this = entityAccesses(namespace).getAPropertyRead() and
5646
//field name is same as some cds declared field
57-
this.(PropRead).getPropertyName() = cdsField.getName() and
58-
//entity name is the same as some cds declared entity
59-
entityAccesses(namespace).getAPropertyRead().toString() = entity.getShortName() and
60-
//and that field belongs to that entity in the cds
61-
entity.(CdlEntity).getAttribute(cdsField.getName()) = cdsField and
47+
this.getPropertyName() = cdlAttribute.getName() and
48+
//and that field belongs to that cdlEntity in the cds
49+
cdlEntity.(CdlEntity).getAttribute(cdlAttribute.getName()) = cdlAttribute and
6250
//and the namespace is the same (fully qualified id match)
63-
entity.(NamespacedEntity).getNamespace() = namespace
51+
cdlEntity.(NamespacedEntity).getNamespace() = namespace
6452
}
6553

66-
SensitiveAnnotatedAttribute getCdsField() { result = cdsField }
54+
SensitiveAnnotatedAttribute getCdsField() { result = cdlAttribute }
55+
56+
string toString() { result = super.toString() }
6757
}
6858

6959
class SensitiveLogExposureConfig extends TaintTracking::Configuration {
Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
WARNING: module 'PathGraph' has been deprecated and may be removed in future (SensitiveExposure.ql:17,8-27)
2-
WARNING: type 'Configuration' has been deprecated and may be removed in future (SensitiveExposure.ql:50,42-70)
3-
WARNING: type 'PathNode' has been deprecated and may be removed in future (SensitiveExposure.ql:60,41-59)
4-
WARNING: type 'PathNode' has been deprecated and may be removed in future (SensitiveExposure.ql:60,68-86)
2+
WARNING: type 'Configuration' has been deprecated and may be removed in future (SensitiveExposure.ql:59,42-70)
3+
WARNING: type 'PathNode' has been deprecated and may be removed in future (SensitiveExposure.ql:69,41-59)
4+
WARNING: type 'PathNode' has been deprecated and may be removed in future (SensitiveExposure.ql:69,68-86)
55
nodes
66
| sensitive-exposure.js:9:32:9:42 | Sample.name |
77
| sensitive-exposure.js:9:32:9:42 | Sample.name |
88
| sensitive-exposure.js:9:32:9:42 | Sample.name |
99
edges
1010
| sensitive-exposure.js:9:32:9:42 | Sample.name | sensitive-exposure.js:9:32:9:42 | Sample.name |
1111
#select
12-
| sensitive-exposure.js:9:32:9:42 | Sample.name | sensitive-exposure.js:9:32:9:42 | Sample.name | sensitive-exposure.js:9:32:9:42 | Sample.name | Log entry depends on the $@ field which is annotated as potentially sensitive. | sensitive-exposure.cds:4:5:4:8 | {\\n ... } | name |
12+
| sensitive-exposure.js:9:32:9:42 | Sample.name | sensitive-exposure.js:9:32:9:42 | Sample.name | sensitive-exposure.js:9:32:9:42 | Sample.name | Log entry depends on the $@ field which is annotated as potentially sensitive. | sensitive-exposure.cds.json:9:17:13:9 | {\\n ... } | name |

0 commit comments

Comments
 (0)