Skip to content

Sink endpoint characteristics #11055

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
Nov 4, 2022
Merged
Show file tree
Hide file tree
Changes from all 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 @@ -6,6 +6,7 @@

private import javascript as JS
import EndpointTypes
import EndpointCharacteristics

/**
* EXPERIMENTAL. This API may change in the future.
Expand Down Expand Up @@ -44,7 +45,14 @@ abstract class AtmConfig extends string {
*
* Holds if `sink` is a known sink of flow.
*/
predicate isKnownSink(JS::DataFlow::Node sink) { none() }
final predicate isKnownSink(JS::DataFlow::Node sink) {
// If the list of characteristics includes positive indicators with maximal confidence for this class, then it's a
// known sink for the class.
exists(EndpointCharacteristic characteristic |
characteristic.getEndpoints(sink) and
characteristic.getImplications(this.getASinkEndpointType(), true, 1.0)
)
}

/**
* EXPERIMENTAL. This API may change in the future.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
/**
* For internal use only.
*/

import experimental.adaptivethreatmodeling.EndpointTypes
private import semmle.javascript.security.dataflow.SqlInjectionCustomizations
private import semmle.javascript.security.dataflow.DomBasedXssCustomizations
private import semmle.javascript.security.dataflow.NosqlInjectionCustomizations
private import semmle.javascript.security.dataflow.TaintedPathCustomizations

/**
* A set of characteristics that a particular endpoint might have. This set of characteristics is used to make decisions
* about whether to include the endpoint in the training set and with what label, as well as whether to score the
* endpoint at inference time.
*/
abstract class EndpointCharacteristic extends string {
/**
* Holds when the string matches the name of the characteristic, which should describe some characteristic of the
* endpoint that is meaningful for determining whether it's a sink and if so of which type
*/
bindingset[this]
EndpointCharacteristic() { any() }

/**
* Holds for endpoints that have this characteristic. This predicate contains the logic that applies characteristics
* to the appropriate set of dataflow nodes.
*/
abstract predicate getEndpoints(DataFlow::Node n);

/**
* This predicate describes what the characteristic tells us about an endpoint.
*
* Params:
* endpointClass: Class 0 is the negative class. Each positive int corresponds to a single sink type.
Copy link
Contributor

Choose a reason for hiding this comment

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

I know what you mean here, but it took me a second since 0 is not negative. I don't have any suggestions on improvement, though.

* isPositiveIndicator: Does this characteristic indicate this endpoint _is_ a member of the class, or that it
* _isn't_ a member of the class?
Comment on lines +35 to +36
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor:

Suggested change
* isPositiveIndicator: Does this characteristic indicate this endpoint _is_ a member of the class, or that it
* _isn't_ a member of the class?
* isPositiveIndicator: If true, this endpoint is a member of the class.

* confidence: A number in [0, 1], which tells us how strong an indicator this characteristic is for the endpoint
* belonging / not belonging to the given class.
Comment on lines +37 to +38
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not confident that this comment change makes things better. It's mostly for my own understanding.

Suggested change
* confidence: A number in [0, 1], which tells us how strong an indicator this characteristic is for the endpoint
* belonging / not belonging to the given class.
* confidence: A float in [0, 1], which tells us how strong an indicator this characteristic is for the endpoint
* belonging / not belonging to the given class. 0 means complete confidence that this characteristic _does not_ indicate belonging to this endpoint. And 1 means complete confidence that this characteristic _does_ belong..

*/
abstract predicate getImplications(
EndpointType endpointClass, boolean isPositiveIndicator, float confidence
);
}

/**
* Endpoints identified as "DomBasedXssSink" by the standard JavaScript libraries are XSS sinks with maximal confidence.
*/
private class DomBasedXssSinkCharacteristic extends EndpointCharacteristic {
DomBasedXssSinkCharacteristic() { this = "DomBasedXssSink" }

override predicate getEndpoints(DataFlow::Node n) { n instanceof DomBasedXss::Sink }

override predicate getImplications(
EndpointType endpointClass, boolean isPositiveIndicator, float confidence
) {
endpointClass instanceof XssSinkType and isPositiveIndicator = true and confidence = 1.0
}
}

