hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2025-12-17 01:03:14 +01:00
Code Issues Packages Projects Releases Wiki Activity

JS: Add decorator edges in API graphs and corresponding MaD tokens

Browse Source
This commit is contained in:
Asger Feldthaus
2022-03-23 18:05:44 +01:00
parent e5f2b830f3
commit cf596a1856
7 changed files with 404 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files

194
javascript/ql/lib/semmle/javascript/ApiGraphs.qll
View File

@@ -230,6 +230,72 @@ module API {
result = this.getASuccessor(Label::promisedError())
}
/**
* Gets any class that has this value as a decorator.
*
* For example:
* ```js
* import { D } from "foo";
*
* // moduleImport("foo").getMember("D").getADecoratedClass()
* @D
* class C1 {}
*
* // moduleImport("foo").getMember("D").getReturn().getADecoratedClass()
* @D()
* class C2 {}
* ```
*/
cached
Node getADecoratedClass() { result = this.getASuccessor(Label::decoratedClass()) }
/**
* Gets any method, field, or accessor that has this value as a decorator.
*
* In the case of an accessor, this gets the return value of a getter, or argument to a setter.
*
* For example:
* ```js
* import { D } from "foo";
*
* class C {
* // moduleImport("foo").getMember("D").getADecoratedMember()
* @D m1() {}
* @D f;
* @D get g() { return this.x; }
*
* // moduleImport("foo").getMember("D").getReturn().getADecoratedMember()
* @D() m2() {}
* @D() f2;
* @D() get g2() { return this.x; }
* }
* ```
*/
cached
Node getADecoratedMember() { result = this.getASuccessor(Label::decoratedMember()) }
/**
* Gets any parameter that has this value as a decorator.
*
* For example:
* ```js
* import { D } from "foo";
*
* class C {
* method(
* // moduleImport("foo").getMember("D").getADecoratedParameter()
* @D
* param1,
* // moduleImport("foo").getMember("D").getReturn().getADecoratedParameter()
* @D()
* param2
* ) {}
* }
* ```
*/
cached
Node getADecoratedParameter() { result = this.getASuccessor(Label::decoratedParameter()) }
/**
* Gets a string representation of the lexicographically least among all shortest access paths
* from the root to this node.
@@ -570,6 +636,15 @@ module API {
lbl = Label::memberFromRef(pw)
)
)
or
decoratorDualEdge(base, lbl, rhs)
or
decoratorRhsEdge(base, lbl, rhs)
or
exists(DataFlow::PropWrite write |
decoratorPropEdge(base, lbl, write) and
rhs = write.getRhs()
)
}
/**
@@ -699,6 +774,98 @@ module API {
lbl = Label::parameter(1) and
ref = awaited(call)
)
or
decoratorDualEdge(base, lbl, ref)
or
decoratorUseEdge(base, lbl, ref)
or
// for fields and accessors, mark the reads as use-nodes
decoratorPropEdge(base, lbl, ref.(DataFlow::PropRead))
)
}
/** Holds if `base` is a use-node that flows to the decorator expression of the given decorator. */
pragma[nomagic]
private predicate useNodeFlowsToDecorator(TApiNode base, Decorator decorator) {
exists(DataFlow::SourceNode decoratorSrc |
use(base, decoratorSrc) and
trackUseNode(decoratorSrc).flowsToExpr(decorator.getExpression())
)
}
/**
* Holds if `ref` is both a use and def-node that should an incoming edge from `base` labelled `lbl`.
*
* This happens because the decorated value escapes into the decorator function, and is then replaced
* by the function's return value. In the JS analysis we generally assume decorators return their input,
* but library models may want to find the return value.
*/
private predicate decoratorDualEdge(TApiNode base, Label::ApiLabel lbl, DataFlow::Node ref) {
exists(ClassDefinition cls |
useNodeFlowsToDecorator(base, cls.getADecorator()) and
lbl = Label::decoratedClass() and
ref = DataFlow::valueNode(cls)
)
or
exists(MethodDefinition method |
useNodeFlowsToDecorator(base, method.getADecorator()) and
not method instanceof AccessorMethodDefinition and
lbl = Label::decoratedMember() and
ref = DataFlow::valueNode(method.getBody())
)
or
exists(Parameter param |
useNodeFlowsToDecorator(base, param.getADecorator()) and
lbl = Label::decoratedParameter() and
ref = DataFlow::parameterNode(param)
)
}
/** Holds if `ref` is a use that should have an incoming edge from `base` labelled `lbl`, induced by a decorator. */
private predicate decoratorUseEdge(TApiNode base, Label::ApiLabel lbl, DataFlow::Node ref) {
exists(SetterMethodDefinition accessor |
useNodeFlowsToDecorator(base,
[accessor.getADecorator(), accessor.getCorrespondingGetter().getADecorator()]) and
lbl = Label::decoratedMember() and
ref = DataFlow::parameterNode(accessor.getBody().getParameter(0))
)
}
/** Holds if `rhs` is a def node that should have an incoming edge from `base` labelled `lbl`, induced by a decorator. */
private predicate decoratorRhsEdge(TApiNode base, Label::ApiLabel lbl, DataFlow::Node rhs) {
exists(GetterMethodDefinition accessor |
useNodeFlowsToDecorator(base,
[accessor.getADecorator(), accessor.getCorrespondingSetter().getADecorator()]) and
lbl = Label::decoratedMember() and
rhs = DataFlow::valueNode(accessor.getBody().getAReturnedExpr())
)
}
/**
* Holds if `ref` is a reference to a field/accessor that should have en incoming edge from base labelled `lbl`.
*
* Since fields do not have their own data-flow nodes, we generate a node for each read or write.
* For property writes, the right-hand side becomes a def-node and property reads become use-nodes.
*
* For accessors, we generate nodes both internally in the accessor, and for each use of the accessor. This
* predicate only gets the uses.
*/
private predicate decoratorPropEdge(TApiNode base, Label::ApiLabel lbl, DataFlow::PropRef ref) {
exists(MemberDefinition fieldLike, DataFlow::ClassNode cls |
fieldLike instanceof FieldDefinition
or
fieldLike instanceof AccessorMethodDefinition
|
useNodeFlowsToDecorator(base, fieldLike.getADecorator()) and
lbl = Label::decoratedMember() and
cls = fieldLike.getDeclaringClass().flow() and
(
fieldLike.isStatic() and
ref = cls.getAClassReference().getAPropertyReference(fieldLike.getName())
or
not fieldLike.isStatic() and
ref = cls.getAnInstanceReference().getAPropertyReference(fieldLike.getName())
)
)
}
@@ -1106,6 +1273,15 @@ module API {
/** Gets the `promisedError` edge label connecting a promise to its rejected value. */
LabelPromisedError promisedError() { any() }
/** Gets the label for an edge leading from a value `D` to any class that has `D` as a decorator. */
LabelDecoratedClass decoratedClass() { any() }
/** Gets the label for an edge leading from a value `D` to any method, field, or accessor that has `D` as a decorator. */
LabelDecoratedMethod decoratedMember() { any() }
/** Gets the label for an edge leading from a value `D` to any parameter that has `D` as a decorator. */
LabelDecoratedParameter decoratedParameter() { any() }
/** Gets an entry-point label for the entry-point `e`. */
LabelEntryPoint entryPoint(API::EntryPoint e) { result.getEntryPoint() = e }
@@ -1140,6 +1316,9 @@ module API {
MkLabelReturn() or
MkLabelPromised() or
MkLabelPromisedError() or
MkLabelDecoratedClass() or
MkLabelDecoratedMember() or
MkLabelDecoratedParameter() or
MkLabelEntryPoint(API::EntryPoint e)
/** A label for an entry-point. */
@@ -1229,6 +1408,21 @@ module API {
class LabelReceiver extends ApiLabel, MkLabelReceiver {
override string toString() { result = "receiver" }
}
/** A label for a class decorated by the current value. */
class LabelDecoratedClass extends ApiLabel, MkLabelDecoratedClass {
override string toString() { result = "decorated-class" }
}
/** A label for a method, field, or accessor decorated by the current value. */
class LabelDecoratedMethod extends ApiLabel, MkLabelDecoratedMember {
override string toString() { result = "decorated-member" }
}
/** A label for a parameter decorated by the current value. */
class LabelDecoratedParameter extends ApiLabel, MkLabelDecoratedParameter {
override string toString() { result = "decorated-parameter" }
}
}
}
}

