Skip to content

Commit 0f27bff

Browse files
committed
JS: Add sinks for server-template tags in AngularJS templates
1 parent d6dbabf commit 0f27bff

3 files changed

Lines changed: 62 additions & 0 deletions

File tree

javascript/ql/src/semmle/javascript/frameworks/AngularJS/AngularJSExpressions.qll

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -841,3 +841,37 @@ class NgDataFlowNode extends TNode {
841841
)
842842
}
843843
}
844+
845+
/** Holds if everything in the given file should be considered part of an AngularJS app. */
846+
private predicate fileIsImplicitlyAngularJS(HTML::HtmlFile file) {
847+
// The file contains ng-* attributes.
848+
exists(HTML::Attribute attrib |
849+
attrib.getName().regexpMatch("ng-.*") and
850+
attrib.getFile() = file
851+
) and
852+
// But does not contain the ng-app root element, implying that file is
853+
// included from elsewhere.
854+
not exists(HTML::Attribute attrib |
855+
attrib.getName() = "ng-app" and
856+
attrib.getFile() = file
857+
)
858+
}
859+
860+
/** Holds if `element` is under a `ng-non-bindable` directive, disabling interpretation by AngularJS. */
861+
private predicate isNonBindable(HTML::Element element) {
862+
exists(element.getParent*().getAttributeByName("ng-non-bindable"))
863+
}
864+
865+
/**
866+
* Holds if the contents and attribute values of the given element are interpreted by AngularJS,
867+
* that is, any placeholder expressions therein, such as `{{x}}`, are evaluated and inserted in the output.
868+
*/
869+
predicate isInterpretedByAngularJS(HTML::Element element) {
870+
(
871+
fileIsImplicitlyAngularJS(element.getFile())
872+
or
873+
exists(element.getParent*().getAttributeByName("ng-app"))
874+
) and
875+
not isNonBindable(element) and
876+
not element.getName() = "script" // script tags are never interpreted
877+
}

javascript/ql/src/semmle/javascript/frameworks/Templating.qll

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,16 @@ module Templating {
9999
/** Holds if this placeholder occurs in JS code. */
100100
predicate isInCodeContext() {
101101
isInScriptTag() or isInCodeAttribute()
102+
/**
103+
* Holds if this placeholder occurs in the definition of another template, which means the output
104+
* is susceptible to code injection.
105+
*/
106+
predicate isInNestedTemplateContext(string templateType) {
107+
templateType = "AngularJS" and
108+
AngularJS::isInterpretedByAngularJS(getParent()) and
109+
// Exclude delimiters that coincide with those of AngularJS's own template engine.
110+
// It's too unlikely to happen, more likely is that one of our heuristics got it wrong.
111+
not getRawText().regexpMatch("(?s)\\{\\{.*\\}\\}")
102112
}
103113

104114
/**

javascript/ql/src/semmle/javascript/security/dataflow/CodeInjectionCustomizations.qll

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,24 @@ module CodeInjection {
6565
}
6666
}
6767

68+
/**
69+
* A server-side template tag occurring in the context of another template language.
70+
*/
71+
class TemplateTagInNestedTemplateContext extends Sink {
72+
string templateType;
73+
74+
TemplateTagInNestedTemplateContext() {
75+
exists(Templating::TemplatePlaceholderTag tag |
76+
tag.isInNestedTemplateContext(templateType) and
77+
this = tag.asDataFlowNode()
78+
)
79+
}
80+
81+
override string getMessageSuffix() {
82+
result = "here and is interpreted by " + templateType + ", which may evaluate it as code"
83+
}
84+
}
85+
6886
/**
6987
* Gets a reference to a `<script />` tag created using `document.createElement`.
7088
*/

0 commit comments

Comments
 (0)