/**
* Endpoints identified as "TaintedPathSink" by the standard JavaScript libraries are path injection sinks with maximal
* confidence.
*/
private class TaintedPathSinkCharacteristic extends EndpointCharacteristic {
TaintedPathSinkCharacteristic() { this = "TaintedPathSink" }

override predicate getEndpoints(DataFlow::Node n) { n instanceof TaintedPath::Sink }

override predicate getImplications(
EndpointType endpointClass, boolean isPositiveIndicator, float confidence
) {
endpointClass instanceof TaintedPathSinkType and isPositiveIndicator = true and confidence = 1.0
}
}

/**
* Endpoints identified as "SqlInjectionSink" by the standard JavaScript libraries are SQL injection sinks with maximal
* confidence.
*/
private class SqlInjectionSinkCharacteristic extends EndpointCharacteristic {
SqlInjectionSinkCharacteristic() { this = "SqlInjectionSink" }

override predicate getEndpoints(DataFlow::Node n) { n instanceof SqlInjection::Sink }

override predicate getImplications(
EndpointType endpointClass, boolean isPositiveIndicator, float confidence
) {
endpointClass instanceof SqlInjectionSinkType and
isPositiveIndicator = true and
confidence = 1.0
}
}

/**
* Endpoints identified as "NosqlInjectionSink" by the standard JavaScript libraries are NoSQL injection sinks with
* maximal confidence.
*/
private class NosqlInjectionSinkCharacteristic extends EndpointCharacteristic {
NosqlInjectionSinkCharacteristic() { this = "NosqlInjectionSink" }

override predicate getEndpoints(DataFlow::Node n) { n instanceof NosqlInjection::Sink }

override predicate getImplications(
EndpointType endpointClass, boolean isPositiveIndicator, float confidence
) {
endpointClass instanceof NosqlInjectionSinkType and
isPositiveIndicator = true and
confidence = 1.0
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,6 @@ class NosqlInjectionAtmConfig extends AtmConfig {
source instanceof NosqlInjection::Source or TaintedObject::isSource(source, _)
}

override predicate isKnownSink(DataFlow::Node sink) { sink instanceof NosqlInjection::Sink }

override predicate isEffectiveSink(DataFlow::Node sinkCandidate) {
not exists(SinkEndpointFilter::getAReasonSinkExcluded(sinkCandidate))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,6 @@ class SqlInjectionAtmConfig extends AtmConfig {

override predicate isKnownSource(DataFlow::Node source) { source instanceof SqlInjection::Source }

override predicate isKnownSink(DataFlow::Node sink) { sink instanceof SqlInjection::Sink }

override predicate isEffectiveSink(DataFlow::Node sinkCandidate) {
not exists(SinkEndpointFilter::getAReasonSinkExcluded(sinkCandidate))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,6 @@ class TaintedPathAtmConfig extends AtmConfig {

override predicate isKnownSource(DataFlow::Node source) { source instanceof TaintedPath::Source }

override predicate isKnownSink(DataFlow::Node sink) { sink instanceof TaintedPath::Sink }

override predicate isEffectiveSink(DataFlow::Node sinkCandidate) {
not exists(SinkEndpointFilter::getAReasonSinkExcluded(sinkCandidate))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,6 @@ class DomBasedXssAtmConfig extends AtmConfig {

override predicate isKnownSource(DataFlow::Node source) { source instanceof DomBasedXss::Source }

override predicate isKnownSink(DataFlow::Node sink) { sink instanceof DomBasedXss::Sink }

override predicate isEffectiveSink(DataFlow::Node sinkCandidate) {
not exists(SinkEndpointFilter::getAReasonSinkExcluded(sinkCandidate))
}
Expand Down