Skip to content

Commit fc56c5a

Browse files
tiferettiferet
committed
Implement the type-specific endpoint filters as EndpointCharacteristics.
Also disambiguate three filters from three different sink types that all have the same name, "not a direct argument to a likely external library call or a heuristic sink".
1 parent 13cb0ab commit fc56c5a

1 file changed

Lines changed: 309 additions & 0 deletions

File tree

‎javascript/ql/experimental/adaptivethreatmodeling/lib/experimental/adaptivethreatmodeling/EndpointCharacteristics.qll‎

Lines changed: 309 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ private import semmle.javascript.security.dataflow.TaintedPathCustomizations
1010
private import CoreKnowledge as CoreKnowledge
1111
private import semmle.javascript.heuristics.SyntacticHeuristics
1212
private import semmle.javascript.filters.ClassifyFiles as ClassifyFiles
13+
private import StandardEndpointFilters as StandardEndpointFilters
1314

1415
/**
1516
* A set of characteristics that a particular endpoint might have. This set of characteristics is used to make decisions
@@ -555,3 +556,311 @@ private class InIrrelevantFileCharacteristic extends StandardEndpointFilterChara
555556
this = "in " + category + " file"
556557
}
557558
}
559+
560+
/** An EndpointFilterCharacteristic that indicates that an endpoint is unlikely to be a NoSQL injection sink. */
561+
abstract private class NosqlInjectionSinkEndpointFilterCharacteristic extends EndpointFilterCharacteristic {
562+
bindingset[this]
563+
NosqlInjectionSinkEndpointFilterCharacteristic() { any() }
564+
565+
override predicate getImplications(
566+
EndpointType endpointClass, boolean isPositiveIndicator, float confidence
567+
) {
568+
endpointClass instanceof NosqlInjectionSinkType and
569+
isPositiveIndicator = false and
570+
confidence = mediumConfidence()
571+
}
572+
}
573+
574+
private class DatabaseAccessCallHeuristicCharacteristic extends NosqlInjectionSinkEndpointFilterCharacteristic {
575+
DatabaseAccessCallHeuristicCharacteristic() { this = "matches database access call heuristic" }
576+
577+
override predicate getEndpoints(DataFlow::Node n) {
578+
exists(DataFlow::CallNode call | n = call.getAnArgument() |
579+
// additional databases accesses that aren't modeled yet
580+
call.(DataFlow::MethodCallNode).getMethodName() =
581+
["create", "createCollection", "createIndexes"]
582+
)
583+
}
584+
}
585+
586+
private class ModeledSinkCharacteristic extends NosqlInjectionSinkEndpointFilterCharacteristic {
587+
ModeledSinkCharacteristic() { this = "modeled sink" }
588+
589+
override predicate getEndpoints(DataFlow::Node n) {
590+
exists(DataFlow::CallNode call | n = call.getAnArgument() |
591+
// Remove modeled sinks
592+
CoreKnowledge::isArgumentToKnownLibrarySinkFunction(n)
593+
)
594+
}
595+
}
596+
597+
private class PredecessorInModeledFlowStepCharacteristic extends NosqlInjectionSinkEndpointFilterCharacteristic {
598+
PredecessorInModeledFlowStepCharacteristic() { this = "predecessor in a modeled flow step" }
599+
600+
override predicate getEndpoints(DataFlow::Node n) {
601+
exists(DataFlow::CallNode call | n = call.getAnArgument() |
602+
// Remove common kinds of unlikely sinks
603+
CoreKnowledge::isKnownStepSrc(n)
604+
)
605+
}
606+
}
607+
608+
private class ModeledDatabaseAccessCharacteristic extends NosqlInjectionSinkEndpointFilterCharacteristic {
609+
ModeledDatabaseAccessCharacteristic() { this = "modeled database access" }
610+
611+
override predicate getEndpoints(DataFlow::Node n) {
612+
exists(DataFlow::CallNode call | n = call.getAnArgument() |
613+
// Remove modeled database calls. Arguments to modeled calls are very likely to be modeled
614+
// as sinks if they are true positives. Therefore arguments that are not modeled as sinks
615+
// are unlikely to be true positives.
616+
call instanceof DatabaseAccess
617+
)
618+
}
619+
}
620+
621+
private class ReceiverIsHTTPRequestExpressionCharacteristic extends NosqlInjectionSinkEndpointFilterCharacteristic {
622+
ReceiverIsHTTPRequestExpressionCharacteristic() { this = "receiver is a HTTP request expression" }
623+
624+
override predicate getEndpoints(DataFlow::Node n) {
625+
exists(DataFlow::CallNode call | n = call.getAnArgument() |
626+
// Remove calls to APIs that aren't relevant to NoSQL injection
627+
call.getReceiver() instanceof Http::RequestNode
628+
)
629+
}
630+
}
631+
632+
private class ReceiverIsHTTPResponseExpressionCharacteristic extends NosqlInjectionSinkEndpointFilterCharacteristic {
633+
ReceiverIsHTTPResponseExpressionCharacteristic() {
634+
this = "receiver is a HTTP response expression"
635+
}
636+
637+
override predicate getEndpoints(DataFlow::Node n) {
638+
exists(DataFlow::CallNode call | n = call.getAnArgument() |
639+
// Remove calls to APIs that aren't relevant to NoSQL injection
640+
call.getReceiver() instanceof Http::ResponseNode
641+
)
642+
}
643+
}
644+
645+
private class NotDirectArgumentToLikelyExternalLibraryCallOrHeuristicSinkNosqlCharacteristic extends NosqlInjectionSinkEndpointFilterCharacteristic {
646+
NotDirectArgumentToLikelyExternalLibraryCallOrHeuristicSinkNosqlCharacteristic() {
647+
this = "not a direct argument to a likely external library call or a heuristic sink (nosql)"
648+
}
649+
650+
override predicate getEndpoints(DataFlow::Node n) {
651+
// Require NoSQL injection sink candidates to be (a) direct arguments to external library calls
652+
// or (b) heuristic sinks for NoSQL injection.
653+
//
654+
// ## Direct arguments to external library calls
655+
//
656+
// The `StandardEndpointFilters::flowsToArgumentOfLikelyExternalLibraryCall` endpoint filter
657+
// allows sink candidates which are within object literals or array literals, for example
658+
// `req.sendFile(_, { path: ENDPOINT })`.
659+
//
660+
// However, the NoSQL injection query deals differently with these types of sinks compared to
661+
// other security queries. Other security queries such as SQL injection tend to treat
662+
// `ENDPOINT` as the ground truth sink, but the NoSQL injection query instead treats
663+
// `{ path: ENDPOINT }` as the ground truth sink and defines an additional flow step to ensure
664+
// data flows from `ENDPOINT` to the ground truth sink `{ path: ENDPOINT }`.
665+
//
666+
// Therefore for the NoSQL injection boosted query, we must ignore sink candidates within object
667+
// literals or array literals, to avoid having multiple alerts for the same security
668+
// vulnerability (one FP where the sink is `ENDPOINT` and one TP where the sink is
669+
// `{ path: ENDPOINT }`). We accomplish this by directly testing that the sink candidate is an
670+
// argument of a likely external library call.
671+
//
672+
// ## Heuristic sinks
673+
//
674+
// We also allow heuristic sinks in addition to direct arguments to external library calls.
675+
// These are copied from the `HeuristicNosqlInjectionSink` class defined within
676+
// `codeql/javascript/ql/src/semmle/javascript/heuristics/AdditionalSinks.qll`.
677+
// We can't reuse the class because importing that file would cause us to treat these
678+
// heuristic sinks as known sinks.
679+
not n = StandardEndpointFilters::getALikelyExternalLibraryCall().getAnArgument() and
680+
not (
681+
isAssignedToOrConcatenatedWith(n, "(?i)(nosql|query)") or
682+
isArgTo(n, "(?i)(query)")
683+
)
684+
}
685+
}
686+
687+
/** An EndpointFilterCharacteristic that indicates that an endpoint is unlikely to be a SQL injection sink. */
688+
abstract private class SqlInjectionSinkEndpointFilterCharacteristic extends EndpointFilterCharacteristic {
689+
bindingset[this]
690+
SqlInjectionSinkEndpointFilterCharacteristic() { any() }
691+
692+
override predicate getImplications(
693+
EndpointType endpointClass, boolean isPositiveIndicator, float confidence
694+
) {
695+
endpointClass instanceof SqlInjectionSinkType and
696+
isPositiveIndicator = false and
697+
confidence = mediumConfidence()
698+
}
699+
}
700+
701+
private class PreparedSQLStatementCharacteristic extends SqlInjectionSinkEndpointFilterCharacteristic {
702+
PreparedSQLStatementCharacteristic() { this = "prepared SQL statement" }
703+
704+
override predicate getEndpoints(DataFlow::Node n) {
705+
exists(DataFlow::CallNode call | n = call.getAnArgument() |
706+
// prepared statements for SQL
707+
any(DataFlow::CallNode cn | cn.getCalleeName() = "prepare")
708+
.getAMethodCall("run")
709+
.getAnArgument() = n
710+
)
711+
}
712+
}
713+
714+
private class ArrayCreationCharacteristic extends SqlInjectionSinkEndpointFilterCharacteristic {
715+
ArrayCreationCharacteristic() { this = "array creation" }
716+
717+
override predicate getEndpoints(DataFlow::Node n) {
718+
exists(DataFlow::CallNode call | n = call.getAnArgument() |
719+
n instanceof DataFlow::ArrayCreationNode
720+
)
721+
}
722+
}
723+
724+
private class HTMLOrRenderingCharacteristic extends SqlInjectionSinkEndpointFilterCharacteristic {
725+
HTMLOrRenderingCharacteristic() { this = "HTML / rendering" }
726+
727+
override predicate getEndpoints(DataFlow::Node n) {
728+
exists(DataFlow::CallNode call | n = call.getAnArgument() |
729+
// UI is unrelated to SQL
730+
call.getCalleeName().regexpMatch("(?i).*(render|html).*")
731+
)
732+
}
733+
}
734+
735+
private class NotAnArgumentToLikelyExternalLibraryCallOrHeuristicSinkCharacteristic extends SqlInjectionSinkEndpointFilterCharacteristic {
736+
NotAnArgumentToLikelyExternalLibraryCallOrHeuristicSinkCharacteristic() {
737+
this = "not an argument to a likely external library call or a heuristic sink"
738+
}
739+
740+
override predicate getEndpoints(DataFlow::Node n) {
741+
// Require SQL injection sink candidates to be (a) arguments to external library calls
742+
// (possibly indirectly), or (b) heuristic sinks.
743+
//
744+
// Heuristic sinks are copied from the `HeuristicSqlInjectionSink` class defined within
745+
// `codeql/javascript/ql/src/semmle/javascript/heuristics/AdditionalSinks.qll`.
746+
// We can't reuse the class because importing that file would cause us to treat these
747+
// heuristic sinks as known sinks.
748+
not StandardEndpointFilters::flowsToArgumentOfLikelyExternalLibraryCall(n) and
749+
not (
750+
isAssignedToOrConcatenatedWith(n, "(?i)(sql|query)") or
751+
isArgTo(n, "(?i)(query)") or
752+
isConcatenatedWithString(n,
753+
"(?s).*(ALTER|COUNT|CREATE|DATABASE|DELETE|DISTINCT|DROP|FROM|GROUP|INSERT|INTO|LIMIT|ORDER|SELECT|TABLE|UPDATE|WHERE).*")
754+
)
755+
}
756+
}
757+
758+
/** An EndpointFilterCharacteristic that indicates that an endpoint is unlikely to be a tainted path injection sink. */
759+
abstract private class TaintedPathSinkEndpointFilterCharacteristic extends EndpointFilterCharacteristic {
760+
bindingset[this]
761+
TaintedPathSinkEndpointFilterCharacteristic() { any() }
762+
763+
override predicate getImplications(
764+
EndpointType endpointClass, boolean isPositiveIndicator, float confidence
765+
) {
766+
endpointClass instanceof TaintedPathSinkType and
767+
isPositiveIndicator = false and
768+
confidence = mediumConfidence()
769+
}
770+
}
771+
772+
private class NotDirectArgumentToLikelyExternalLibraryCallOrHeuristicSinkTaintedPathCharacteristic extends TaintedPathSinkEndpointFilterCharacteristic {
773+
NotDirectArgumentToLikelyExternalLibraryCallOrHeuristicSinkTaintedPathCharacteristic() {
774+
this =
775+
"not a direct argument to a likely external library call or a heuristic sink (tainted path)"
776+
}
777+
778+
override predicate getEndpoints(DataFlow::Node n) {
779+
// Require path injection sink candidates to be (a) arguments to external library calls
780+
// (possibly indirectly), or (b) heuristic sinks.
781+
//
782+
// Heuristic sinks are mostly copied from the `HeuristicTaintedPathSink` class defined within
783+
// `codeql/javascript/ql/src/semmle/javascript/heuristics/AdditionalSinks.qll`.
784+
// We can't reuse the class because importing that file would cause us to treat these
785+
// heuristic sinks as known sinks.
786+
not StandardEndpointFilters::flowsToArgumentOfLikelyExternalLibraryCall(n) and
787+
not (
788+
isAssignedToOrConcatenatedWith(n, "(?i)(file|folder|dir|absolute)")
789+
or
790+
isArgTo(n, "(?i)(get|read)file")
791+
or
792+
exists(string pathPattern |
793+
// paths with at least two parts, and either a trailing or leading slash
794+
pathPattern = "(?i)([a-z0-9_.-]+/){2,}" or
795+
pathPattern = "(?i)(/[a-z0-9_.-]+){2,}"
796+
|
797+
isConcatenatedWithString(n, pathPattern)
798+
)
799+
or
800+
isConcatenatedWithStrings(".*/", n, "/.*")
801+
or
802+
// In addition to the names from `HeuristicTaintedPathSink` in the
803+
// `isAssignedToOrConcatenatedWith` predicate call above, we also allow the noisier "path"
804+
// name.
805+
isAssignedToOrConcatenatedWith(n, "(?i)path")
806+
)
807+
}
808+
}
809+
810+
/** An EndpointFilterCharacteristic that indicates that an endpoint is unlikely to be an XSS sink. */
811+
abstract private class XssSinkEndpointFilterCharacteristic extends EndpointFilterCharacteristic {
812+
bindingset[this]
813+
XssSinkEndpointFilterCharacteristic() { any() }
814+
815+
override predicate getImplications(
816+
EndpointType endpointClass, boolean isPositiveIndicator, float confidence
817+
) {
818+
endpointClass instanceof XssSinkType and
819+
isPositiveIndicator = false and
820+
confidence = mediumConfidence()
821+
}
822+
}
823+
824+
private class SetStateCallsInReactApplicationsCharacteristic extends XssSinkEndpointFilterCharacteristic {
825+
SetStateCallsInReactApplicationsCharacteristic() {
826+
this = "setState calls ought to be safe in react applications"
827+
}
828+
829+
override predicate getEndpoints(DataFlow::Node n) {
830+
exists(DataFlow::CallNode call | n = call.getAnArgument() | call.getCalleeName() = "setState")
831+
}
832+
}
833+
834+
private class NotDirectArgumentToLikelyExternalLibraryCallOrHeuristicSinkXssCharacteristic extends XssSinkEndpointFilterCharacteristic {
835+
NotDirectArgumentToLikelyExternalLibraryCallOrHeuristicSinkXssCharacteristic() {
836+
this = "not a direct argument to a likely external library call or a heuristic sink (XSS)"
837+
}
838+
839+
override predicate getEndpoints(DataFlow::Node n) {
840+
// Require XSS sink candidates to be (a) arguments to external library calls (possibly
841+
// indirectly), or (b) heuristic sinks.
842+
//
843+
// Heuristic sinks are copied from the `HeuristicDomBasedXssSink` class defined within
844+
// `codeql/javascript/ql/src/semmle/javascript/heuristics/AdditionalSinks.qll`.
845+
// We can't reuse the class because importing that file would cause us to treat these
846+
// heuristic sinks as known sinks.
847+
not StandardEndpointFilters::flowsToArgumentOfLikelyExternalLibraryCall(n) and
848+
not (
849+
isAssignedToOrConcatenatedWith(n, "(?i)(html|innerhtml)")
850+
or
851+
isArgTo(n, "(?i)(html|render)")
852+
or
853+
n instanceof StringOps::HtmlConcatenationLeaf
854+
or
855+
isConcatenatedWithStrings("(?is).*<[a-z ]+.*", n, "(?s).*>.*")
856+
or
857+
// In addition to the heuristic sinks from `HeuristicDomBasedXssSink`, explicitly allow
858+
// property writes like `elem.innerHTML = <TAINT>` that may not be picked up as HTML
859+
// concatenation leaves.
860+
exists(DataFlow::PropWrite pw |
861+
pw.getPropertyName().regexpMatch("(?i).*html*") and
862+
pw.getRhs() = n
863+
)
864+
)
865+
}
866+
}

0 commit comments

Comments
 (0)