37
javascript/ql/lib/semmle/javascript/Classes.qll
View File

@@ -895,7 +895,14 @@ class SyntheticConstructor extends Function {
* }
* ```
*/
abstract class AccessorMethodDeclaration extends MethodDeclaration { }
abstract class AccessorMethodDeclaration extends MethodDeclaration {
/** Get the corresponding getter (if this is a setter) or setter (if this is a getter). */
AccessorMethodDeclaration getOtherAccessor() {
getterSetterPair(this, result)
or
getterSetterPair(result, this)
}
}
/**
* A concrete accessor method definition in a class, that is, an accessor method with a function body.
@@ -958,6 +965,9 @@ abstract class AccessorMethodSignature extends MethodSignature, AccessorMethodDe
*/
class GetterMethodDeclaration extends AccessorMethodDeclaration, @property_getter {
override string getAPrimaryQlClass() { result = "GetterMethodDeclaration" }
/** Gets the correspinding setter declaration, if any. */
SetterMethodDeclaration getCorrespondingSetter() { getterSetterPair(this, result) }
}
/**
@@ -1020,6 +1030,9 @@ class GetterMethodSignature extends GetterMethodDeclaration, AccessorMethodSigna
*/
class SetterMethodDeclaration extends AccessorMethodDeclaration, @property_setter {
override string getAPrimaryQlClass() { result = "SetterMethodDeclaration" }
/** Gets the correspinding getter declaration, if any. */
GetterMethodDeclaration getCorrespondingGetter() { getterSetterPair(result, this) }
}
/**
@@ -1263,3 +1276,25 @@ class IndexSignature extends @index_signature, MemberSignature {
override string getAPrimaryQlClass() { result = "IndexSignature" }
}
private boolean getStaticness(AccessorMethodDefinition member) {
member.isStatic() and result = true
or
not member.isStatic() and result = false
}
pragma[nomagic]
private AccessorMethodDefinition getAnAccessorFromClass(
ClassDefinition cls, string name, boolean static
) {
result = cls.getMember(name) and
static = getStaticness(result)
}
pragma[nomagic]
private predicate getterSetterPair(GetterMethodDeclaration getter, SetterMethodDeclaration setter) {
exists(ClassDefinition cls, string name, boolean static |
getter = getAnAccessorFromClass(cls, name, static) and
setter = getAnAccessorFromClass(cls, name, static)
)
}

21
javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsSpecific.qll
View File

@@ -124,6 +124,15 @@ API::Node getExtraSuccessorFromNode(API::Node node, AccessPathToken token) {
token.getName() = "Parameter" and
token.getAnArgument() = "this" and
result = node.getReceiver()
or
token.getName() = "DecoratedClass" and
result = node.getADecoratedClass()
or
token.getName() = "DecoratedMember" and
result = node.getADecoratedMember()
or
token.getName() = "DecoratedParameter" and
result = node.getADecoratedParameter()
}
/**
@@ -210,7 +219,11 @@ InvokeNode getAnInvocationOf(API::Node node) { result = node.getAnInvocation() }
*/
bindingset[name]
predicate isExtraValidTokenNameInIdentifyingAccessPath(string name) {
name = ["Member", "Instance", "Awaited", "ArrayElement", "Element", "MapValue", "NewCall", "Call"]
name =
[
"Member", "Instance", "Awaited", "ArrayElement", "Element", "MapValue", "NewCall", "Call",
"DecoratedClass", "DecoratedMember", "DecoratedParameter"
]
}
/**
@@ -218,7 +231,11 @@ predicate isExtraValidTokenNameInIdentifyingAccessPath(string name) {
* in an identifying access path.
*/
predicate isExtraValidNoArgumentTokenInIdentifyingAccessPath(string name) {
name = ["Instance", "Awaited", "ArrayElement", "Element", "MapValue", "NewCall", "Call"]
name =
[
"Instance", "Awaited", "ArrayElement", "Element", "MapValue", "NewCall", "Call",
"DecoratedClass", "DecoratedMember", "DecoratedParameter"
]
}
/**

9
javascript/ql/test/library-tests/frameworks/data/paramDecorator.ts Normal file
View File

@@ -0,0 +1,9 @@
import * as testlib from 'testlib';
// parameter decorators are only valid in TypeScript so this test is in a .ts file
class C {
decoratedParamSource(@testlib.ParamDecorator x) {
sink(x) // NOT OK
}
}

32
javascript/ql/test/library-tests/frameworks/data/test.expected
View File

@@ -1,5 +1,6 @@
consistencyIssue
taintFlow
| paramDecorator.ts:6:48:6:48 | x | paramDecorator.ts:7:10:7:10 | x |
| test.js:5:30:5:37 | source() | test.js:5:8:5:38 | testlib ... urce()) |
| test.js:6:22:6:29 | source() | test.js:6:8:6:30 | preserv ... urce()) |
| test.js:7:41:7:48 | source() | test.js:7:8:7:49 | require ... urce()) |
@@ -35,6 +36,25 @@ taintFlow
| test.js:97:17:97:24 | source() | test.js:97:17:97:24 | source() |
| test.js:102:16:102:34 | testlib.getSource() | test.js:103:8:103:13 | source |
| test.js:102:16:102:34 | testlib.getSource() | test.js:104:8:104:24 | source.continue() |
| test.js:111:12:111:19 | source() | test.js:111:12:111:19 | source() |
| test.js:113:17:113:17 | x | test.js:114:10:114:10 | x |
| test.js:132:10:132:25 | this.fieldSource | test.js:132:10:132:25 | this.fieldSource |
| test.js:133:10:133:37 | OtherCl ... dSource | test.js:133:10:133:37 | OtherCl ... dSource |
| test.js:134:22:134:29 | source() | test.js:134:22:134:29 | source() |
| test.js:135:34:135:41 | source() | test.js:135:34:135:41 | source() |
| test.js:144:16:144:23 | source() | test.js:144:16:144:23 | source() |
| test.js:147:29:147:36 | source() | test.js:147:29:147:36 | source() |
| test.js:150:19:150:19 | x | test.js:151:10:151:10 | x |
| test.js:152:12:152:19 | source() | test.js:152:12:152:19 | source() |
| test.js:156:32:156:32 | x | test.js:157:10:157:10 | x |
| test.js:158:12:158:19 | source() | test.js:158:12:158:19 | source() |
| test.js:162:20:162:20 | x | test.js:163:10:163:10 | x |
| test.js:164:12:164:19 | source() | test.js:164:12:164:19 | source() |
| test.js:172:12:172:19 | source() | test.js:172:12:172:19 | source() |
| test.js:176:23:176:23 | x | test.js:177:10:177:10 | x |
| test.js:182:12:182:19 | source() | test.js:182:12:182:19 | source() |
| test.js:187:31:187:31 | x | test.js:189:10:189:10 | x |
| test.js:203:32:203:39 | source() | test.js:203:32:203:39 | source() |
isSink
| test.js:54:18:54:25 | source() | test-sink |
| test.js:55:22:55:29 | source() | test-sink |
@@ -81,6 +101,18 @@ isSink
| test.js:95:17:95:24 | source() | test-sink |
| test.js:96:17:96:24 | source() | test-sink |
| test.js:97:17:97:24 | source() | test-sink |
| test.js:111:12:111:19 | source() | test-sink |
| test.js:134:22:134:29 | source() | test-sink |
| test.js:135:34:135:41 | source() | test-sink |
| test.js:144:16:144:23 | source() | test-sink |
| test.js:147:29:147:36 | source() | test-sink |
| test.js:152:12:152:19 | source() | test-sink |
| test.js:158:12:158:19 | source() | test-sink |
| test.js:164:12:164:19 | source() | test-sink |
| test.js:172:12:172:19 | source() | test-sink |
| test.js:182:12:182:19 | source() | test-sink |
| test.js:196:12:196:29 | this._wrappedField | test-sink |
| test.js:203:32:203:39 | source() | test-sink |
syntaxErrors
| Member[foo |
| Member[foo] .Member[bar] |

99
javascript/ql/test/library-tests/frameworks/data/test.js
View File

@@ -104,3 +104,102 @@ function testFlowThroughReceiver() {
sink(source.continue()); // NOT OK
sink(source.blah()); // OK
}
@testlib.ClassDecorator
class DecoratedClass {
returnValueIsSink() {
return source(); // NOT OK
}
inputIsSource(x) {
sink(x); // NOT OK
}
}
class OtherClass {
@testlib.FieldDecoratorSink
fieldSink;
@testlib.FieldDecoratorSink
static staticFieldSink;
@testlib.FieldDecoratorSource
fieldSource;
@testlib.FieldDecoratorSource
static staticFieldSource;
useFields() {
sink(this.fieldSource); // NOT OK
sink(OtherClass.staticFieldSource); // NOT OK
this.fieldSink = source(); // NOT OK
OtherClass.staticFieldSink = source(); // NOT OK
sink(this.staticFieldSource); // OK - not a valid field access
sink(OtherClass.fieldSource); // OK - not a valid field access
this.staticFieldSink = source(); // OK - not a valid field access
OtherClass.fieldSink = source(); // OK - not a valid field access
}
@testlib.FieldDecoratorSink
fieldSink2 = source(); // NOT OK
@testlib.FieldDecoratorSink
static staticFieldSink2 = source(); // NOT OK
@testlib.MethodDecorator
decoratedMethod(x) {
sink(x); // NOT OK
return source(); // NOT OK
}
@testlib.MethodDecorator
static decoratedStaticMethod(x) {
sink(x); // NOT OK
return source(); // NOT OK
}
@testlib.MethodDecoratorWithArgs({ something: true })
decoratedMethod2(x) {
sink(x); // NOT OK
return source(); // NOT OK
}
@testlib.FieldDecoratorSink
get sinkViaGetter() {
// If a field with this decorator should be seen as a sink it generally means the framework
// will read it and pass it to an underlying sink. Therefore the return value of its getter
// should be seen as a sink as well.
return source(); // NOT OK
}
@testlib.FieldDecoratorSource
set sourceViaSetter(x) {
sink(x); // NOT OK
}
get sinkViaGetterIndirect() {
// Same as 'sinkViaGetter', but where the decorator is placed on the corresponding setter
return source(); // NOT OK
}
@testlib.FieldDecoratorSink // indirectly decorate the getter above
set sinkViaGetterIndirect(x) {}
set sourceViaSetterIndirect(x) {
// Same as 'sourceViaSetter', but where the decorator is placed on the corresponding getter
sink(x); // NOT OK
}
@testlib.FieldDecoratorSource // indirectly decorate the setter above
get sourceViaSetterIndirect() {}
@testlib.FieldDecoratorSink
get accessorAroundField() {
return this._wrappedField; // OK - the alert occurs at the assignment to 'accessorAroundField'
}
set accessorAroundField(x) {
this._wrappedField = x;
}
useWrappedField() {
this.accessorAroundField = source(); // NOT OK
}
}

16
javascript/ql/test/library-tests/frameworks/data/test.ql
View File

@@ -33,12 +33,26 @@ class Sinks extends ModelInput::SinkModelCsv {
"testlib;;Member[mySinkExceptLast].Argument[0..N-2];test-sink",
"testlib;;Member[mySinkIfArityTwo].WithArity[2].Argument[0];test-sink",
"testlib;;Member[sink1, sink2, sink3 ].Argument[0];test-sink",
"testlib;;Member[ClassDecorator].DecoratedClass.Instance.Member[returnValueIsSink].ReturnValue;test-sink",
"testlib;;Member[FieldDecoratorSink].DecoratedMember;test-sink",
"testlib;;Member[MethodDecorator].DecoratedMember.ReturnValue;test-sink",
"testlib;;Member[MethodDecoratorWithArgs].ReturnValue.DecoratedMember.ReturnValue;test-sink",
]
}
}
class Sources extends ModelInput::SourceModelCsv {
override predicate row(string row) { row = "testlib;;Member[getSource].ReturnValue;test-source" }
override predicate row(string row) {
row =
[
"testlib;;Member[getSource].ReturnValue;test-source",
"testlib;;Member[ClassDecorator].DecoratedClass.Instance.Member[inputIsSource].Parameter[0];test-source",
"testlib;;Member[FieldDecoratorSource].DecoratedMember;test-source",
"testlib;;Member[ParamDecorator].DecoratedParameter;test-source",
"testlib;;Member[MethodDecorator].DecoratedMember.Parameter[0];test-source",
"testlib;;Member[MethodDecoratorWithArgs].ReturnValue.DecoratedMember.Parameter[0];test-source",
]
}
}
class BasicTaintTracking extends TaintTracking::Configuration {