Merge branch 'main' into wrong-number-msg

This commit is contained in:
Geoffrey White
2022-11-08 14:47:19 +00:00
110 changed files with 2704 additions and 292 deletions

View File

@@ -50,19 +50,18 @@ class StrcatFunction extends TaintFunction, DataFlowFunction, ArrayFunction, Sid
}
override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
this.getName() = ["strncat", "wcsncat", "_mbsncat", "_mbsncat_l"] and
input.isParameter(2) and
output.isParameterDeref(0)
or
this.getName() = ["_mbsncat_l", "_mbsnbcat_l"] and
input.isParameter(3) and
output.isParameterDeref(0)
or
input.isParameterDeref(0) and
output.isParameterDeref(0)
or
input.isParameterDeref(1) and
output.isParameterDeref(0)
(
this.getName() = ["strncat", "wcsncat", "_mbsncat", "_mbsncat_l"] and
input.isParameter(2)
or
this.getName() = ["_mbsncat_l", "_mbsnbcat_l"] and
input.isParameter(3)
or
input.isParameterDeref(0)
or
input.isParameterDeref(1)
) and
(output.isParameterDeref(0) or output.isReturnValueDeref())
}
override predicate hasArrayInput(int param) {

View File

@@ -5,7 +5,7 @@ int source();
void sink(...);
bool random();
int test1() {
void test1() {
int x = source();
for (int i = 0; i < 10; i++) {
x = 0;
@@ -13,7 +13,7 @@ int test1() {
sink(x); // $ SPURIOUS: ir
}
int test2(int iterations) {
void test2(int iterations) {
int x = source();
for (int i = 0; i < iterations; i++) {
x = 0;
@@ -21,7 +21,7 @@ int test2(int iterations) {
sink(x); // $ ast,ir
}
int test3() {
void test3() {
int x = 0;
for (int i = 0; i < 10; i++) {
x = source();
@@ -29,7 +29,7 @@ int test3() {
sink(x); // $ ast,ir
}
int test4() {
void test4() {
int x = source();
for (int i = 0; i < 10; i++) {
if (random())
@@ -39,7 +39,7 @@ int test4() {
sink(x); // $ ast,ir
}
int test5() {
void test5() {
int x = source();
for (int i = 0; i < 10; i++) {
if (random())
@@ -49,7 +49,7 @@ int test5() {
sink(x); // $ ast,ir
}
int test6() {
void test6() {
int y;
int x = source();
for (int i = 0; i < 10 && (y = 1); i++) {
@@ -57,7 +57,7 @@ int test6() {
sink(x); // $ ast,ir
}
int test7() {
void test7() {
int y;
int x = source();
for (int i = 0; i < 10 && (y = 1); i++) {
@@ -66,7 +66,7 @@ int test7() {
sink(x); // $ SPURIOUS: ir
}
int test8() {
void test8() {
int x = source();
// It appears to the analysis that the condition can exit after `i < 10`
// without having assigned to `x`. That is an effect of how the
@@ -78,7 +78,7 @@ int test8() {
sink(x); // $ SPURIOUS: ast,ir
}
int test9() {
void test9() {
int y;
int x = source();
for (int i = 0; (y = 1) && i < 10; i++) {
@@ -86,21 +86,21 @@ int test9() {
sink(x); // $ ast,ir
}
int test10() {
void test10() {
int x = source();
for (int i = 0; (x = 1) && i < 10; i++) {
}
sink(x); // no flow
}
int test10(int b, int d) {
void test10(int b, int d) {
int i = 0;
int x = source();
if (b)
goto L;
for (; i < 10; i += d) {
x = 0;
L:
L: ;
}
sink(x); // $ ir MISSING: ast
}

View File

@@ -5964,6 +5964,7 @@
| taint.cpp:172:10:172:15 | buffer | taint.cpp:172:3:172:8 | call to strcat | |
| taint.cpp:172:10:172:15 | buffer | taint.cpp:172:10:172:15 | ref arg buffer | TAINT |
| taint.cpp:172:10:172:15 | ref arg buffer | taint.cpp:173:8:173:13 | buffer | |
| taint.cpp:172:18:172:24 | tainted | taint.cpp:172:3:172:8 | call to strcat | TAINT |
| taint.cpp:172:18:172:24 | tainted | taint.cpp:172:10:172:15 | ref arg buffer | TAINT |
| taint.cpp:180:19:180:19 | p | taint.cpp:180:19:180:19 | p | |
| taint.cpp:180:19:180:19 | p | taint.cpp:181:9:181:9 | p | |
@@ -6373,12 +6374,14 @@
| taint.cpp:561:9:561:13 | dest1 | taint.cpp:561:9:561:13 | ref arg dest1 | TAINT |
| taint.cpp:561:9:561:13 | ref arg dest1 | taint.cpp:560:24:560:28 | dest1 | |
| taint.cpp:561:9:561:13 | ref arg dest1 | taint.cpp:562:7:562:11 | dest1 | |
| taint.cpp:561:16:561:21 | source | taint.cpp:561:2:561:7 | call to strcat | TAINT |
| taint.cpp:561:16:561:21 | source | taint.cpp:561:9:561:13 | ref arg dest1 | TAINT |
| taint.cpp:562:7:562:11 | ref arg dest1 | taint.cpp:560:24:560:28 | dest1 | |
| taint.cpp:564:9:564:13 | dest2 | taint.cpp:564:2:564:7 | call to strcat | |
| taint.cpp:564:9:564:13 | dest2 | taint.cpp:564:9:564:13 | ref arg dest2 | TAINT |
| taint.cpp:564:9:564:13 | ref arg dest2 | taint.cpp:560:37:560:41 | dest2 | |
| taint.cpp:564:9:564:13 | ref arg dest2 | taint.cpp:565:7:565:11 | dest2 | |
| taint.cpp:564:16:564:20 | clean | taint.cpp:564:2:564:7 | call to strcat | TAINT |
| taint.cpp:564:16:564:20 | clean | taint.cpp:564:9:564:13 | ref arg dest2 | TAINT |
| taint.cpp:565:7:565:11 | ref arg dest2 | taint.cpp:560:37:560:41 | dest2 | |
| taint.cpp:572:37:572:41 | dest1 | taint.cpp:572:37:572:41 | dest1 | |
@@ -6405,9 +6408,12 @@
| taint.cpp:574:36:574:40 | ref arg dest1 | taint.cpp:572:37:572:41 | dest1 | |
| taint.cpp:574:36:574:40 | ref arg dest1 | taint.cpp:575:7:575:11 | dest1 | |
| taint.cpp:574:36:574:40 | ref arg dest1 | taint.cpp:576:8:576:12 | dest1 | |
| taint.cpp:574:43:574:45 | ptr | taint.cpp:574:25:574:34 | call to _mbsncat_l | TAINT |
| taint.cpp:574:43:574:45 | ptr | taint.cpp:574:36:574:40 | ref arg dest1 | TAINT |
| taint.cpp:574:48:574:48 | n | taint.cpp:574:25:574:34 | call to _mbsncat_l | TAINT |
| taint.cpp:574:48:574:48 | n | taint.cpp:574:36:574:40 | ref arg dest1 | TAINT |
| taint.cpp:574:51:574:56 | ref arg source | taint.cpp:573:49:573:54 | source | |
| taint.cpp:574:51:574:56 | source | taint.cpp:574:25:574:34 | call to _mbsncat_l | TAINT |
| taint.cpp:574:51:574:56 | source | taint.cpp:574:36:574:40 | ref arg dest1 | TAINT |
| taint.cpp:575:7:575:11 | ref arg dest1 | taint.cpp:572:37:572:41 | dest1 | |
| taint.cpp:575:7:575:11 | ref arg dest1 | taint.cpp:576:8:576:12 | dest1 | |
@@ -6421,8 +6427,11 @@
| taint.cpp:580:36:580:40 | ref arg dest3 | taint.cpp:572:85:572:89 | dest3 | |
| taint.cpp:580:36:580:40 | ref arg dest3 | taint.cpp:581:7:581:11 | dest3 | |
| taint.cpp:580:36:580:40 | ref arg dest3 | taint.cpp:582:8:582:12 | dest3 | |
| taint.cpp:580:43:580:45 | ptr | taint.cpp:580:25:580:34 | call to _mbsncat_l | TAINT |
| taint.cpp:580:43:580:45 | ptr | taint.cpp:580:36:580:40 | ref arg dest3 | TAINT |
| taint.cpp:580:48:580:48 | n | taint.cpp:580:25:580:34 | call to _mbsncat_l | TAINT |
| taint.cpp:580:48:580:48 | n | taint.cpp:580:36:580:40 | ref arg dest3 | TAINT |
| taint.cpp:580:51:580:55 | clean | taint.cpp:580:25:580:34 | call to _mbsncat_l | TAINT |
| taint.cpp:580:51:580:55 | clean | taint.cpp:580:36:580:40 | ref arg dest3 | TAINT |
| taint.cpp:580:51:580:55 | ref arg clean | taint.cpp:573:32:573:36 | clean | |
| taint.cpp:581:7:581:11 | ref arg dest3 | taint.cpp:572:85:572:89 | dest3 | |

View File

@@ -574,8 +574,8 @@ void test__mbsncat_l(unsigned char* dest1, unsigned const char* ptr, unsigned ch
unsigned char* dest2 = _mbsncat_l(dest1, ptr, n, source);
sink(dest1); // $ SPURIOUS: ast,ir
sink(*dest1); // $ ast,ir
sink(dest2); // $ SPURIOUS: ir
sink(*dest2); // $ ir
sink(dest2); // $ SPURIOUS: ast,ir
sink(*dest2); // $ ast,ir
unsigned char* dest4 = _mbsncat_l(dest3, ptr, n, clean);
sink(dest3);

View File

@@ -819,13 +819,15 @@ A class definition has the following syntax:
::
class ::= qldoc? annotations "class" classname "extends" type ("," type)* "{" member* "}"
class ::= qldoc? annotations "class" classname ("extends" type ("," type)*)? ("instanceof" type ("," type)*)? "{" member* "}"
The identifier following the ``class`` keyword is the name of the class.
The types specified after the ``extends`` keyword are the *base types* of the class.
A class domain type is said to *inherit* from the base types of the associated class type, a character type is said to *inherit* from its associated class domain type and a class type is said to *inherit* from its associated character type. In addition, inheritance is transitive: If a type ``A`` inherits from a type ``B``, and ``B`` inherits from a type ``C``, then ``A`` inherits from ``C``.
The types specified after the ``instanceof`` keyword are the *instanceof types* of the class.
A class type is said to *inherit* from the base types. In addition, inheritance is transitive: If a type ``A`` inherits from a type ``B``, and ``B`` inherits from a type ``C``, then ``A`` inherits from ``C``.
A class adds a mapping from the class name to the class declaration to the current module's declared type environment.
@@ -833,6 +835,20 @@ A valid class can be annotated with ``abstract``, ``final``, ``library``, and ``
A valid class may not inherit from a final class, from itself, or from more than one primitive type.
A valid class must have at least one base type or instanceof type.
Class dependencies
~~~~~~~~~~~~~~~~~~
The program is invalid if there is a cycle of class dependencies.
The following are class dependencies:
- ``C`` depends on ``C.C``
- ``C.C`` depends on ``C.extends``
- If ``C`` is abstract then it depends on all classes ``D`` such that ``C`` is a base type of ``D``.
- ``C.extends`` depends on ``D.D`` for each base type ``D`` of ``C``.
- ``C.extends`` depends on ``D`` for each instanceof type ``D`` of ``C``.
Class environments
~~~~~~~~~~~~~~~~~~
@@ -848,7 +864,9 @@ For each of member predicates and fields a class *inherits* and *declares*, and
The program is invalid if any of these environments is not definite.
For each of member predicates and fields a domain type *exports* an environment. This is the union of the exported ``X`` environments of types the class inherits from, excluding any elements that are ``overridden`` by another element.
For each of member predicates and fields a domain type *exports* an environment. We say the *exported X extends environment* is the union of the exported ``X`` environments of types the class inherits from, excluding any elements that are ``overridden`` by another element.
We say the *exported X instanceof environement* is the union of the exported ``X`` environments of types that a instanceof type inherits from, excluding any elements that are ``overridden`` by another element.
The *exported X environment* of the domain type is the union of the exported ``X`` extends environment and the exported ``X`` instanceof environement.
Members
~~~~~~~
@@ -1090,11 +1108,7 @@ A super expression has the following syntax:
super_expr ::= "super" | type "." "super"
For a super expression to be valid, the ``this`` keyword must have a type and value in the typing environment. The type of the expression is the same as the type of ``this`` in the typing environment.
A super expression may only occur in a QL program as the receiver expression for a predicate call.
If a super expression includes a ``type``, then that type must be a class that the enclosing class inherits from.
For a super expression to be valid, the ``this`` keyword must have a type and value in the typing environment. The type of the expression is the same as the domain type of the type of ``this`` in the typing environment.
The value of a super expression is the same as the value of ``this`` in the named tuple.
@@ -1147,13 +1161,6 @@ A valid call with results *resolves* to a set of predicates. The ways a call can
- If the call has no receiver and the predicate name is a selection identifier, then the qualifier is resolved as a module (see "`Module resolution <#module-resolution>`__"). The identifier is then resolved in the exported predicate environment of the qualifier module.
- If the call has a super expression as the receiver, then it resolves to a member predicate in a class that the enclosing class inherits from:
- If the super expression is unqualified and there is a single class that the current class inherits from, then the super-class is that class.
- If the super expression is unqualified and there are multiple classes that the current class inherits from, then the super-class is the domain type.
- Otherwise, the super-class is the class named by the qualifier of the super expression.
The predicate is resolved by looking up its name and arity in the exported predicate environment of the super-class.
- If the type of the receiver is the same as the enclosing class, the predicate is resolved by looking up its name and arity in the visible predicate environment of the class.
- If the type of the receiver is not the same as the enclosing class, the predicate is resolved by looking up its name and arity in the exported predicate environment of the class or domain type.
@@ -1177,11 +1184,20 @@ If the resolved predicate is built in, then the call may not include a closure.
If the call includes a closure, then all declared predicate arguments, the enclosing type of the declaration (if it exists), and the result type of the declaration (if it exists) must be compatible. If one of those types is a subtype of ``int``, then all the other arguments must be a subtype of ``int``.
A call to a member predicate may be a *direct* call:
- If the receiver is not a super expression it is not direct.
- If the receiver is ``A.super`` and ``A`` is an instanceof type and not a base type then it is not direct.
- If the receiver is ``A.super`` and ``A`` is a base type type and not an instanceof type then it is direct.
- If the receiver is ``A.super`` and ``A`` is a base type and an instanceof type then the call is not valid.
- If the receiver is ``super`` and the member predicate is in the exported member predicate environment of an instanceof type and not in the exported member predicate environment of a base type then it isn't direct.
- If the receiver is ``super`` and the member predicate is in the exported member predicate environment of a base type and not in the exported member predicate environment of an instanceof type then it is direct.
- If the receiver is ``super`` and the member predicate is in the exported member predicate environment of a base type and in the exported member predicate environment of an instanceof type then the call is not valid.
If the call resolves to a member predicate, then the *receiver values* are as follows. If the call has a receiver, then the receiver values are the values of that receiver. If the call does not have a receiver, then the single receiver value is the value of ``this`` in the contextual named tuple.
The *tuple prefixes* of a call with results include one value from each of the argument expressions' values, in the same order as the order of the arguments. If the call resolves to a non-member predicate, then those values are exactly the tuple prefixes of the call. If the call instead resolves to a member predicate, then the tuple prefixes additionally include a receiver value, ordered before the argument values.
The *matching tuples* of a call with results are all ordered tuples that are one of the tuple prefixes followed by any value of the same type as the call. If the call has no closure, then all matching tuples must additionally satisfy the resolved predicate of the call, unless the receiver is a super expression, in which case they must *directly* satisfy the resolved predicate of the call. If the call has a ``*`` or ``+`` closure, then the matching tuples must satisfy or directly satisfy the associated closure of the resolved predicate.
The *matching tuples* of a call with results are all ordered tuples that are one of the tuple prefixes followed by any value of the same type as the call. If the call has no closure, then all matching tuples must additionally satisfy the resolved predicate of the call, unless the call is direct in which case they must *directly* satisfy the resolved predicate of the call. If the call has a ``*`` or ``+`` closure, then the matching tuples must satisfy or directly satisfy the associated closure of the resolved predicate.
The values of a call with results are the final elements of each of the call's matching tuples.
@@ -1534,13 +1550,15 @@ The identifier is called the *predicate name* of the call.
A call must resolve to a predicate, using the same definition of resolve as for calls with results (see "`Calls with results <#calls-with-results>`__").
A call may be direct using the same definition of direct as for calls with results (see "`Calls with results <#calls-with-results>`__").
The resolved predicate must not have a result type.
If the resolved predicate is a built-in member predicate of a primitive type, then the call may not include a closure. If the call does have a closure, then the call must resolve to a predicate with relational arity of 2.
The *candidate tuples* of a call are the ordered tuples formed by selecting a value from each of the arguments of the call.
If the call has no closure, then it matches whenever one of the candidate tuples satisfies the resolved predicate of the call, unless the call has a super expression as a receiver, in which case the candidate tuple must *directly* satisfy the resolved predicate. If the call has ``*`` or ``+`` closure, then the call matches whenever one of the candidate tuples satisfies or directly satisfies the associated closure of the resolved predicate.
If the call has no closure, then it matches whenever one of the candidate tuples satisfies the resolved predicate of the call, unless the call is direct, in which case the candidate tuple must *directly* satisfy the resolved predicate. If the call has ``*`` or ``+`` closure, then the call matches whenever one of the candidate tuples satisfies or directly satisfies the associated closure of the resolved predicate.
Disambiguation of formulas
~~~~~~~~~~~~~~~~~~~~~~~~~~
@@ -1926,10 +1944,12 @@ Predicates, and types can *depend* and *strictly depend* on each other. Such dep
- A predicate containing a predicate call depends on the predicate to which the call resolves. If the call has negative or zero polarity then the dependency is strict.
- A predicate containing a predicate call, which resolves to a member predicate and does not have a ``super`` expression as a qualifier, depends on the dispatch dependencies of the root definitions of the target of the call. If the call has negative or zero polarity then the dependencies are strict. The predicate strictly depends on the strict dispatch dependencies of the root definitions.
- A predicate containing a predicate call, which resolves to a member predicate, where the call is not direct, depends on the dispatch dependencies of the root definitions of the target of the call. If the call has negative or zero polarity then the dependencies are strict. The predicate strictly depends on the strict dispatch dependencies of the root definitions.
- For each class ``C`` in the program, for each base class ``B`` of ``C``, ``C.extends`` depends on ``B.B``.
- For each class ``C`` in the program, for each instanceof type ``B`` of ``C``, ``C.extends`` depends on ``B``.
- For each class ``C`` in the program, for each base type ``B`` of ``C`` that is not a class type, ``C.extends`` depends on ``B``.
- For each class ``C`` in the program, ``C.class`` depends on ``C.C``.
@@ -1976,6 +1996,7 @@ Each layer of the stratification is *populated* in order. To populate a layer, e
- To populate the type ``C.extends`` for a class ``C``, identify each named tuple that has the following properties:
- The value of ``this`` is in all non-class base types of ``C``.
- The value of ``this`` is in all instanceof types of ``C``.
- The keys of the tuple are ``this`` and the union of the public fields from each base type.
- For each class base type ``B`` of ``C`` there is a named tuple with variables from the public fields of ``B`` and ``this`` that the given tuple and some tuple in ``B.B`` both extend.
@@ -2060,7 +2081,7 @@ The complete grammar for QL is as follows:
| "{" formula "}"
| "=" literalId "(" (predicateRef "/" int ("," predicateRef "/" int)*)? ")" "(" (exprs)? ")"
class ::= qldoc? annotations "class" classname "extends" type ("," type)* "{" member* "}"
class ::= qldoc? annotations "class" classname ("extends" type ("," type)*)? ("instanceof" type ("," type)*)? "{" member* "}"
member ::= character | predicate | field

View File

@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* Added data flow summaries for tainted Android intents sent to activities via `Activity.startActivities`.

View File

@@ -26,6 +26,9 @@ module SummaryComponent {
/** Gets a summary component for `Element`. */
SummaryComponent element() { result = content(any(CollectionContent c)) }
/** Gets a summary component for `ArrayElement`. */
SummaryComponent arrayElement() { result = content(any(ArrayContent c)) }
/** Gets a summary component for `MapValue`. */
SummaryComponent mapValue() { result = content(any(MapValueContent c)) }
@@ -52,6 +55,11 @@ module SummaryComponentStack {
result = push(SummaryComponent::element(), object)
}
/** Gets a stack representing `ArrayElement` of `object`. */
SummaryComponentStack arrayElementOf(SummaryComponentStack object) {
result = push(SummaryComponent::arrayElement(), object)
}
/** Gets a stack representing `MapValue` of `object`. */
SummaryComponentStack mapValueOf(SummaryComponentStack object) {
result = push(SummaryComponent::mapValue(), object)

View File

@@ -1,7 +1,10 @@
import java
private import semmle.code.java.frameworks.android.Android
private import semmle.code.java.dataflow.DataFlow
private import semmle.code.java.dataflow.ExternalFlow
private import semmle.code.java.dataflow.FlowSteps
private import semmle.code.java.dataflow.FlowSummary
private import semmle.code.java.dataflow.internal.BaseSSA as BaseSsa
/** The class `android.content.Intent`. */
class TypeIntent extends Class {
@@ -242,19 +245,52 @@ private class StartComponentMethodAccess extends MethodAccess {
/** Gets the intent argument of this call. */
Argument getIntentArg() {
result.getType() instanceof TypeIntent and
(
result.getType() instanceof TypeIntent or
result.getType().(Array).getElementType() instanceof TypeIntent
) and
result = this.getAnArgument()
}
/** Holds if this targets a component of type `targetType`. */
predicate targetsComponentType(RefType targetType) {
predicate targetsComponentType(AndroidComponent targetType) {
exists(NewIntent newIntent |
DataFlow::localExprFlow(newIntent, this.getIntentArg()) and
reaches(newIntent, this.getIntentArg()) and
newIntent.getClassArg().getType().(ParameterizedType).getATypeArgument() = targetType
)
}
}
/**
* Holds if `src` reaches the intent argument `arg` of `StartComponentMethodAccess`
* through intra-procedural steps.
*/
private predicate reaches(Expr src, Argument arg) {
any(StartComponentMethodAccess ma).getIntentArg() = arg and
src = arg
or
exists(Expr mid, BaseSsa::BaseSsaVariable ssa, BaseSsa::BaseSsaUpdate upd |
reaches(mid, arg) and
mid = ssa.getAUse() and
upd = ssa.getAnUltimateLocalDefinition() and
src = upd.getDefiningExpr().(VariableAssign).getSource()
)
or
exists(CastingExpr e | e.getExpr() = src | reaches(e, arg))
or
exists(ChooseExpr e | e.getAResultExpr() = src | reaches(e, arg))
or
exists(AssignExpr e | e.getSource() = src | reaches(e, arg))
or
exists(ArrayCreationExpr e | e.getInit().getAnInit() = src | reaches(e, arg))
or
exists(StmtExpr e | e.getResultExpr() = src | reaches(e, arg))
or
exists(NotNullExpr e | e.getExpr() = src | reaches(e, arg))
or
exists(WhenExpr e | e.getBranch(_).getAResult() = src | reaches(e, arg))
}
/**
* A value-preserving step from the intent argument of a `startActivity` call to
* a `getIntent` call in the activity the intent targeted in its constructor.
@@ -271,6 +307,87 @@ private class StartActivityIntentStep extends AdditionalValueStep {
}
}
/**
* Holds if `targetType` is targeted by an existing `StartComponentMethodAccess` call
* and it's identified by `id`.
*/
private predicate isTargetableType(AndroidComponent targetType, string id) {
exists(StartComponentMethodAccess ma | ma.targetsComponentType(targetType)) and
targetType.getQualifiedName() = id
}
private class StartActivitiesSyntheticCallable extends SyntheticCallable {
AndroidComponent targetType;
StartActivitiesSyntheticCallable() {
exists(string id |
this = "android.content.Activity.startActivities()+" + id and
isTargetableType(targetType, id)
)
}
override StartComponentMethodAccess getACall() {
result.getMethod().hasName("startActivities") and
result.targetsComponentType(targetType)
}
override predicate propagatesFlow(
SummaryComponentStack input, SummaryComponentStack output, boolean preservesValue
) {
exists(ActivityIntentSyntheticGlobal glob | glob.getTargetType() = targetType |
input = SummaryComponentStack::arrayElementOf(SummaryComponentStack::argument(0)) and
output = SummaryComponentStack::singleton(SummaryComponent::syntheticGlobal(glob)) and
preservesValue = true
)
}
}
private class GetIntentSyntheticCallable extends SyntheticCallable {
AndroidComponent targetType;
GetIntentSyntheticCallable() {
exists(string id |
this = "android.content.Activity.getIntent()+" + id and
isTargetableType(targetType, id)
)
}
override Call getACall() {
result.getCallee() instanceof AndroidGetIntentMethod and
result.getEnclosingCallable().getDeclaringType() = targetType
}
override predicate propagatesFlow(
SummaryComponentStack input, SummaryComponentStack output, boolean preservesValue
) {
exists(ActivityIntentSyntheticGlobal glob | glob.getTargetType() = targetType |
input = SummaryComponentStack::singleton(SummaryComponent::syntheticGlobal(glob)) and
output = SummaryComponentStack::return() and
preservesValue = true
)
}
}
private class ActivityIntentSyntheticGlobal extends SummaryComponent::SyntheticGlobal {
AndroidComponent targetType;
ActivityIntentSyntheticGlobal() {
exists(string id |
this = "ActivityIntentSyntheticGlobal+" + id and
isTargetableType(targetType, id)
)
}
AndroidComponent getTargetType() { result = targetType }
}
private class RequiredComponentStackForStartActivities extends RequiredSummaryComponentStack {
override predicate required(SummaryComponent head, SummaryComponentStack tail) {
head = SummaryComponent::arrayElement() and
tail = SummaryComponentStack::argument(0)
}
}
/**
* A value-preserving step from the intent argument of a `sendBroadcast` call to
* the intent parameter in the `onReceive` method of the receiver the

View File

@@ -26,5 +26,10 @@ where
not exists(getARead(v)) and
// Discarded exceptions are covered by another query.
not exists(CatchClause cc | cc.getVariable().getVariable() = v) and
// Exclude common Kotlin pattern to do something n times: `for(i in 1..n) { doSomething() }
not exists(EnhancedForStmt f |
f.getVariable().getVariable() = v and
f.getExpr().getType().(RefType).hasQualifiedName("kotlin.ranges", ["IntRange", "LongRange"])
) and
not readImplicitly(v)
select v, "Variable '" + v + "' is never read."

View File

@@ -57,6 +57,8 @@ private predicate candidateMethod(RefType t, Method m, string name, int numParam
m.getNumberOfParameters() = numParam and
m = m.getSourceDeclaration() and
not m.getAnAnnotation() instanceof DeprecatedAnnotation and
// Exclude compiler generated methods, such as Kotlin `$default` methods:
not m.isCompilerGenerated() and
not whitelist(name)
}

View File

@@ -7,3 +7,8 @@ class C {
prop(this)
}
}
class A {
fun <T : Any> fn(value: T, i: Int = 1) {}
fun fn(value: String, i: Int = 1) {}
}

View File

@@ -0,0 +1,3 @@
| test.kt:8:10:8:10 | int e | Variable 'int e' is never read. |
| test.kt:14:11:14:13 | int idx | Variable 'int idx' is never read. |
| test.kt:14:16:14:16 | int e | Variable 'int e' is never read. |

View File

@@ -0,0 +1 @@
Violations of Best Practice/Dead Code/UnreadLocal.ql

View File

@@ -0,0 +1,25 @@
fun fn0(size: Int) {
for (idx in 1..size) {
println()
}
}
fun fn1(a: Array<Int>) {
for (e in a) {
println()
}
}
fun fn2(a: Array<Int>) {
for ((idx, e) in a.withIndex()) {
println()
}
}
fun fn3() {
for (i in 1 until 10) {
println()
}
}
// Diagnostic Matches: % Couldn't find a Java equivalent function to kotlin.Int.rangeTo in java.lang.Integer%

View File

@@ -24,6 +24,12 @@ public class TestStartActivityToGetIntent {
Intent[] intents = new Intent[] {intent};
ctx.startActivities(intents);
}
{
Intent intent = new Intent(null, AnotherActivity.class);
intent.putExtra("data", (String) source("ctx-start-acts-2"));
Intent[] intents = new Intent[] {intent};
ctx.startActivities(intents);
}
{
Intent intent = new Intent(null, SomeActivity.class);
intent.putExtra("data", (String) source("act-start"));
@@ -35,6 +41,12 @@ public class TestStartActivityToGetIntent {
Intent[] intents = new Intent[] {intent};
act.startActivities(intents);
}
{
Intent intent = new Intent(null, Object.class);
intent.putExtra("data", (String) source("start-activities-should-not-reach"));
Intent[] intents = new Intent[] {intent};
act.startActivities(intents);
}
{
Intent intent = new Intent(null, SomeActivity.class);
intent.putExtra("data", (String) source("start-for-result"));
@@ -79,9 +91,16 @@ public class TestStartActivityToGetIntent {
static class SomeActivity extends Activity {
public void test() {
sink(getIntent().getStringExtra("data")); // $ hasValueFlow=ctx-start hasValueFlow=act-start hasValueFlow=start-for-result hasValueFlow=start-if-needed hasValueFlow=start-matching hasValueFlow=start-from-child hasValueFlow=start-from-frag hasValueFlow=4-arg MISSING: hasValueFlow=ctx-start-acts hasValueFlow=act-start-acts
// @formatter:off
sink(getIntent().getStringExtra("data")); // $ hasValueFlow=ctx-start hasValueFlow=act-start hasValueFlow=start-for-result hasValueFlow=start-if-needed hasValueFlow=start-matching hasValueFlow=start-from-child hasValueFlow=start-from-frag hasValueFlow=4-arg hasValueFlow=ctx-start-acts hasValueFlow=act-start-acts
// @formatter:on
}
}
static class AnotherActivity extends Activity {
public void test() {
sink(getIntent().getStringExtra("data")); // $ hasValueFlow=ctx-start-acts-2
}
}
static class SafeActivity extends Activity {

View File

@@ -1,5 +1,5 @@
name: codeql/javascript-experimental-atm-lib
version: 0.3.7
version: 0.4.1
extractor: javascript
library: true
groups:

View File

@@ -1,5 +1,5 @@
name: codeql/javascript-experimental-atm-model
version: 0.2.1
version: 0.3.1
groups:
- javascript
- experimental

View File

@@ -1,6 +1,6 @@
---
dependencies:
codeql/javascript-experimental-atm-model:
version: 0.2.1-2022-09-06-08h55m54s.bubbly-basin-xpztl8fh.f3c3c9360a727959e428ecc6932257e6a546dc65d8a9baac525a49247123822d
version: 0.3.0
compiled: false
lockVersion: 1.0.0

View File

@@ -6,4 +6,4 @@ groups:
- experimental
dependencies:
codeql/javascript-experimental-atm-lib: ${workspace}
codeql/javascript-experimental-atm-model: "0.2.1-2022-09-06-08h55m54s.bubbly-basin-xpztl8fh.f3c3c9360a727959e428ecc6932257e6a546dc65d8a9baac525a49247123822d"
codeql/javascript-experimental-atm-model: "0.3.0"

View File

@@ -1,6 +1,6 @@
---
dependencies:
codeql/javascript-experimental-atm-model:
version: 0.2.1-2022-09-06-08h55m54s.bubbly-basin-xpztl8fh.f3c3c9360a727959e428ecc6932257e6a546dc65d8a9baac525a49247123822d
version: 0.3.0
compiled: false
lockVersion: 1.0.0

View File

@@ -1,6 +1,6 @@
name: codeql/javascript-experimental-atm-queries
language: javascript
version: 0.3.7
version: 0.4.1
suites: codeql-suites
defaultSuiteFile: codeql-suites/javascript-atm-code-scanning.qls
groups:
@@ -8,4 +8,4 @@ groups:
- experimental
dependencies:
codeql/javascript-experimental-atm-lib: ${workspace}
codeql/javascript-experimental-atm-model: "0.2.1-2022-09-06-08h55m54s.bubbly-basin-xpztl8fh.f3c3c9360a727959e428ecc6932257e6a546dc65d8a9baac525a49247123822d"
codeql/javascript-experimental-atm-model: "0.3.0"

View File

@@ -1,6 +1,6 @@
---
dependencies:
codeql/javascript-experimental-atm-model:
version: 0.2.1-2022-09-06-08h55m54s.bubbly-basin-xpztl8fh.f3c3c9360a727959e428ecc6932257e6a546dc65d8a9baac525a49247123822d
version: 0.3.0
compiled: false
lockVersion: 1.0.0

View File

@@ -233,6 +233,8 @@ module AccessPath {
baseName = fromReference(write.getBase(), root)
or
baseName = fromRhs(write.getBase(), root)
or
baseName = fromRhs(GetLaterAccess::getLaterBaseAccess(write), root)
)
or
exists(GlobalVariable var |
@@ -266,6 +268,100 @@ module AccessPath {
)
}
/** A module for computing an access to a variable that happens after a property has been written onto it */
private module GetLaterAccess {
/**
* Gets an access to a variable that is written to in `write`, where the access is after the write.
*
* This allows `fromRhs` to compute an access path for e.g. the below example:
* ```JavaScript
* function foo(x) {
* var obj = {
* bar: x // `x` has the access path "foo.bar" starting from the root `this`.
* };
* this.foo = obj;
* }
* ```
*/
pragma[noopt]
DataFlow::Node getLaterBaseAccess(DataFlow::PropWrite write) {
exists(
ControlFlowNode writeNode, BindingPattern access, VarRef otherAccess, Variable variable,
StmtContainer container
|
access = getBaseVar(write) and
writeNode = write.getWriteNode() and
access = getAnAccessInContainer(variable, container, true) and
variable = getARelevantVariable() and // manual magic
otherAccess = getAnAccessInContainer(variable, container, false) and
access != otherAccess and
result.asExpr() = otherAccess
|
exists(BasicBlock bb, int i, int j |
bb.getNode(i) = writeNode and
bb.getNode(j) = otherAccess and
i < j
)
or
otherAccess.getBasicBlock() = getASuccessorBBThatReadsVar(write) // more manual magic - outlined into a helper predicate.
)
}
/** Gets a variable ref that `write` writes a property to. */
VarRef getBaseVar(DataFlow::PropWrite write) {
result = write.getBase().asExpr()
or
exists(Assignment assign |
write.getBase().asExpr() = assign.getRhs() and
result = assign.getLhs()
)
or
exists(VariableDeclarator decl |
write.getBase().asExpr() = decl.getInit() and
result = decl.getBindingPattern()
)
}
/** Gets an access to `var` inside `container` where `usedInWrite` indicates whether the access is the base of a property write. */
private VarRef getAnAccessInContainer(Variable var, StmtContainer container, boolean usedInWrite) {
result.getVariable() = var and
result.getContainer() = container and
var.isLocal() and
if result = getBaseVar(_) then usedInWrite = true else usedInWrite = false
}
/** Gets a variable that is relevant for the computations in the `GetLaterAccess` module. */
private Variable getARelevantVariable() {
// The variable might be used where `getLaterBaseAccess()` is called.
exists(DataFlow::Node node |
exists(fromRhs(node, _)) and
node.asExpr().(VarAccess).getVariable() = result
) and
// There is a write that writes to the variable.
getBaseVar(_).getVariable() = result and
// It's local.
result.isLocal() and // we skip global variables, because that turns messy quick.
// There is both a "write" and "read" in the same container of the variable.
exists(StmtContainer container |
exists(getAnAccessInContainer(result, container, true)) and // a "write", an access to the variable that is the base of a property reference.
exists(getAnAccessInContainer(result, container, false)) // a "read", an access to the variable that is not the base of a property reference.
)
}
/** Gets a basic-block that has a read of the variable that is written to by `write`, where the basicblock occurs after `start`. */
private ReachableBasicBlock getASuccessorBBThatReadsVar(DataFlow::PropWrite write) {
exists(VarAccess baseExpr, Variable var, ControlFlowNode writeNode |
baseExpr = getBaseVar(write) and
var = baseExpr.getVariable() and
var = getARelevantVariable() and
writeNode = write.getWriteNode() and
writeNode.getBasicBlock().(ReachableBasicBlock).strictlyDominates(result) and
// manual magic.
result = getAnAccessInContainer(getARelevantVariable(), _, false).getBasicBlock()
)
}
}
/**
* Gets a node that refers to the given access path relative to the given `root` node,
* or `root` itself if the access path is empty.

View File

@@ -1692,10 +1692,10 @@ module DataFlow {
*/
predicate localFieldStep(DataFlow::Node pred, DataFlow::Node succ) {
exists(ClassNode cls, string prop |
pred = cls.getADirectSuperClass*().getAReceiverNode().getAPropertyWrite(prop).getRhs() or
pred = AccessPath::getAnAssignmentTo(cls.getADirectSuperClass*().getAReceiverNode(), prop) or
pred = cls.getInstanceMethod(prop)
|
succ = cls.getAReceiverNode().getAPropertyRead(prop)
succ = AccessPath::getAReferenceTo(cls.getAReceiverNode(), prop)
)
}

View File

@@ -170,6 +170,10 @@ nodes
| lib/lib.js:277:23:277:26 | opts |
| lib/lib.js:277:23:277:30 | opts.bla |
| lib/lib.js:277:23:277:30 | opts.bla |
| lib/lib.js:279:19:279:22 | opts |
| lib/lib.js:279:19:279:26 | opts.bla |
| lib/lib.js:281:23:281:35 | this.opts.bla |
| lib/lib.js:281:23:281:35 | this.opts.bla |
| lib/lib.js:307:39:307:42 | name |
| lib/lib.js:307:39:307:42 | name |
| lib/lib.js:308:23:308:26 | name |
@@ -504,8 +508,13 @@ edges
| lib/lib.js:268:22:268:24 | obj | lib/lib.js:268:22:268:32 | obj.version |
| lib/lib.js:276:8:276:11 | opts | lib/lib.js:277:23:277:26 | opts |
| lib/lib.js:276:8:276:11 | opts | lib/lib.js:277:23:277:26 | opts |
| lib/lib.js:276:8:276:11 | opts | lib/lib.js:279:19:279:22 | opts |
| lib/lib.js:276:8:276:11 | opts | lib/lib.js:279:19:279:22 | opts |
| lib/lib.js:277:23:277:26 | opts | lib/lib.js:277:23:277:30 | opts.bla |
| lib/lib.js:277:23:277:26 | opts | lib/lib.js:277:23:277:30 | opts.bla |
| lib/lib.js:279:19:279:22 | opts | lib/lib.js:279:19:279:26 | opts.bla |
| lib/lib.js:279:19:279:26 | opts.bla | lib/lib.js:281:23:281:35 | this.opts.bla |
| lib/lib.js:279:19:279:26 | opts.bla | lib/lib.js:281:23:281:35 | this.opts.bla |
| lib/lib.js:307:39:307:42 | name | lib/lib.js:308:23:308:26 | name |
| lib/lib.js:307:39:307:42 | name | lib/lib.js:308:23:308:26 | name |
| lib/lib.js:307:39:307:42 | name | lib/lib.js:308:23:308:26 | name |
@@ -714,6 +723,7 @@ edges
| lib/lib.js:261:11:261:33 | "rm -rf ... + name | lib/lib.js:257:35:257:38 | name | lib/lib.js:261:30:261:33 | name | This string concatenation which depends on $@ is later used in a $@. | lib/lib.js:257:35:257:38 | name | library input | lib/lib.js:261:3:261:34 | cp.exec ... + name) | shell command |
| lib/lib.js:268:10:268:32 | "rm -rf ... version | lib/lib.js:267:46:267:48 | obj | lib/lib.js:268:22:268:32 | obj.version | This string concatenation which depends on $@ is later used in a $@. | lib/lib.js:267:46:267:48 | obj | library input | lib/lib.js:268:2:268:33 | cp.exec ... ersion) | shell command |
| lib/lib.js:277:11:277:30 | "rm -rf " + opts.bla | lib/lib.js:276:8:276:11 | opts | lib/lib.js:277:23:277:30 | opts.bla | This string concatenation which depends on $@ is later used in a $@. | lib/lib.js:276:8:276:11 | opts | library input | lib/lib.js:277:3:277:31 | cp.exec ... ts.bla) | shell command |
| lib/lib.js:281:11:281:35 | "rm -rf ... pts.bla | lib/lib.js:276:8:276:11 | opts | lib/lib.js:281:23:281:35 | this.opts.bla | This string concatenation which depends on $@ is later used in a $@. | lib/lib.js:276:8:276:11 | opts | library input | lib/lib.js:281:3:281:36 | cp.exec ... ts.bla) | shell command |
| lib/lib.js:308:11:308:26 | "rm -rf " + name | lib/lib.js:307:39:307:42 | name | lib/lib.js:308:23:308:26 | name | This string concatenation which depends on $@ is later used in a $@. | lib/lib.js:307:39:307:42 | name | library input | lib/lib.js:308:3:308:27 | cp.exec ... + name) | shell command |
| lib/lib.js:315:10:315:25 | "rm -rf " + name | lib/lib.js:314:40:314:43 | name | lib/lib.js:315:22:315:25 | name | This string concatenation which depends on $@ is later used in a $@. | lib/lib.js:314:40:314:43 | name | library input | lib/lib.js:315:2:315:26 | cp.exec ... + name) | shell command |
| lib/lib.js:320:11:320:26 | "rm -rf " + name | lib/lib.js:314:40:314:43 | name | lib/lib.js:320:23:320:26 | name | This string concatenation which depends on $@ is later used in a $@. | lib/lib.js:314:40:314:43 | name | library input | lib/lib.js:320:3:320:27 | cp.exec ... + name) | shell command |

View File

@@ -278,7 +278,7 @@ module.exports.Foo = class Foo {
this.opts = {};
this.opts.bla = opts.bla
cp.exec("rm -rf " + this.opts.bla); // NOT OK - but FN [INCONSISTENCY]
cp.exec("rm -rf " + this.opts.bla); // NOT OK
}
}

View File

@@ -15,6 +15,41 @@ nodes
| lib/index.js:19:26:19:29 | data |
| lib/index.js:22:7:22:10 | data |
| lib/index.js:22:7:22:10 | data |
| lib/index.js:41:32:41:35 | opts |
| lib/index.js:41:32:41:35 | opts |
| lib/index.js:42:3:42:19 | opts |
| lib/index.js:42:10:42:13 | opts |
| lib/index.js:42:10:42:19 | opts \|\| {} |
| lib/index.js:44:21:44:24 | opts |
| lib/index.js:44:21:44:32 | opts.varName |
| lib/index.js:51:21:51:32 | opts.varName |
| lib/index.js:51:21:51:32 | opts.varName |
| lib/index.js:51:21:51:32 | opts.varName |
| lib/index.js:86:15:86:19 | taint |
| lib/index.js:86:15:86:19 | taint |
| lib/index.js:87:18:87:22 | taint |
| lib/index.js:89:36:89:40 | taint |
| lib/index.js:93:32:93:36 | taint |
| lib/index.js:98:30:98:34 | taint |
| lib/index.js:103:21:103:47 | this.op ... dOption |
| lib/index.js:103:21:103:47 | this.op ... dOption |
| lib/index.js:104:21:104:47 | this.op ... dOption |
| lib/index.js:104:21:104:47 | this.op ... dOption |
| lib/index.js:105:21:105:47 | this.op ... dOption |
| lib/index.js:105:21:105:47 | this.op ... dOption |
| lib/index.js:106:21:106:30 | this.taint |
| lib/index.js:106:21:106:30 | this.taint |
| lib/index.js:112:17:112:21 | taint |
| lib/index.js:112:17:112:21 | taint |
| lib/index.js:113:20:113:24 | taint |
| lib/index.js:121:34:121:38 | taint |
| lib/index.js:129:32:129:36 | taint |
| lib/index.js:136:23:136:49 | this.op ... dOption |
| lib/index.js:136:23:136:49 | this.op ... dOption |
| lib/index.js:137:23:137:49 | this.op ... dOption |
| lib/index.js:137:23:137:49 | this.op ... dOption |
| lib/index.js:138:23:138:32 | this.taint |
| lib/index.js:138:23:138:32 | this.taint |
edges
| lib/index.js:1:35:1:38 | data | lib/index.js:2:21:2:24 | data |
| lib/index.js:1:35:1:38 | data | lib/index.js:2:21:2:24 | data |
@@ -32,8 +67,53 @@ edges
| lib/index.js:19:26:19:29 | data | lib/index.js:22:7:22:10 | data |
| lib/index.js:19:26:19:29 | data | lib/index.js:22:7:22:10 | data |
| lib/index.js:19:26:19:29 | data | lib/index.js:22:7:22:10 | data |
| lib/index.js:41:32:41:35 | opts | lib/index.js:42:10:42:13 | opts |
| lib/index.js:41:32:41:35 | opts | lib/index.js:42:10:42:13 | opts |
| lib/index.js:42:3:42:19 | opts | lib/index.js:44:21:44:24 | opts |
| lib/index.js:42:10:42:13 | opts | lib/index.js:42:10:42:19 | opts \|\| {} |
| lib/index.js:42:10:42:19 | opts \|\| {} | lib/index.js:42:3:42:19 | opts |
| lib/index.js:44:21:44:24 | opts | lib/index.js:44:21:44:32 | opts.varName |
| lib/index.js:44:21:44:32 | opts.varName | lib/index.js:51:21:51:32 | opts.varName |
| lib/index.js:44:21:44:32 | opts.varName | lib/index.js:51:21:51:32 | opts.varName |
| lib/index.js:44:21:44:32 | opts.varName | lib/index.js:51:21:51:32 | opts.varName |
| lib/index.js:86:15:86:19 | taint | lib/index.js:87:18:87:22 | taint |
| lib/index.js:86:15:86:19 | taint | lib/index.js:87:18:87:22 | taint |
| lib/index.js:86:15:86:19 | taint | lib/index.js:89:36:89:40 | taint |
| lib/index.js:86:15:86:19 | taint | lib/index.js:89:36:89:40 | taint |
| lib/index.js:86:15:86:19 | taint | lib/index.js:93:32:93:36 | taint |
| lib/index.js:86:15:86:19 | taint | lib/index.js:93:32:93:36 | taint |
| lib/index.js:86:15:86:19 | taint | lib/index.js:98:30:98:34 | taint |
| lib/index.js:86:15:86:19 | taint | lib/index.js:98:30:98:34 | taint |
| lib/index.js:87:18:87:22 | taint | lib/index.js:106:21:106:30 | this.taint |
| lib/index.js:87:18:87:22 | taint | lib/index.js:106:21:106:30 | this.taint |
| lib/index.js:89:36:89:40 | taint | lib/index.js:103:21:103:47 | this.op ... dOption |
| lib/index.js:89:36:89:40 | taint | lib/index.js:103:21:103:47 | this.op ... dOption |
| lib/index.js:93:32:93:36 | taint | lib/index.js:104:21:104:47 | this.op ... dOption |
| lib/index.js:93:32:93:36 | taint | lib/index.js:104:21:104:47 | this.op ... dOption |
| lib/index.js:98:30:98:34 | taint | lib/index.js:105:21:105:47 | this.op ... dOption |
| lib/index.js:98:30:98:34 | taint | lib/index.js:105:21:105:47 | this.op ... dOption |
| lib/index.js:112:17:112:21 | taint | lib/index.js:113:20:113:24 | taint |
| lib/index.js:112:17:112:21 | taint | lib/index.js:113:20:113:24 | taint |
| lib/index.js:112:17:112:21 | taint | lib/index.js:121:34:121:38 | taint |
| lib/index.js:112:17:112:21 | taint | lib/index.js:121:34:121:38 | taint |
| lib/index.js:112:17:112:21 | taint | lib/index.js:129:32:129:36 | taint |
| lib/index.js:112:17:112:21 | taint | lib/index.js:129:32:129:36 | taint |
| lib/index.js:113:20:113:24 | taint | lib/index.js:138:23:138:32 | this.taint |
| lib/index.js:113:20:113:24 | taint | lib/index.js:138:23:138:32 | this.taint |
| lib/index.js:121:34:121:38 | taint | lib/index.js:136:23:136:49 | this.op ... dOption |
| lib/index.js:121:34:121:38 | taint | lib/index.js:136:23:136:49 | this.op ... dOption |
| lib/index.js:129:32:129:36 | taint | lib/index.js:137:23:137:49 | this.op ... dOption |
| lib/index.js:129:32:129:36 | taint | lib/index.js:137:23:137:49 | this.op ... dOption |
#select
| lib/index.js:2:21:2:24 | data | lib/index.js:1:35:1:38 | data | lib/index.js:2:21:2:24 | data | This string concatenation which depends on $@ is later $@. | lib/index.js:1:35:1:38 | data | library input | lib/index.js:2:15:2:30 | "(" + data + ")" | interpreted as code |
| lib/index.js:6:26:6:29 | name | lib/index.js:5:35:5:38 | name | lib/index.js:6:26:6:29 | name | This string concatenation which depends on $@ is later $@. | lib/index.js:5:35:5:38 | name | library input | lib/index.js:6:17:6:29 | "obj." + name | interpreted as code |
| lib/index.js:14:21:14:24 | data | lib/index.js:13:38:13:41 | data | lib/index.js:14:21:14:24 | data | This string concatenation which depends on $@ is later $@. | lib/index.js:13:38:13:41 | data | library input | lib/index.js:14:15:14:30 | "(" + data + ")" | interpreted as code |
| lib/index.js:22:7:22:10 | data | lib/index.js:19:26:19:29 | data | lib/index.js:22:7:22:10 | data | This string concatenation which depends on $@ is later $@. | lib/index.js:19:26:19:29 | data | library input | lib/index.js:25:24:25:26 | str | interpreted as code |
| lib/index.js:51:21:51:32 | opts.varName | lib/index.js:41:32:41:35 | opts | lib/index.js:51:21:51:32 | opts.varName | This string concatenation which depends on $@ is later $@. | lib/index.js:41:32:41:35 | opts | library input | lib/index.js:51:10:51:52 | " var ... ing();" | interpreted as code |
| lib/index.js:103:21:103:47 | this.op ... dOption | lib/index.js:86:15:86:19 | taint | lib/index.js:103:21:103:47 | this.op ... dOption | This string concatenation which depends on $@ is later $@. | lib/index.js:86:15:86:19 | taint | library input | lib/index.js:103:10:103:67 | " var ... ing();" | interpreted as code |
| lib/index.js:104:21:104:47 | this.op ... dOption | lib/index.js:86:15:86:19 | taint | lib/index.js:104:21:104:47 | this.op ... dOption | This string concatenation which depends on $@ is later $@. | lib/index.js:86:15:86:19 | taint | library input | lib/index.js:104:10:104:67 | " var ... ing();" | interpreted as code |
| lib/index.js:105:21:105:47 | this.op ... dOption | lib/index.js:86:15:86:19 | taint | lib/index.js:105:21:105:47 | this.op ... dOption | This string concatenation which depends on $@ is later $@. | lib/index.js:86:15:86:19 | taint | library input | lib/index.js:105:10:105:67 | " var ... ing();" | interpreted as code |
| lib/index.js:106:21:106:30 | this.taint | lib/index.js:86:15:86:19 | taint | lib/index.js:106:21:106:30 | this.taint | This string concatenation which depends on $@ is later $@. | lib/index.js:86:15:86:19 | taint | library input | lib/index.js:106:10:106:50 | " var ... ing();" | interpreted as code |
| lib/index.js:136:23:136:49 | this.op ... dOption | lib/index.js:112:17:112:21 | taint | lib/index.js:136:23:136:49 | this.op ... dOption | This string concatenation which depends on $@ is later $@. | lib/index.js:112:17:112:21 | taint | library input | lib/index.js:136:12:136:69 | " var ... ing();" | interpreted as code |
| lib/index.js:137:23:137:49 | this.op ... dOption | lib/index.js:112:17:112:21 | taint | lib/index.js:137:23:137:49 | this.op ... dOption | This string concatenation which depends on $@ is later $@. | lib/index.js:112:17:112:21 | taint | library input | lib/index.js:137:12:137:69 | " var ... ing();" | interpreted as code |
| lib/index.js:138:23:138:32 | this.taint | lib/index.js:112:17:112:21 | taint | lib/index.js:138:23:138:32 | this.taint | This string concatenation which depends on $@ is later $@. | lib/index.js:112:17:112:21 | taint | library input | lib/index.js:138:12:138:52 | " var ... ing();" | interpreted as code |

View File

@@ -33,3 +33,109 @@ export function greySink(data) {
}
});
}
function codeIsAlive() {
new Template().compile();
}
export function Template(text, opts) {
opts = opts || {};
var options = {};
options.varName = opts.varName;
this.opts = options;
}
Template.prototype = {
compile: function () {
var opts = this.opts;
eval(" var " + opts.varName + " = something();"); // NOT OK
},
// The below are justs tests that ensure the global-access-path computations terminate.
pathsTerminate1: function (node, prev) {
node.tree = {
ancestor: node,
number: rand ? prev.tree.number + 1 : 0,
};
},
pathsTerminate2: function (A) {
try {
var B = A.p1;
var C = B.p2;
C.p5 = C;
} catch (ex) {}
},
pathsTerminate3: function (A) {
var x = foo();
while (Math.random()) {
x.r = x;
}
},
pathsTerminate4: function () {
var dest = foo();
var range = foo();
while (Math.random() < 0.5) {
range.tabstop = dest;
if (Math.random() < 0.5) {
dest.firstNonLinked = range;
}
}
},
};
export class AccessPathClass {
constructor(taint) {
this.taint = taint;
var options1 = {taintedOption: taint};
this.options1 = options1;
var options2;
options2 = {taintedOption: taint};
this.options2 = options2;
var options3;
options3 = {};
options3.taintedOption = taint;
this.options3 = options3;
}
doesTaint() {
eval(" var " + this.options1.taintedOption + " = something();"); // NOT OK
eval(" var " + this.options2.taintedOption + " = something();"); // NOT OK
eval(" var " + this.options3.taintedOption + " = something();"); // NOT OK
eval(" var " + this.taint + " = something();"); // NOT OK
}
}
export class AccessPathClassBB {
constructor(taint) {
this.taint = taint;
var options1 = {taintedOption: taint};
if (Math.random() < 0.5) { console.log("foo"); }
this.options1 = options1;
var options2;
if (Math.random() < 0.5) { console.log("foo"); }
options2 = {taintedOption: taint};
if (Math.random() < 0.5) { console.log("foo"); }
this.options2 = options2;
var options3;
if (Math.random() < 0.5) { console.log("foo"); }
options3 = {};
if (Math.random() < 0.5) { console.log("foo"); }
options3.taintedOption = taint;
if (Math.random() < 0.5) { console.log("foo"); }
this.options3 = options3;
}
doesTaint() {
eval(" var " + this.options1.taintedOption + " = something();"); // NOT OK
eval(" var " + this.options2.taintedOption + " = something();"); // NOT OK
eval(" var " + this.options3.taintedOption + " = something();"); // NOT OK
eval(" var " + this.taint + " = something();"); // NOT OK
}
}

View File

@@ -248,7 +248,9 @@ edges
| lib.js:55:15:55:21 | path[0] | lib.js:55:11:55:22 | obj[path[0]] |
| lib.js:59:18:59:18 | s | lib.js:61:17:61:17 | s |
| lib.js:59:18:59:18 | s | lib.js:61:17:61:17 | s |
| lib.js:61:17:61:17 | s | lib.js:68:11:68:26 | path |
| lib.js:61:17:61:17 | s | lib.js:68:18:68:26 | this.path |
| lib.js:61:17:61:17 | s | lib.js:70:17:70:20 | path |
| lib.js:68:11:68:26 | path | lib.js:70:17:70:20 | path |
| lib.js:68:18:68:26 | this.path | lib.js:68:11:68:26 | path |
| lib.js:70:17:70:20 | path | lib.js:70:17:70:23 | path[0] |

View File

@@ -22,6 +22,10 @@ def codeql_workspace(repository_name = "codeql"):
_swift_prebuilt_version,
repo_arch,
),
patches = [
"@%s//swift/third_party/swift-llvm-support:patches/remove_getFallthrougDest_assert.patch" % repository_name,
],
patch_args = ["-p1"],
build_file = "@%s//swift/third_party/swift-llvm-support:BUILD.swift-prebuilt.bazel" % repository_name,
sha256 = sha256,
)

View File

@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* The `ActiveSupport` extensions `Object#try` and `Object#try!` are now recognised as code executions.

View File

@@ -252,6 +252,30 @@ private module Cached {
cfn.isJoin()
or
cfn.getAPredecessor().isBranch()
or
/*
* In cases such as
*
* ```rb
* if x or y
* foo
* else
* bar
* ```
*
* we have a CFG that looks like
*
* x --false--> [false] x or y --false--> bar
* \ |
* --true--> y --false--
* \
* --true--> [true] x or y --true--> foo
*
* and we want to ensure that both `foo` and `bar` start a new basic block,
* in order to get a `ConditionalBlock` out of the disjunction.
*/
exists(cfn.getAPredecessor(any(SuccessorTypes::ConditionalSuccessor s)))
}
/**

View File

@@ -147,6 +147,9 @@ class ExprNode extends Node, TExprNode {
class ParameterNode extends Node, TParameterNode instanceof ParameterNodeImpl {
/** Gets the parameter corresponding to this node, if any. */
final Parameter getParameter() { result = super.getParameter() }
/** Gets the name of the parameter, if any. */
final string getName() { result = this.getParameter().(NamedParameter).getName() }
}
/**

View File

@@ -81,6 +81,29 @@ module ActiveSupport {
preservesValue = true
}
}
/**
* A call to `Object#try`, which may execute its first argument as a Ruby
* method call.
* ```rb
* x = "abc"
* x.try(:upcase) # => "ABC"
* y = nil
* y.try(:upcase) # => nil
* ```
* `Object#try!` behaves similarly but raises `NoMethodError` if the
* receiver is not `nil` and does not respond to the method.
*/
class TryCallCodeExecution extends CodeExecution::Range instanceof DataFlow::CallNode {
TryCallCodeExecution() {
this.asExpr().getExpr() instanceof UnknownMethodCall and
this.getMethodName() = ["try", "try!"]
}
override DataFlow::Node getCode() { result = super.getArgument(0) }
override predicate runsArbitraryCode() { none() }
}
}
/**

View File

@@ -7,6 +7,7 @@ private import codeql.ruby.DataFlow
private import codeql.ruby.dataflow.FlowSummary
import core.BasicObject::BasicObject
import core.Object::Object
import core.Gem::Gem
import core.Kernel::Kernel
import core.Module
import core.Array

View File

@@ -0,0 +1,102 @@
/**
* Provides modeling for the `Gem` module and `.gemspec` files.
*/
private import ruby
private import Ast
private import codeql.ruby.ApiGraphs
/** Provides modeling for the `Gem` module and `.gemspec` files. */
module Gem {
/**
* A .gemspec file that lists properties of a Ruby gem.
* The contents of a .gemspec file generally look like:
* ```Ruby
* Gem::Specification.new do |s|
* s.name = 'library-name'
* s.require_path = "lib"
* # more properties
* end
* ```
*/
class GemSpec instanceof File {
API::Node specCall;
GemSpec() {
this.getExtension() = "gemspec" and
specCall = API::root().getMember("Gem").getMember("Specification").getMethod("new") and
specCall.getLocation().getFile() = this
}
/** Gets the name of this .gemspec file. */
string toString() { result = File.super.getBaseName() }
/**
* Gets a value of the `name` property of this .gemspec file.
* These properties are set using the `Gem::Specification.new` method.
*/
private Expr getSpecProperty(string name) {
exists(Expr rhs |
rhs =
specCall
.getBlock()
.getParameter(0)
.getMethod(name + "=")
.getParameter(0)
.asSink()
.asExpr()
.getExpr()
.(Ast::AssignExpr)
.getRightOperand()
|
result = rhs
or
// some properties are arrays, we just unfold them
result = rhs.(ArrayLiteral).getAnElement()
)
}
/** Gets the name of the gem */
string getName() { result = getSpecProperty("name").getConstantValue().getString() }
/** Gets a path that is loaded when the gem is required */
private string getARequirePath() {
result = getSpecProperty(["require_paths", "require_path"]).getConstantValue().getString()
or
not exists(getSpecProperty(["require_paths", "require_path"]).getConstantValue().getString()) and
result = "lib" // the default is "lib"
}
/** Gets a file that could be loaded when the gem is required. */
private File getAPossiblyRequiredFile() {
result = File.super.getParentContainer().getFolder(getARequirePath()).getAChildContainer*()
}
/** Gets a class/module that is exported by this gem. */
private ModuleBase getAPublicModule() {
result.(Toplevel).getLocation().getFile() = getAPossiblyRequiredFile()
or
result = getAPublicModule().getAModule()
or
result = getAPublicModule().getAClass()
or
result = getAPublicModule().getStmt(_).(SingletonClass)
}
/** Gets a parameter from an exported method, which is an input to this gem. */
DataFlow::ParameterNode getAnInputParameter() {
exists(MethodBase method | method = getAPublicModule().getAMethod() |
result.getParameter() = method.getAParameter() and
method.isPublic()
)
}
}
/** Gets a parameter that is an input to a named gem. */
DataFlow::ParameterNode getALibraryInput() {
exists(GemSpec spec |
exists(spec.getName()) and // we only consider `.gemspec` files that have a name
result = spec.getAnInputParameter()
)
}
}

View File

@@ -7,8 +7,7 @@ private import codeql.ruby.dataflow.internal.DataFlowImplForRegExp
private import codeql.ruby.typetracking.TypeTracker
private import codeql.ruby.ApiGraphs
private import codeql.ruby.dataflow.internal.DataFlowPrivate as DataFlowPrivate
private import codeql.ruby.dataflow.internal.FlowSummaryImpl as FlowSummaryImpl
private import codeql.ruby.dataflow.FlowSummary as FlowSummary
private import codeql.ruby.TaintTracking
private import codeql.ruby.frameworks.core.String
class RegExpConfiguration extends Configuration {
@@ -38,8 +37,8 @@ class RegExpConfiguration extends Configuration {
}
override predicate isAdditionalFlowStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
// include taint flow through `String` summaries,
FlowSummaryImpl::Private::Steps::summaryLocalStep(nodeFrom, nodeTo, false) and
// include taint flow through `String` summaries
TaintTracking::localTaintStep(nodeFrom, nodeTo) and
nodeFrom.(DataFlowPrivate::SummaryNode).getSummarizedCallable() instanceof
String::SummarizedCallable
or

View File

@@ -49,6 +49,9 @@ module CommandInjection {
class ShellwordsEscapeAsSanitizer extends Sanitizer {
ShellwordsEscapeAsSanitizer() {
this = API::getTopLevelMember("Shellwords").getAMethodCall(["escape", "shellescape"])
or
// The method is also added as `String#shellescape`.
this.(DataFlow::CallNode).getMethodName() = "shellescape"
}
}
}

View File

@@ -0,0 +1,105 @@
/**
* Provides default sources, sinks and sanitizers for reasoning about
* shell command constructed from library input vulnerabilities, as
* well as extension points for adding your own.
*/
private import codeql.ruby.DataFlow
private import codeql.ruby.DataFlow2
private import codeql.ruby.ApiGraphs
private import codeql.ruby.frameworks.core.Gem::Gem as Gem
private import codeql.ruby.AST as Ast
private import codeql.ruby.Concepts as Concepts
/**
* Module containing sources, sinks, and sanitizers for shell command constructed from library input.
*/
module UnsafeShellCommandConstruction {
/** A source for shell command constructed from library input vulnerabilities. */
abstract class Source extends DataFlow::Node { }
/** An input parameter to a gem seen as a source. */
private class LibraryInputAsSource extends Source instanceof DataFlow::ParameterNode {
LibraryInputAsSource() {
this = Gem::getALibraryInput() and
// we exclude arguments named `cmd` or similar, as they seem to execute commands on purpose
not exists(string name | name = super.getName() |
name = ["cmd", "command"]
or
name.regexpMatch(".*(Cmd|Command)$")
)
}
}
/** A sink for shell command constructed from library input vulnerabilities. */
abstract class Sink extends DataFlow::Node {
/** Gets a description of how the string in this sink was constructed. */
abstract string describe();
/** Gets the dataflow node where the string is constructed. */
DataFlow::Node getStringConstruction() { result = this }
/** Gets the dataflow node that executed the string as a shell command. */
abstract DataFlow::Node getCommandExecution();
}
/** Holds if the string constructed at `source` is executed at `shellExec` */
predicate isUsedAsShellCommand(DataFlow::Node source, Concepts::SystemCommandExecution shellExec) {
source = backtrackShellExec(TypeTracker::TypeBackTracker::end(), shellExec)
}
import codeql.ruby.typetracking.TypeTracker as TypeTracker
private DataFlow::LocalSourceNode backtrackShellExec(
TypeTracker::TypeBackTracker t, Concepts::SystemCommandExecution shellExec
) {
t.start() and
result = any(DataFlow::Node n | shellExec.isShellInterpreted(n)).getALocalSource()
or
exists(TypeTracker::TypeBackTracker t2 |
result = backtrackShellExec(t2, shellExec).backtrack(t2, t)
)
}
/**
* A string constructed from a string-literal (e.g. `"foo #{sink}"`),
* where the resulting string ends up being executed as a shell command.
*/
class StringInterpolationAsSink extends Sink {
Concepts::SystemCommandExecution s;
Ast::StringLiteral lit;
StringInterpolationAsSink() {
isUsedAsShellCommand(any(DataFlow::Node n | n.asExpr().getExpr() = lit), s) and
this.asExpr().getExpr() = lit.getComponent(_)
}
override string describe() { result = "string construction" }
override DataFlow::Node getCommandExecution() { result = s }
override DataFlow::Node getStringConstruction() { result.asExpr().getExpr() = lit }
}
import codeql.ruby.security.TaintedFormatStringSpecific as TaintedFormat
/**
* A string constructed from a printf-style call,
* where the resulting string ends up being executed as a shell command.
*/
class TaintedFormatStringAsSink extends Sink {
Concepts::SystemCommandExecution s;
TaintedFormat::PrintfStyleCall call;
TaintedFormatStringAsSink() {
isUsedAsShellCommand(call, s) and
this = [call.getFormatArgument(_), call.getFormatString()]
}
override string describe() { result = "formatted string" }
override DataFlow::Node getCommandExecution() { result = s }
override DataFlow::Node getStringConstruction() { result = call }
}
}

View File

@@ -0,0 +1,35 @@
/**
* Provides a taint tracking configuration for reasoning about shell command
* constructed from library input vulnerabilities
*
* Note, for performance reasons: only import this file if `Configuration` is needed,
* otherwise `UnsafeShellCommandConstructionCustomizations` should be imported instead.
*/
import codeql.ruby.DataFlow
import UnsafeShellCommandConstructionCustomizations::UnsafeShellCommandConstruction
private import codeql.ruby.TaintTracking
private import CommandInjectionCustomizations::CommandInjection as CommandInjection
private import codeql.ruby.dataflow.BarrierGuards
/**
* A taint-tracking configuration for detecting shell command constructed from library input vulnerabilities.
*/
class Configuration extends TaintTracking::Configuration {
Configuration() { this = "UnsafeShellCommandConstruction" }
override predicate isSource(DataFlow::Node source) { source instanceof Source }
override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
override predicate isSanitizer(DataFlow::Node node) {
node instanceof CommandInjection::Sanitizer or // using all sanitizers from `rb/command-injection`
node instanceof StringConstCompareBarrier or
node instanceof StringConstArrayInclusionCallBarrier
}
// override to require the path doesn't have unmatched return steps
override DataFlow::FlowFeature getAFeature() {
result instanceof DataFlow::FeatureHasSourceCallContext
}
}

View File

@@ -0,0 +1,4 @@
---
category: newQuery
---
* Added a new query, `rb/shell-command-constructed-from-input`, to detect libraries that unsafely construct shell commands from their inputs.

View File

@@ -0,0 +1,73 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>
Dynamically constructing a shell command with inputs from exported
functions may inadvertently change the meaning of the shell command.
Clients using the exported function may use inputs containing
characters that the shell interprets in a special way, for instance
quotes and spaces.
This can result in the shell command misbehaving, or even
allowing a malicious user to execute arbitrary commands on the system.
</p>
</overview>
<recommendation>
<p>
If possible, provide the dynamic arguments to the shell as an array
to APIs such as <code>system(..)</code> to avoid interpretation by the shell.
</p>
<p>
Alternatively, if the shell command must be constructed
dynamically, then add code to ensure that special characters
do not alter the shell command unexpectedly.
</p>
</recommendation>
<example>
<p>
The following example shows a dynamically constructed shell
command that downloads a file from a remote URL.
</p>
<sample src="examples/unsafe-shell-command-construction.rb" />
<p>
The shell command will, however, fail to work as intended if the
input contains spaces or other special characters interpreted in a
special way by the shell.
</p>
<p>
Even worse, a client might pass in user-controlled
data, not knowing that the input is interpreted as a shell command.
This could allow a malicious user to provide the input <code>http://example.org; cat /etc/passwd</code>
in order to execute the command <code>cat /etc/passwd</code>.
</p>
<p>
To avoid such potentially catastrophic behaviors, provide the
input from exported functions as an argument that does not
get interpreted by a shell:
</p>
<sample src="examples/unsafe-shell-command-construction_fixed.rb" />
</example>
<references>
<li>
OWASP:
<a href="https://www.owasp.org/index.php/Command_Injection">Command Injection</a>.
</li>
</references>
</qhelp>

View File

@@ -0,0 +1,26 @@
/**
* @name Unsafe shell command constructed from library input
* @description Using externally controlled strings in a command line may allow a malicious
* user to change the meaning of the command.
* @kind path-problem
* @problem.severity error
* @security-severity 6.3
* @precision high
* @id rb/shell-command-constructed-from-input
* @tags correctness
* security
* external/cwe/cwe-078
* external/cwe/cwe-088
* external/cwe/cwe-073
*/
import codeql.ruby.security.UnsafeShellCommandConstructionQuery
import DataFlow::PathGraph
from Configuration config, DataFlow::PathNode source, DataFlow::PathNode sink, Sink sinkNode
where
config.hasFlowPath(source, sink) and
sinkNode = sink.getNode()
select sinkNode.getStringConstruction(), source, sink,
"This " + sinkNode.describe() + " which depends on $@ is later used in a $@.", source.getNode(),
"library input", sinkNode.getCommandExecution(), "shell command"

View File

@@ -0,0 +1,5 @@
module Utils
def download(path)
system("wget #{path}") # NOT OK
end
end

View File

@@ -0,0 +1,6 @@
module Utils
def download(path)
# using an array to call `system` is safe
system("wget", path) # OK
end
end

View File

@@ -1,4 +1,4 @@
WARNING: Type BarrierGuard has been deprecated and may be removed in future (barrier-guards.ql:8,3-15)
WARNING: Type BarrierGuard has been deprecated and may be removed in future (barrier-guards.ql:9,3-15)
oldStyleBarrierGuards
| barrier-guards.rb:3:4:3:15 | ... == ... | barrier-guards.rb:4:5:4:7 | foo | barrier-guards.rb:3:4:3:6 | foo | true |
| barrier-guards.rb:9:4:9:24 | call to include? | barrier-guards.rb:10:5:10:7 | foo | barrier-guards.rb:9:21:9:23 | foo | true |
@@ -20,3 +20,50 @@ newStyleBarrierGuards
| barrier-guards.rb:71:5:71:7 | foo |
| barrier-guards.rb:83:5:83:7 | foo |
| barrier-guards.rb:91:5:91:7 | foo |
controls
| barrier-guards.rb:3:4:3:15 | ... == ... | barrier-guards.rb:4:5:4:7 | foo | true |
| barrier-guards.rb:3:4:3:15 | ... == ... | barrier-guards.rb:6:5:6:7 | foo | false |
| barrier-guards.rb:9:4:9:24 | call to include? | barrier-guards.rb:10:5:10:7 | foo | true |
| barrier-guards.rb:9:4:9:24 | call to include? | barrier-guards.rb:12:5:12:7 | foo | false |
| barrier-guards.rb:15:4:15:15 | ... != ... | barrier-guards.rb:16:5:16:7 | foo | true |
| barrier-guards.rb:15:4:15:15 | ... != ... | barrier-guards.rb:18:5:18:7 | foo | false |
| barrier-guards.rb:21:8:21:19 | ... == ... | barrier-guards.rb:22:5:22:7 | foo | false |
| barrier-guards.rb:21:8:21:19 | ... == ... | barrier-guards.rb:24:5:24:7 | foo | true |
| barrier-guards.rb:27:8:27:19 | ... != ... | barrier-guards.rb:28:5:28:7 | foo | false |
| barrier-guards.rb:27:8:27:19 | ... != ... | barrier-guards.rb:30:5:30:7 | foo | true |
| barrier-guards.rb:37:4:37:20 | call to include? | barrier-guards.rb:38:5:38:7 | foo | true |
| barrier-guards.rb:37:4:37:20 | call to include? | barrier-guards.rb:40:5:40:7 | foo | false |
| barrier-guards.rb:43:4:43:15 | ... == ... | barrier-guards.rb:44:5:46:5 | self | true |
| barrier-guards.rb:49:4:49:15 | ... == ... | barrier-guards.rb:50:5:53:5 | self | true |
| barrier-guards.rb:56:4:56:15 | ... == ... | barrier-guards.rb:57:5:57:13 | my_lambda | true |
| barrier-guards.rb:70:4:70:21 | call to include? | barrier-guards.rb:71:5:71:7 | foo | true |
| barrier-guards.rb:70:4:70:21 | call to include? | barrier-guards.rb:73:5:73:7 | foo | false |
| barrier-guards.rb:76:4:76:21 | call to include? | barrier-guards.rb:77:5:77:7 | foo | true |
| barrier-guards.rb:76:4:76:21 | call to include? | barrier-guards.rb:79:5:79:7 | foo | false |
| barrier-guards.rb:82:4:82:25 | ... != ... | barrier-guards.rb:83:5:83:7 | foo | true |
| barrier-guards.rb:82:4:82:25 | ... != ... | barrier-guards.rb:85:5:85:7 | foo | false |
| barrier-guards.rb:88:4:88:25 | ... == ... | barrier-guards.rb:89:5:89:7 | foo | true |
| barrier-guards.rb:88:4:88:25 | ... == ... | barrier-guards.rb:91:5:91:7 | foo | false |
| barrier-guards.rb:96:4:96:12 | call to condition | barrier-guards.rb:97:5:97:8 | bars | true |
| barrier-guards.rb:100:4:100:21 | call to include? | barrier-guards.rb:101:5:101:7 | foo | true |
| barrier-guards.rb:100:4:100:21 | call to include? | barrier-guards.rb:103:5:103:7 | foo | false |
| barrier-guards.rb:106:4:106:4 | call to x | barrier-guards.rb:106:4:106:9 | [false] ... or ... | false |
| barrier-guards.rb:106:4:106:4 | call to x | barrier-guards.rb:106:9:106:9 | self | false |
| barrier-guards.rb:106:4:106:4 | call to x | barrier-guards.rb:109:5:109:8 | bars | false |
| barrier-guards.rb:106:4:106:9 | [false] ... or ... | barrier-guards.rb:109:5:109:8 | bars | false |
| barrier-guards.rb:106:4:106:9 | [true] ... or ... | barrier-guards.rb:107:5:107:7 | foo | true |
| barrier-guards.rb:106:9:106:9 | call to y | barrier-guards.rb:106:4:106:9 | [false] ... or ... | false |
| barrier-guards.rb:106:9:106:9 | call to y | barrier-guards.rb:109:5:109:8 | bars | false |
| barrier-guards.rb:112:4:112:4 | call to x | barrier-guards.rb:112:4:112:10 | [true] ... and ... | true |
| barrier-guards.rb:112:4:112:4 | call to x | barrier-guards.rb:112:10:112:10 | self | true |
| barrier-guards.rb:112:4:112:4 | call to x | barrier-guards.rb:113:5:113:7 | foo | true |
| barrier-guards.rb:112:4:112:10 | [false] ... and ... | barrier-guards.rb:115:5:115:8 | bars | false |
| barrier-guards.rb:112:4:112:10 | [true] ... and ... | barrier-guards.rb:113:5:113:7 | foo | true |
| barrier-guards.rb:112:10:112:10 | call to y | barrier-guards.rb:112:4:112:10 | [true] ... and ... | true |
| barrier-guards.rb:112:10:112:10 | call to y | barrier-guards.rb:113:5:113:7 | foo | true |
| barrier-guards.rb:118:4:118:8 | [false] not ... | barrier-guards.rb:121:5:121:8 | bars | false |
| barrier-guards.rb:118:4:118:8 | [true] not ... | barrier-guards.rb:119:5:119:7 | foo | true |
| barrier-guards.rb:118:8:118:8 | call to x | barrier-guards.rb:118:4:118:8 | [false] not ... | true |
| barrier-guards.rb:118:8:118:8 | call to x | barrier-guards.rb:118:4:118:8 | [true] not ... | false |
| barrier-guards.rb:118:8:118:8 | call to x | barrier-guards.rb:119:5:119:7 | foo | false |
| barrier-guards.rb:118:8:118:8 | call to x | barrier-guards.rb:121:5:121:8 | bars | true |

View File

@@ -2,6 +2,7 @@ import codeql.ruby.dataflow.internal.DataFlowPublic
import codeql.ruby.dataflow.BarrierGuards
import codeql.ruby.controlflow.CfgNodes
import codeql.ruby.controlflow.ControlFlowGraph
import codeql.ruby.controlflow.BasicBlocks
import codeql.ruby.DataFlow
query predicate oldStyleBarrierGuards(
@@ -14,3 +15,10 @@ query predicate newStyleBarrierGuards(DataFlow::Node n) {
n instanceof StringConstCompareBarrier or
n instanceof StringConstArrayInclusionCallBarrier
}
query predicate controls(CfgNode condition, BasicBlock bb, SuccessorTypes::ConditionalSuccessor s) {
exists(ConditionBlock cb |
cb.controls(bb, s) and
condition = cb.getLastNode()
)
}

View File

@@ -102,3 +102,21 @@ if bars.include?(foo)
else
foo
end
if x or y then
foo
else
bars
end
if x and y then
foo
else
bars
end
if not x then
foo
else
bars
end

View File

@@ -5,3 +5,12 @@ constantizeCalls
loggerInstantiations
| active_support.rb:6:1:6:33 | call to new |
| active_support.rb:7:1:7:40 | call to new |
codeExecutions
| active_support.rb:1:1:1:22 | call to constantize |
| active_support.rb:3:1:3:13 | call to constantize |
| active_support.rb:4:1:4:18 | call to safe_constantize |
| active_support.rb:296:5:296:18 | call to try |
| active_support.rb:297:5:297:17 | call to try |
| active_support.rb:298:5:298:19 | call to try! |
| active_support.rb:298:5:298:35 | call to try! |
| active_support.rb:299:5:299:18 | call to try! |

View File

@@ -1,9 +1,12 @@
import codeql.ruby.frameworks.ActiveSupport
import codeql.ruby.DataFlow
import codeql.ruby.frameworks.stdlib.Logger
import codeql.ruby.Concepts
query DataFlow::Node constantizeCalls(ActiveSupport::CoreExtensions::String::Constantize c) {
result = c.getCode()
}
query predicate loggerInstantiations(Logger::LoggerInstantiation l) { any() }
query predicate codeExecutions(CodeExecution c) { any() }

View File

@@ -290,3 +290,11 @@ def m_deep_dup
x = source "a"
sink x.deep_dup # $hasValueFlow=a
end
def m_try(method)
x = "abc"
x.try(:upcase)
x.try(method)
x.try!(:upcase).try!(:downcase)
x.try!(method)
end

View File

@@ -15,6 +15,8 @@ edges
| CommandInjection.rb:81:20:81:25 | **args : | CommandInjection.rb:82:22:82:25 | args : |
| CommandInjection.rb:82:22:82:25 | args : | CommandInjection.rb:82:22:82:37 | ...[...] : |
| CommandInjection.rb:82:22:82:37 | ...[...] : | CommandInjection.rb:82:14:82:39 | "echo #{...}" |
| CommandInjection.rb:94:16:94:21 | call to params : | CommandInjection.rb:94:16:94:28 | ...[...] : |
| CommandInjection.rb:94:16:94:28 | ...[...] : | CommandInjection.rb:95:16:95:28 | "cat #{...}" |
nodes
| CommandInjection.rb:6:15:6:20 | call to params : | semmle.label | call to params : |
| CommandInjection.rb:6:15:6:26 | ...[...] : | semmle.label | ...[...] : |
@@ -37,6 +39,9 @@ nodes
| CommandInjection.rb:82:14:82:39 | "echo #{...}" | semmle.label | "echo #{...}" |
| CommandInjection.rb:82:22:82:25 | args : | semmle.label | args : |
| CommandInjection.rb:82:22:82:37 | ...[...] : | semmle.label | ...[...] : |
| CommandInjection.rb:94:16:94:21 | call to params : | semmle.label | call to params : |
| CommandInjection.rb:94:16:94:28 | ...[...] : | semmle.label | ...[...] : |
| CommandInjection.rb:95:16:95:28 | "cat #{...}" | semmle.label | "cat #{...}" |
subpaths
#select
| CommandInjection.rb:7:10:7:15 | #{...} | CommandInjection.rb:6:15:6:20 | call to params : | CommandInjection.rb:7:10:7:15 | #{...} | This command depends on a $@. | CommandInjection.rb:6:15:6:20 | call to params | user-provided value |
@@ -51,3 +56,4 @@ subpaths
| CommandInjection.rb:65:14:65:29 | "echo #{...}" | CommandInjection.rb:64:18:64:23 | number : | CommandInjection.rb:65:14:65:29 | "echo #{...}" | This command depends on a $@. | CommandInjection.rb:64:18:64:23 | number | user-provided value |
| CommandInjection.rb:73:14:73:34 | "echo #{...}" | CommandInjection.rb:72:23:72:33 | blah_number : | CommandInjection.rb:73:14:73:34 | "echo #{...}" | This command depends on a $@. | CommandInjection.rb:72:23:72:33 | blah_number | user-provided value |
| CommandInjection.rb:82:14:82:39 | "echo #{...}" | CommandInjection.rb:81:20:81:25 | **args : | CommandInjection.rb:82:14:82:39 | "echo #{...}" | This command depends on a $@. | CommandInjection.rb:81:20:81:25 | **args | user-provided value |
| CommandInjection.rb:95:16:95:28 | "cat #{...}" | CommandInjection.rb:94:16:94:21 | call to params : | CommandInjection.rb:95:16:95:28 | "cat #{...}" | This command depends on a $@. | CommandInjection.rb:94:16:94:21 | call to params | user-provided value |

View File

@@ -88,3 +88,13 @@ module Types
end
end
end
class Foo < ActionController::Base
def create
file = params[:file]
system("cat #{file}")
# .shellescape
system("cat #{file.shellescape}") # OK, because file is shell escaped
end
end

View File

@@ -0,0 +1,44 @@
edges
| impl/sub/notImported.rb:2:12:2:17 | target : | impl/sub/notImported.rb:3:19:3:27 | #{...} |
| impl/sub/other2.rb:2:12:2:17 | target : | impl/sub/other2.rb:3:19:3:27 | #{...} |
| impl/sub/other.rb:2:12:2:17 | target : | impl/sub/other.rb:3:19:3:27 | #{...} |
| impl/unsafeShell.rb:2:12:2:17 | target : | impl/unsafeShell.rb:3:19:3:27 | #{...} |
| impl/unsafeShell.rb:6:12:6:12 | x : | impl/unsafeShell.rb:7:32:7:32 | x |
| impl/unsafeShell.rb:15:47:15:64 | innocent_file_path : | impl/unsafeShell.rb:20:21:20:41 | #{...} |
| impl/unsafeShell.rb:23:15:23:23 | file_path : | impl/unsafeShell.rb:26:19:26:30 | #{...} |
| impl/unsafeShell.rb:33:12:33:17 | target : | impl/unsafeShell.rb:34:19:34:27 | #{...} |
| impl/unsafeShell.rb:37:10:37:10 | x : | impl/unsafeShell.rb:38:19:38:22 | #{...} |
| impl/unsafeShell.rb:47:16:47:21 | target : | impl/unsafeShell.rb:48:19:48:27 | #{...} |
nodes
| impl/sub/notImported.rb:2:12:2:17 | target : | semmle.label | target : |
| impl/sub/notImported.rb:3:19:3:27 | #{...} | semmle.label | #{...} |
| impl/sub/other2.rb:2:12:2:17 | target : | semmle.label | target : |
| impl/sub/other2.rb:3:19:3:27 | #{...} | semmle.label | #{...} |
| impl/sub/other.rb:2:12:2:17 | target : | semmle.label | target : |
| impl/sub/other.rb:3:19:3:27 | #{...} | semmle.label | #{...} |
| impl/unsafeShell.rb:2:12:2:17 | target : | semmle.label | target : |
| impl/unsafeShell.rb:3:19:3:27 | #{...} | semmle.label | #{...} |
| impl/unsafeShell.rb:6:12:6:12 | x : | semmle.label | x : |
| impl/unsafeShell.rb:7:32:7:32 | x | semmle.label | x |
| impl/unsafeShell.rb:15:47:15:64 | innocent_file_path : | semmle.label | innocent_file_path : |
| impl/unsafeShell.rb:20:21:20:41 | #{...} | semmle.label | #{...} |
| impl/unsafeShell.rb:23:15:23:23 | file_path : | semmle.label | file_path : |
| impl/unsafeShell.rb:26:19:26:30 | #{...} | semmle.label | #{...} |
| impl/unsafeShell.rb:33:12:33:17 | target : | semmle.label | target : |
| impl/unsafeShell.rb:34:19:34:27 | #{...} | semmle.label | #{...} |
| impl/unsafeShell.rb:37:10:37:10 | x : | semmle.label | x : |
| impl/unsafeShell.rb:38:19:38:22 | #{...} | semmle.label | #{...} |
| impl/unsafeShell.rb:47:16:47:21 | target : | semmle.label | target : |
| impl/unsafeShell.rb:48:19:48:27 | #{...} | semmle.label | #{...} |
subpaths
#select
| impl/sub/notImported.rb:3:14:3:28 | "cat #{...}" | impl/sub/notImported.rb:2:12:2:17 | target : | impl/sub/notImported.rb:3:19:3:27 | #{...} | This string construction which depends on $@ is later used in a $@. | impl/sub/notImported.rb:2:12:2:17 | target | library input | impl/sub/notImported.rb:3:5:3:34 | call to popen | shell command |
| impl/sub/other2.rb:3:14:3:28 | "cat #{...}" | impl/sub/other2.rb:2:12:2:17 | target : | impl/sub/other2.rb:3:19:3:27 | #{...} | This string construction which depends on $@ is later used in a $@. | impl/sub/other2.rb:2:12:2:17 | target | library input | impl/sub/other2.rb:3:5:3:34 | call to popen | shell command |
| impl/sub/other.rb:3:14:3:28 | "cat #{...}" | impl/sub/other.rb:2:12:2:17 | target : | impl/sub/other.rb:3:19:3:27 | #{...} | This string construction which depends on $@ is later used in a $@. | impl/sub/other.rb:2:12:2:17 | target | library input | impl/sub/other.rb:3:5:3:34 | call to popen | shell command |
| impl/unsafeShell.rb:3:14:3:28 | "cat #{...}" | impl/unsafeShell.rb:2:12:2:17 | target : | impl/unsafeShell.rb:3:19:3:27 | #{...} | This string construction which depends on $@ is later used in a $@. | impl/unsafeShell.rb:2:12:2:17 | target | library input | impl/unsafeShell.rb:3:5:3:34 | call to popen | shell command |
| impl/unsafeShell.rb:7:14:7:33 | call to sprintf | impl/unsafeShell.rb:6:12:6:12 | x : | impl/unsafeShell.rb:7:32:7:32 | x | This formatted string which depends on $@ is later used in a $@. | impl/unsafeShell.rb:6:12:6:12 | x | library input | impl/unsafeShell.rb:8:5:8:25 | call to popen | shell command |
| impl/unsafeShell.rb:20:14:20:42 | "which #{...}" | impl/unsafeShell.rb:15:47:15:64 | innocent_file_path : | impl/unsafeShell.rb:20:21:20:41 | #{...} | This string construction which depends on $@ is later used in a $@. | impl/unsafeShell.rb:15:47:15:64 | innocent_file_path | library input | impl/unsafeShell.rb:20:5:20:48 | call to popen | shell command |
| impl/unsafeShell.rb:26:14:26:31 | "cat #{...}" | impl/unsafeShell.rb:23:15:23:23 | file_path : | impl/unsafeShell.rb:26:19:26:30 | #{...} | This string construction which depends on $@ is later used in a $@. | impl/unsafeShell.rb:23:15:23:23 | file_path | library input | impl/unsafeShell.rb:26:5:26:37 | call to popen | shell command |
| impl/unsafeShell.rb:34:14:34:28 | "cat #{...}" | impl/unsafeShell.rb:33:12:33:17 | target : | impl/unsafeShell.rb:34:19:34:27 | #{...} | This string construction which depends on $@ is later used in a $@. | impl/unsafeShell.rb:33:12:33:17 | target | library input | impl/unsafeShell.rb:34:5:34:34 | call to popen | shell command |
| impl/unsafeShell.rb:38:14:38:23 | "cat #{...}" | impl/unsafeShell.rb:37:10:37:10 | x : | impl/unsafeShell.rb:38:19:38:22 | #{...} | This string construction which depends on $@ is later used in a $@. | impl/unsafeShell.rb:37:10:37:10 | x | library input | impl/unsafeShell.rb:38:5:38:29 | call to popen | shell command |
| impl/unsafeShell.rb:48:14:48:28 | "cat #{...}" | impl/unsafeShell.rb:47:16:47:21 | target : | impl/unsafeShell.rb:48:19:48:27 | #{...} | This string construction which depends on $@ is later used in a $@. | impl/unsafeShell.rb:47:16:47:21 | target | library input | impl/unsafeShell.rb:48:5:48:34 | call to popen | shell command |

View File

@@ -0,0 +1 @@
queries/security/cwe-078/UnsafeShellCommandConstruction.ql

View File

@@ -0,0 +1,6 @@
class Foobar
def foo1(target)
IO.popen("cat #{target}", "w") # NOT OK - everything assumed to be imported...
end
end

View File

@@ -0,0 +1,7 @@
class Foobar
def foo1(target)
IO.popen("cat #{target}", "w") # NOT OK
end
end
require 'sub/other2'

View File

@@ -0,0 +1,5 @@
class Foobar
def foo1(target)
IO.popen("cat #{target}", "w") # NOT OK
end
end

View File

@@ -0,0 +1,50 @@
class Foobar
def foo1(target)
IO.popen("cat #{target}", "w") # NOT OK
end
def foo2(x)
format = sprintf("cat %s", x) # NOT OK
IO.popen(format, "w")
end
def fileRead1(path)
File.read(path) # OK
end
def my_exec(cmd, command, myCmd, myCommand, innocent_file_path)
IO.popen("which #{cmd}", "w") # OK - the parameter is named `cmd`, so it's meant to be a command
IO.popen("which #{command}", "w") # OK - the parameter is named `command`, so it's meant to be a command
IO.popen("which #{myCmd}", "w") # OK - the parameter is named `myCmd`, so it's meant to be a command
IO.popen("which #{myCommand}", "w") # OK - the parameter is named `myCommand`, so it's meant to be a command
IO.popen("which #{innocent_file_path}", "w") # NOT OK - the parameter is named `innocent_file_path`, so it's not meant to be a command
end
def escaped(file_path)
IO.popen("cat #{file_path.shellescape}", "w") # OK - the parameter is escaped
IO.popen("cat #{file_path}", "w") # NOT OK - the parameter is not escaped
end
end
require File.join(File.dirname(__FILE__), 'sub', 'other')
class Foobar2
def foo1(target)
IO.popen("cat #{target}", "w") # NOT OK
end
def id(x)
IO.popen("cat #{x}", "w") # NOT OK - the parameter is not a constant.
return x
end
def thisIsSafe()
IO.popen("echo #{id('foo')}", "w") # OK - only using constants.
end
# class methods
def self.foo(target)
IO.popen("cat #{target}", "w") # NOT OK
end
end

View File

@@ -0,0 +1,5 @@
Gem::Specification.new do |s|
s.name = 'unsafe-shell'
s.require_path = "impl"
end

View File

@@ -12,15 +12,14 @@ Each class in the schema gets a corresponding `struct` in `TrapClasses.h`, where
"""
import functools
import pathlib
from typing import Dict
import typing
import inflection
from swift.codegen.lib import cpp, schema
def _get_type(t: str) -> str:
def _get_type(t: str, add_or_none_except: typing.Optional[str] = None) -> str:
if t is None:
# this is a predicate
return "bool"
@@ -29,11 +28,15 @@ def _get_type(t: str) -> str:
if t == "boolean":
return "bool"
if t[0].isupper():
return f"TrapLabel<{t}Tag>"
if add_or_none_except is not None and t != add_or_none_except:
suffix = "OrNone"
else:
suffix = ""
return f"TrapLabel<{t}{suffix}Tag>"
return t
def _get_field(cls: schema.Class, p: schema.Property) -> cpp.Field:
def _get_field(cls: schema.Class, p: schema.Property, add_or_none_except: typing.Optional[str] = None) -> cpp.Field:
trap_name = None
if not p.is_single:
trap_name = inflection.camelize(f"{cls.name}_{p.name}")
@@ -41,7 +44,7 @@ def _get_field(cls: schema.Class, p: schema.Property) -> cpp.Field:
trap_name = inflection.pluralize(trap_name)
args = dict(
field_name=p.name + ("_" if p.name in cpp.cpp_keywords else ""),
type=_get_type(p.type),
base_type=_get_type(p.type, add_or_none_except),
is_optional=p.is_optional,
is_repeated=p.is_repeated,
is_predicate=p.is_predicate,
@@ -52,8 +55,13 @@ def _get_field(cls: schema.Class, p: schema.Property) -> cpp.Field:
class Processor:
def __init__(self, data: Dict[str, schema.Class]):
self._classmap = data
def __init__(self, data: schema.Schema):
self._classmap = data.classes
if data.null:
root_type = next(iter(data.classes))
self._add_or_none_except = root_type
else:
self._add_or_none_except = None
@functools.lru_cache(maxsize=None)
def _get_class(self, name: str) -> cpp.Class:
@@ -64,7 +72,10 @@ class Processor:
return cpp.Class(
name=name,
bases=[self._get_class(b) for b in cls.bases],
fields=[_get_field(cls, p) for p in cls.properties if "cpp_skip" not in p.pragmas],
fields=[
_get_field(cls, p, self._add_or_none_except)
for p in cls.properties if "cpp_skip" not in p.pragmas
],
final=not cls.derived,
trap_name=trap_name,
)
@@ -78,8 +89,8 @@ class Processor:
def generate(opts, renderer):
assert opts.cpp_output
processor = Processor(schema.load_file(opts.schema).classes)
processor = Processor(schema.load_file(opts.schema))
out = opts.cpp_output
for dir, classes in processor.get_classes().items():
renderer.render(cpp.ClassList(classes, opts.schema,
include_parent=bool(dir)), out / dir / "TrapClasses")
include_parent=bool(dir)), out / dir / "TrapClasses")

View File

@@ -13,6 +13,7 @@ Moreover:
as columns
The type hierarchy will be translated to corresponding `union` declarations.
"""
import typing
import inflection
@@ -23,14 +24,21 @@ from typing import Set, List
log = logging.getLogger(__name__)
def dbtype(typename):
""" translate a type to a dbscheme counterpart, using `@lower_underscore` format for classes """
def dbtype(typename: str, add_or_none_except: typing.Optional[str] = None) -> str:
""" translate a type to a dbscheme counterpart, using `@lower_underscore` format for classes.
For class types, appends an underscore followed by `null` if provided
"""
if typename[0].isupper():
return "@" + inflection.underscore(typename)
underscored = inflection.underscore(typename)
if add_or_none_except is not None and typename != add_or_none_except:
suffix = "_or_none"
else:
suffix = ""
return f"@{underscored}{suffix}"
return typename
def cls_to_dbscheme(cls: schema.Class):
def cls_to_dbscheme(cls: schema.Class, add_or_none_except: typing.Optional[str] = None):
""" Yield all dbscheme entities needed to model class `cls` """
if cls.derived:
yield Union(dbtype(cls.name), (dbtype(c) for c in cls.derived))
@@ -48,7 +56,7 @@ def cls_to_dbscheme(cls: schema.Class):
columns=[
Column("id", type=dbtype(cls.name), binding=binding),
] + [
Column(f.name, dbtype(f.type)) for f in cls.properties if f.is_single
Column(f.name, dbtype(f.type, add_or_none_except)) for f in cls.properties if f.is_single
],
dir=dir,
)
@@ -61,7 +69,7 @@ def cls_to_dbscheme(cls: schema.Class):
columns=[
Column("id", type=dbtype(cls.name)),
Column("index", type="int"),
Column(inflection.singularize(f.name), dbtype(f.type)),
Column(inflection.singularize(f.name), dbtype(f.type, add_or_none_except)),
],
dir=dir,
)
@@ -71,7 +79,7 @@ def cls_to_dbscheme(cls: schema.Class):
name=inflection.tableize(f"{cls.name}_{f.name}"),
columns=[
Column("id", type=dbtype(cls.name)),
Column(f.name, dbtype(f.type)),
Column(f.name, dbtype(f.type, add_or_none_except)),
],
dir=dir,
)
@@ -87,7 +95,17 @@ def cls_to_dbscheme(cls: schema.Class):
def get_declarations(data: schema.Schema):
return [d for cls in data.classes.values() for d in cls_to_dbscheme(cls)]
add_or_none_except = data.root_class.name if data.null else None
declarations = [d for cls in data.classes.values() for d in cls_to_dbscheme(cls, add_or_none_except)]
if data.null:
property_classes = {
prop.type for cls in data.classes.values() for prop in cls.properties
if cls.name != data.null and prop.type and prop.type[0].isupper()
}
declarations += [
Union(dbtype(t, data.null), [dbtype(t), dbtype(data.null)]) for t in sorted(property_classes)
]
return declarations
def get_includes(data: schema.Schema, include_dir: pathlib.Path, swift_dir: pathlib.Path):

View File

@@ -147,7 +147,7 @@ def get_ql_property(cls: schema.Class, prop: schema.Property, prev_child: str =
return ql.Property(**args)
def get_ql_class(cls: schema.Class, lookup: typing.Dict[str, schema.Class]):
def get_ql_class(cls: schema.Class):
pragmas = {k: True for k in cls.pragmas if k.startswith("ql")}
prev_child = ""
properties = []
@@ -314,7 +314,7 @@ def generate(opts, renderer):
data = schema.load_file(input)
classes = {name: get_ql_class(cls, data.classes) for name, cls in data.classes.items()}
classes = {name: get_ql_class(cls) for name, cls in data.classes.items()}
if not classes:
raise NoClasses
root = next(iter(classes.values()))

View File

@@ -41,10 +41,10 @@ def get_cpp_type(schema_type: str):
def get_field(c: dbscheme.Column):
args = {
"field_name": c.schema_name,
"type": c.type,
"base_type": c.type,
}
args.update(cpp.get_field_override(c.schema_name))
args["type"] = get_cpp_type(args["type"])
args["base_type"] = get_cpp_type(args["base_type"])
return cpp.Field(**args)

View File

@@ -16,7 +16,7 @@ cpp_keywords = {"alignas", "alignof", "and", "and_eq", "asm", "atomic_cancel", "
"xor", "xor_eq"}
_field_overrides = [
(re.compile(r"(start|end)_(line|column)|(.*_)?index|width|num_.*"), {"type": "unsigned"}),
(re.compile(r"(start|end)_(line|column)|(.*_)?index|width|num_.*"), {"base_type": "unsigned"}),
(re.compile(r"(.*)_"), lambda m: {"field_name": m[1]}),
]
@@ -32,7 +32,7 @@ def get_field_override(field: str):
@dataclass
class Field:
field_name: str
type: str
base_type: str
is_optional: bool = False
is_repeated: bool = False
is_predicate: bool = False
@@ -40,13 +40,18 @@ class Field:
first: bool = False
def __post_init__(self):
if self.is_optional:
self.type = f"std::optional<{self.type}>"
if self.is_repeated:
self.type = f"std::vector<{self.type}>"
if self.field_name in cpp_keywords:
self.field_name += "_"
@property
def type(self) -> str:
type = self.base_type
if self.is_optional:
type = f"std::optional<{type}>"
if self.is_repeated:
type = f"std::vector<{type}>"
return type
# using @property breaks pystache internals here
def get_streamer(self):
if self.type == "std::string":
@@ -60,6 +65,10 @@ class Field:
def is_single(self):
return not (self.is_optional or self.is_repeated or self.is_predicate)
@property
def is_label(self):
return self.base_type.startswith("TrapLabel<")
@dataclass
class Trap:

View File

@@ -115,6 +115,8 @@ child = _ChildModifier()
doc = _DocModifier
desc = _DescModifier
use_for_null = _annotate(null=True)
qltest = _Namespace(
skip=_Pragma("qltest_skip"),
collapse_hierarchy=_Pragma("qltest_collapse_hierarchy"),

View File

@@ -55,6 +55,14 @@ class Property:
def is_predicate(self) -> bool:
return self.kind == self.Kind.PREDICATE
@property
def has_class_type(self) -> bool:
return bool(self.type) and self.type[0].isupper()
@property
def has_builtin_type(self) -> bool:
return bool(self.type) and self.type[0].islower()
SingleProperty = functools.partial(Property, Property.Kind.SINGLE)
OptionalProperty = functools.partial(Property, Property.Kind.OPTIONAL)
@@ -104,6 +112,16 @@ class Class:
class Schema:
classes: Dict[str, Class] = field(default_factory=dict)
includes: Set[str] = field(default_factory=set)
null: Optional[str] = None
@property
def root_class(self):
# always the first in the dictionary
return next(iter(self.classes.values()))
@property
def null_class(self):
return self.classes[self.null] if self.null else None
predicate_marker = object()
@@ -195,6 +213,8 @@ def _get_class(cls: type) -> Class:
raise Error(f"Class name must be capitalized, found {cls.__name__}")
if len({b._group for b in cls.__bases__ if hasattr(b, "_group")}) > 1:
raise Error(f"Bases with mixed groups for {cls.__name__}")
if any(getattr(b, "_null", False) for b in cls.__bases__):
raise Error(f"Null class cannot be derived")
return Class(name=cls.__name__,
bases=[b.__name__ for b in cls.__bases__ if b is not object],
derived={d.__name__ for d in cls.__subclasses__()},
@@ -233,6 +253,7 @@ def load(m: types.ModuleType) -> Schema:
known = {"int", "string", "boolean"}
known.update(n for n in m.__dict__ if not n.startswith("__"))
import swift.codegen.lib.schema.defs as defs
null = None
for name, data in m.__dict__.items():
if hasattr(defs, name):
continue
@@ -247,8 +268,13 @@ def load(m: types.ModuleType) -> Schema:
f"Only one root class allowed, found second root {name}")
cls.check_types(known)
classes[name] = cls
if getattr(data, "_null", False):
if null is not None:
raise Error(f"Null class {null} already defined, second null class {name} not allowed")
null = name
cls.is_null_class = True
return Schema(includes=includes, classes=_toposort_classes_by_group(classes))
return Schema(includes=includes, classes=_toposort_classes_by_group(classes), null=null)
def load_file(path: pathlib.Path) -> Schema:

View File

@@ -17,6 +17,8 @@ namespace codeql {
{{#classes}}
struct {{name}}{{#has_bases}} : {{#bases}}{{^first}}, {{/first}}{{ref.name}}{{/bases}}{{/has_bases}} {
static constexpr const char* NAME = "{{name}}";
{{#final}}
explicit {{name}}(TrapLabel<{{name}}Tag> id) : id{id} {}
@@ -33,6 +35,41 @@ struct {{name}}{{#has_bases}} : {{#bases}}{{^first}}, {{/first}}{{ref.name}}{{/b
}
{{/final}}
{{^final}}
protected:
{{/final}}
template <typename F>
void forEachLabel(F f) {
{{#final}}
f("id", -1, id);
{{/final}}
{{#bases}}
{{ref.name}}::forEachLabel(f);
{{/bases}}
{{#fields}}
{{#is_label}}
{{#is_repeated}}
for (auto i = 0u; i < {{field_name}}.size(); ++i) {
{{#is_optional}}
if ({{field_name}}[i]) f("{{field_name}}", i, *{{field_name}}[i]);
{{/is_optional}}
{{^is_optional}}
f("{{field_name}}", i, {{field_name}}[i]);
{{/is_optional}}
}
{{/is_repeated}}
{{^is_repeated}}
{{#is_optional}}
if ({{field_name}}) f("{{field_name}}", -1, *{{field_name}});
{{/is_optional}}
{{^is_optional}}
f("{{field_name}}", -1, {{field_name}});
{{/is_optional}}
{{/is_repeated}}
{{/is_label}}
{{/fields}}
}
protected:
void emit({{^final}}TrapLabel<{{name}}Tag> id, {{/final}}std::ostream& out) const;
};

View File

@@ -14,12 +14,24 @@ namespace codeql {
// {{table_name}}
struct {{name}}Trap {
{{#fields}}
static constexpr const char* NAME = "{{name}}Trap";
{{#fields}}
{{type}} {{field_name}}{};
{{/fields}}
{{/fields}}
template <typename F>
void forEachLabel(F f) {
{{#fields}}
{{#is_label}}
f("{{field_name}}", -1, {{field_name}});
{{/is_label}}
{{/fields}}
}
};
std::ostream &operator<<(std::ostream &out, const {{name}}Trap &e);
{{#id}}
namespace detail {

View File

@@ -80,6 +80,25 @@ def test_class_with_field(generate, type, expected, property_cls, optional, repe
]
def test_class_field_with_null(generate, input):
input.null = "Null"
a = cpp.Class(name="A")
assert generate([
schema.Class(name="A", derived={"B"}),
schema.Class(name="B", bases=["A"], properties=[
schema.SingleProperty("x", "A"),
schema.SingleProperty("y", "B"),
])
]) == [
a,
cpp.Class(name="B", bases=[a], final=True, trap_name="Bs",
fields=[
cpp.Field("x", "TrapLabel<ATag>"),
cpp.Field("y", "TrapLabel<BOrNoneTag>"),
]),
]
def test_class_with_predicate(generate):
assert generate([
schema.Class(name="MyClass", properties=[

View File

@@ -18,8 +18,9 @@ def dir_param(request):
@pytest.fixture
def generate(opts, input, renderer):
def func(classes):
def func(classes, null=None):
input.classes = {cls.name: cls for cls in classes}
input.null = null
(out, data), = run_generation(dbschemegen.generate, opts, renderer).items()
assert out is opts.dbscheme
return data
@@ -359,5 +360,114 @@ def test_class_with_derived_and_repeated_property(generate, dir_param):
)
def test_null_class(generate):
assert generate([
schema.Class(
name="Base",
derived={"W", "X", "Y", "Z", "Null"},
),
schema.Class(
name="W",
bases=["Base"],
properties=[
schema.SingleProperty("w", "W"),
schema.SingleProperty("x", "X"),
schema.OptionalProperty("y", "Y"),
schema.RepeatedProperty("z", "Z"),
]
),
schema.Class(
name="X",
bases=["Base"],
),
schema.Class(
name="Y",
bases=["Base"],
),
schema.Class(
name="Z",
bases=["Base"],
),
schema.Class(
name="Null",
bases=["Base"],
),
], null="Null") == dbscheme.Scheme(
src=schema_file,
includes=[],
declarations=[
dbscheme.Union(
lhs="@base",
rhs=["@null", "@w", "@x", "@y", "@z"],
),
dbscheme.Table(
name="ws",
columns=[
dbscheme.Column('id', '@w', binding=True),
dbscheme.Column('w', '@w_or_none'),
dbscheme.Column('x', '@x_or_none'),
],
),
dbscheme.Table(
name="w_ies",
keyset=dbscheme.KeySet(["id"]),
columns=[
dbscheme.Column('id', '@w'),
dbscheme.Column('y', '@y_or_none'),
],
),
dbscheme.Table(
name="w_zs",
keyset=dbscheme.KeySet(["id", "index"]),
columns=[
dbscheme.Column('id', '@w'),
dbscheme.Column('index', 'int'),
dbscheme.Column('z', '@z_or_none'),
],
),
dbscheme.Table(
name="xes",
columns=[
dbscheme.Column('id', '@x', binding=True),
],
),
dbscheme.Table(
name="ys",
columns=[
dbscheme.Column('id', '@y', binding=True),
],
),
dbscheme.Table(
name="zs",
columns=[
dbscheme.Column('id', '@z', binding=True),
],
),
dbscheme.Table(
name="nulls",
columns=[
dbscheme.Column('id', '@null', binding=True),
],
),
dbscheme.Union(
lhs="@w_or_none",
rhs=["@w", "@null"],
),
dbscheme.Union(
lhs="@x_or_none",
rhs=["@x", "@null"],
),
dbscheme.Union(
lhs="@y_or_none",
rhs=["@y", "@null"],
),
dbscheme.Union(
lhs="@z_or_none",
rhs=["@z", "@null"],
),
],
)
if __name__ == '__main__':
sys.exit(pytest.main([__file__] + sys.argv[1:]))

View File

@@ -13,6 +13,8 @@ def test_empty_schema():
assert data.classes == {}
assert data.includes == set()
assert data.null is None
assert data.null_class is None
def test_one_empty_class():
@@ -24,6 +26,7 @@ def test_one_empty_class():
assert data.classes == {
'MyClass': schema.Class('MyClass'),
}
assert data.root_class is data.classes['MyClass']
def test_two_empty_classes():
@@ -39,6 +42,7 @@ def test_two_empty_classes():
'MyClass1': schema.Class('MyClass1', derived={'MyClass2'}),
'MyClass2': schema.Class('MyClass2', bases=['MyClass1']),
}
assert data.root_class is data.classes['MyClass1']
def test_no_external_bases():
@@ -452,7 +456,8 @@ def test_property_docstring_newline():
property.""")
assert data.classes == {
'A': schema.Class('A', properties=[schema.SingleProperty('x', 'int', description=["very important", "property."])])
'A': schema.Class('A',
properties=[schema.SingleProperty('x', 'int', description=["very important", "property."])])
}
@@ -566,5 +571,54 @@ def test_class_default_doc_name():
}
def test_null_class():
@schema.load
class data:
class Root:
pass
@defs.use_for_null
class Null(Root):
pass
assert data.classes == {
'Root': schema.Class('Root', derived={'Null'}),
'Null': schema.Class('Null', bases=['Root']),
}
assert data.null == 'Null'
assert data.null_class is data.classes[data.null]
def test_null_class_cannot_be_derived():
with pytest.raises(schema.Error):
@schema.load
class data:
class Root:
pass
@defs.use_for_null
class Null(Root):
pass
class Impossible(Null):
pass
def test_null_class_cannot_be_defined_multiple_times():
with pytest.raises(schema.Error):
@schema.load
class data:
class Root:
pass
@defs.use_for_null
class Null1(Root):
pass
@defs.use_for_null
class Null2(Root):
pass
if __name__ == '__main__':
sys.exit(pytest.main([__file__] + sys.argv[1:]))

View File

@@ -60,20 +60,39 @@ class SwiftDispatcher {
}
template <typename Entry>
void emit(const Entry& entry) {
trap.emit(entry);
void emit(Entry&& entry) {
bool valid = true;
entry.forEachLabel([&valid, &entry, this](const char* field, int index, auto& label) {
using Label = std::remove_reference_t<decltype(label)>;
if (!label.valid()) {
std::cerr << entry.NAME << " has undefined " << field;
if (index >= 0) {
std::cerr << '[' << index << ']';
}
if constexpr (std::is_base_of_v<typename Label::Tag, UnspecifiedElementTag>) {
std::cerr << ", replacing with unspecified element\n";
label = emitUnspecified(idOf(entry), field, index);
} else {
std::cerr << ", skipping emission\n";
valid = false;
}
}
});
if (valid) {
trap.emit(entry);
}
}
template <typename Entry>
void emit(const std::optional<Entry>& entry) {
void emit(std::optional<Entry>&& entry) {
if (entry) {
emit(*entry);
emit(std::move(*entry));
}
}
template <typename... Cases>
void emit(const std::variant<Cases...>& entry) {
std::visit([this](const auto& e) { this->emit(e); }, entry);
void emit(std::variant<Cases...>&& entry) {
std::visit([this](auto&& e) { this->emit(std::move(e)); }, std::move(entry));
}
// This is a helper method to emit TRAP entries for AST nodes that we don't fully support yet.
@@ -88,13 +107,39 @@ class SwiftDispatcher {
emit(ElementIsUnknownTrap{label});
}
TrapLabel<UnspecifiedElementTag> emitUnspecified(std::optional<TrapLabel<ElementTag>>&& parent,
const char* property,
int index) {
UnspecifiedElement entry{trap.createLabel<UnspecifiedElementTag>()};
entry.error = "element was unspecified by the extractor";
entry.parent = std::move(parent);
entry.property = property;
if (index >= 0) {
entry.index = index;
}
trap.emit(entry);
return entry.id;
}
template <typename E>
std::optional<TrapLabel<ElementTag>> idOf(const E& entry) {
if constexpr (HasId<E>::value) {
return entry.id;
} else {
return std::nullopt;
}
}
// This method gives a TRAP label for already emitted AST node.
// If the AST node was not emitted yet, then the emission is dispatched to a corresponding
// visitor (see `visit(T *)` methods below).
template <typename E, typename... Args, std::enable_if_t<IsStorable<E>>* = nullptr>
TrapLabelOf<E> fetchLabel(const E& e, Args&&... args) {
if constexpr (std::is_constructible_v<bool, const E&>) {
assert(e && "fetching a label on a null entity, maybe fetchOptionalLabel is to be used?");
if (!e) {
// this will be treated on emission
return undefined_label;
}
}
// this is required so we avoid any recursive loop: a `fetchLabel` during the visit of `e` might
// end up calling `fetchLabel` on `e` itself, so we want the visit of `e` to call `fetchLabel`
@@ -205,17 +250,18 @@ class SwiftDispatcher {
return std::nullopt;
}
// map `fetchLabel` on the iterable `arg`, returning a vector of all labels
// map `fetchLabel` on the iterable `arg`
// universal reference `Arg&&` is used to catch both temporary and non-const references, not
// for perfect forwarding
template <typename Iterable>
auto fetchRepeatedLabels(Iterable&& arg) {
std::vector<decltype(fetchLabel(*arg.begin()))> ret;
using Label = decltype(fetchLabel(*arg.begin()));
TrapLabelVectorWrapper<typename Label::Tag> ret;
if constexpr (HasSize<Iterable>::value) {
ret.reserve(arg.size());
ret.data.reserve(arg.size());
}
for (auto&& e : arg) {
ret.push_back(fetchLabel(e));
ret.data.push_back(fetchLabel(e));
}
return ret;
}
@@ -270,6 +316,12 @@ class SwiftDispatcher {
template <typename T>
struct HasSize<T, decltype(std::declval<T>().size(), void())> : std::true_type {};
template <typename T, typename = void>
struct HasId : std::false_type {};
template <typename T>
struct HasId<T, decltype(std::declval<T>().id, void())> : std::true_type {};
void attachLocation(swift::SourceLoc start,
swift::SourceLoc end,
TrapLabel<LocatableTag> locatableLabel) {
@@ -293,19 +345,20 @@ class SwiftDispatcher {
TrapLabel<Tag> fetchLabelFromUnion(const llvm::PointerUnion<Ts...> u) {
TrapLabel<Tag> ret{};
// with logical op short-circuiting, this will stop trying on the first successful fetch
// don't feel tempted to replace the variable with the expression inside the `assert`, or
// building with `NDEBUG` will not trigger the fetching
bool unionCaseFound = (... || fetchLabelFromUnionCase<Tag, Ts>(u, ret));
assert(unionCaseFound && "llvm::PointerUnion not set to a known case");
if (!unionCaseFound) {
// TODO emit error/warning here
return undefined_label;
}
return ret;
}
template <typename Tag, typename T, typename... Ts>
bool fetchLabelFromUnionCase(const llvm::PointerUnion<Ts...> u, TrapLabel<Tag>& output) {
// we rely on the fact that when we extract `ASTNode` instances (which only happens
// on `BraceStmt` elements), we cannot encounter a standalone `TypeRepr` there, so we skip
// this case; extracting `TypeRepr`s here would be problematic as we would not be able to
// provide the corresponding type
// on `BraceStmt`/`IfConfigDecl` elements), we cannot encounter a standalone `TypeRepr` there,
// so we skip this case; extracting `TypeRepr`s here would be problematic as we would not be
// able to provide the corresponding type
if constexpr (!std::is_same_v<T, swift::TypeRepr*>) {
if (auto e = u.template dyn_cast<T>()) {
output = fetchLabel(e);

View File

@@ -4,9 +4,14 @@
#include <iomanip>
#include <iostream>
#include <string>
#include <vector>
namespace codeql {
struct UndefinedTrapLabel {};
constexpr UndefinedTrapLabel undefined_label{};
class UntypedTrapLabel {
uint64_t id_;
@@ -18,14 +23,17 @@ class UntypedTrapLabel {
protected:
UntypedTrapLabel() : id_{undefined} {}
UntypedTrapLabel(uint64_t id) : id_{id} {}
UntypedTrapLabel(uint64_t id) : id_{id} { assert(id != undefined); }
public:
bool valid() const { return id_ != undefined; }
explicit operator bool() const { return valid(); }
friend std::ostream& operator<<(std::ostream& out, UntypedTrapLabel l) {
// TODO: this is a temporary fix to catch us from outputting undefined labels to trap
// this should be moved to a validity check, probably aided by code generation and carried out
// by `SwiftDispatcher`
assert(l.id_ != undefined && "outputting an undefined label!");
assert(l && "outputting an undefined label!");
out << '#' << std::hex << l.id_ << std::dec;
return out;
}
@@ -44,14 +52,37 @@ class TrapLabel : public UntypedTrapLabel {
using Tag = TagParam;
TrapLabel() = default;
TrapLabel(UndefinedTrapLabel) : TrapLabel() {}
TrapLabel& operator=(UndefinedTrapLabel) {
*this = TrapLabel{};
return *this;
}
// The caller is responsible for ensuring ID uniqueness.
static TrapLabel unsafeCreateFromExplicitId(uint64_t id) { return {id}; }
static TrapLabel unsafeCreateFromUntyped(UntypedTrapLabel label) { return {label.id_}; }
template <typename OtherTag>
TrapLabel(const TrapLabel<OtherTag>& other) : UntypedTrapLabel(other) {
static_assert(std::is_base_of_v<Tag, OtherTag>, "wrong label assignment!");
template <typename SourceTag>
TrapLabel(const TrapLabel<SourceTag>& other) : UntypedTrapLabel(other) {
static_assert(std::is_base_of_v<Tag, SourceTag>, "wrong label assignment!");
}
};
// wrapper class to allow directly assigning a vector of TrapLabel<A> to a vector of
// TrapLabel<B> if B is a base of A, using move semantics rather than copying
template <typename TagParam>
struct TrapLabelVectorWrapper {
using Tag = TagParam;
std::vector<TrapLabel<TagParam>> data;
template <typename DestinationTag>
operator std::vector<TrapLabel<DestinationTag>>() && {
static_assert(std::is_base_of_v<DestinationTag, Tag>, "wrong label assignment!");
// reinterpret_cast is safe because TrapLabel instances differ only on the type, not the
// underlying data
return std::move(reinterpret_cast<std::vector<TrapLabel<DestinationTag>>&>(data));
}
};

View File

@@ -11,6 +11,7 @@ import codeql.swift.elements.Location
import codeql.swift.elements.UnknownFile
import codeql.swift.elements.UnknownLocation
import codeql.swift.elements.UnresolvedElement
import codeql.swift.elements.UnspecifiedElement
import codeql.swift.elements.decl.AbstractFunctionDecl
import codeql.swift.elements.decl.AbstractStorageDecl
import codeql.swift.elements.decl.AbstractTypeParamDecl

View File

@@ -0,0 +1,22 @@
private import codeql.swift.generated.UnspecifiedElement
import codeql.swift.elements.Location
class UnspecifiedElement extends Generated::UnspecifiedElement {
override string toString() {
exists(string source, string index |
(
source = " from " + this.getParent().getPrimaryQlClasses()
or
not this.hasParent() and source = ""
) and
(
index = "[" + this.getIndex() + "]"
or
not this.hasIndex() and index = ""
) and
result = "missing " + this.getProperty() + index + source
)
}
override Location getImmediateLocation() { result = this.getParent().(Locatable).getLocation() }
}

View File

@@ -0,0 +1,4 @@
// generated by codegen/codegen.py, remove this comment if you wish to edit this file
private import codeql.swift.generated.Raw
predicate constructUnspecifiedElement(Raw::UnspecifiedElement id) { any() }

View File

@@ -0,0 +1,108 @@
private import codeql.swift.elements.expr.BinaryExpr
private import codeql.swift.elements.expr.Expr
private import codeql.swift.elements.expr.PrefixUnaryExpr
/**
* A bitwise operation, such as:
* ```
* a & b
* ```
*/
class BitwiseOperation extends Expr {
BitwiseOperation() {
this instanceof BinaryBitwiseOperation or
this instanceof UnaryBitwiseOperation
}
/**
* Gets an operand of this bitwise operation.
*/
Expr getAnOperand() {
result =
[this.(BinaryBitwiseOperation).getAnOperand(), this.(UnaryBitwiseOperation).getOperand()]
}
}
/**
* A binary bitwise operation, such as:
* ```
* a & b
* ```
*/
class BinaryBitwiseOperation extends BinaryExpr {
BinaryBitwiseOperation() {
this instanceof AndBitwiseExpr or
this instanceof OrBitwiseExpr or
this instanceof XorBitwiseExpr or
this instanceof ShiftLeftBitwiseExpr or
this instanceof ShiftRightBitwiseExpr
}
}
/**
* A bitwise AND expression.
* ```
* a & b
* ```
*/
class AndBitwiseExpr extends BinaryExpr {
AndBitwiseExpr() { this.getStaticTarget().getName() = "&(_:_:)" }
}
/**
* A bitwise OR expression.
* ```
* a | b
* ```
*/
class OrBitwiseExpr extends BinaryExpr {
OrBitwiseExpr() { this.getStaticTarget().getName() = "|(_:_:)" }
}
/**
* A bitwise XOR expression.
* ```
* a ^ b
* ```
*/
class XorBitwiseExpr extends BinaryExpr {
XorBitwiseExpr() { this.getStaticTarget().getName() = "^(_:_:)" }
}
/**
* A bitwise shift left expression.
* ```
* a << b
* ```
*/
class ShiftLeftBitwiseExpr extends BinaryExpr {
ShiftLeftBitwiseExpr() { this.getStaticTarget().getName() = "<<(_:_:)" }
}
/**
* A bitwise shift right expression.
* ```
* a >> b
* ```
*/
class ShiftRightBitwiseExpr extends BinaryExpr {
ShiftRightBitwiseExpr() { this.getStaticTarget().getName() = ">>(_:_:)" }
}
/**
* A unary bitwise operation, such as:
* ```
* ~a
* ```
*/
class UnaryBitwiseOperation extends PrefixUnaryExpr instanceof NotBitwiseExpr { }
/**
* A bitwise NOT expression.
* ```
* ~a
* ```
*/
class NotBitwiseExpr extends PrefixUnaryExpr {
NotBitwiseExpr() { this.getStaticTarget().getName() = "~(_:)" }
}

View File

@@ -165,6 +165,21 @@ private module Impl {
)
}
private Element getImmediateChildOfUnspecifiedElement(
UnspecifiedElement e, int index, string partialPredicateCall
) {
exists(int b, int bLocatable, int n |
b = 0 and
bLocatable = b + 1 + max(int i | i = -1 or exists(getImmediateChildOfLocatable(e, i, _)) | i) and
n = bLocatable and
(
none()
or
result = getImmediateChildOfLocatable(e, index - b, partialPredicateCall)
)
)
}
private Element getImmediateChildOfDecl(Decl e, int index, string partialPredicateCall) {
exists(int b, int bAstNode, int n |
b = 0 and
@@ -4871,6 +4886,8 @@ private module Impl {
or
result = getImmediateChildOfUnknownLocation(e, index, partialAccessor)
or
result = getImmediateChildOfUnspecifiedElement(e, index, partialAccessor)
or
result = getImmediateChildOfEnumCaseDecl(e, index, partialAccessor)
or
result = getImmediateChildOfExtensionDecl(e, index, partialAccessor)

View File

@@ -51,6 +51,18 @@ module Raw {
override string toString() { result = "DbLocation" }
}
class UnspecifiedElement extends @unspecified_element, Locatable {
override string toString() { result = "UnspecifiedElement" }
Element getParent() { unspecified_element_parents(this, result) }
string getProperty() { unspecified_elements(this, result, _) }
int getIndex() { unspecified_element_indices(this, result) }
string getError() { unspecified_elements(this, _, result) }
}
class Decl extends @decl, AstNode {
ModuleDecl getModule() { decls(this, result) }
}

View File

@@ -10,6 +10,7 @@ module Synth {
TDbLocation(Raw::DbLocation id) { constructDbLocation(id) } or
TUnknownFile() or
TUnknownLocation() or
TUnspecifiedElement(Raw::UnspecifiedElement id) { constructUnspecifiedElement(id) } or
TAccessorDecl(Raw::AccessorDecl id) { constructAccessorDecl(id) } or
TAssociatedTypeDecl(Raw::AssociatedTypeDecl id) { constructAssociatedTypeDecl(id) } or
TClassDecl(Raw::ClassDecl id) { constructClassDecl(id) } or
@@ -321,7 +322,7 @@ module Synth {
class TFile = TDbFile or TUnknownFile;
class TLocatable = TArgument or TAstNode or TComment;
class TLocatable = TArgument or TAstNode or TComment or TUnspecifiedElement;
class TLocation = TDbLocation or TUnknownLocation;
@@ -499,6 +500,11 @@ module Synth {
cached
TUnknownLocation convertUnknownLocationFromRaw(Raw::Element e) { none() }
cached
TUnspecifiedElement convertUnspecifiedElementFromRaw(Raw::Element e) {
result = TUnspecifiedElement(e)
}
cached
TAccessorDecl convertAccessorDeclFromRaw(Raw::Element e) { result = TAccessorDecl(e) }
@@ -1464,6 +1470,8 @@ module Synth {
result = convertAstNodeFromRaw(e)
or
result = convertCommentFromRaw(e)
or
result = convertUnspecifiedElementFromRaw(e)
}
cached
@@ -2199,6 +2207,11 @@ module Synth {
cached
Raw::Element convertUnknownLocationToRaw(TUnknownLocation e) { none() }
cached
Raw::Element convertUnspecifiedElementToRaw(TUnspecifiedElement e) {
e = TUnspecifiedElement(result)
}
cached
Raw::Element convertAccessorDeclToRaw(TAccessorDecl e) { e = TAccessorDecl(result) }
@@ -3162,6 +3175,8 @@ module Synth {
result = convertAstNodeToRaw(e)
or
result = convertCommentToRaw(e)
or
result = convertUnspecifiedElementToRaw(e)
}
cached

View File

@@ -2,6 +2,7 @@
import codeql.swift.elements.CommentConstructor
import codeql.swift.elements.DbFileConstructor
import codeql.swift.elements.DbLocationConstructor
import codeql.swift.elements.UnspecifiedElementConstructor
import codeql.swift.elements.decl.AccessorDeclConstructor
import codeql.swift.elements.decl.AssociatedTypeDeclConstructor
import codeql.swift.elements.decl.ClassDeclConstructor

View File

@@ -0,0 +1,60 @@
// generated by codegen/codegen.py
private import codeql.swift.generated.Synth
private import codeql.swift.generated.Raw
import codeql.swift.elements.Element
import codeql.swift.elements.Locatable
module Generated {
class UnspecifiedElement extends Synth::TUnspecifiedElement, Locatable {
override string getAPrimaryQlClass() { result = "UnspecifiedElement" }
/**
* Gets the parent of this unspecified element, if it exists.
*
* This includes nodes from the "hidden" AST. It can be overridden in subclasses to change the
* behavior of both the `Immediate` and non-`Immediate` versions.
*/
Element getImmediateParent() {
result =
Synth::convertElementFromRaw(Synth::convertUnspecifiedElementToRaw(this)
.(Raw::UnspecifiedElement)
.getParent())
}
/**
* Gets the parent of this unspecified element, if it exists.
*/
final Element getParent() { result = getImmediateParent().resolve() }
/**
* Holds if `getParent()` exists.
*/
final predicate hasParent() { exists(getParent()) }
/**
* Gets the property of this unspecified element.
*/
string getProperty() {
result = Synth::convertUnspecifiedElementToRaw(this).(Raw::UnspecifiedElement).getProperty()
}
/**
* Gets the index of this unspecified element, if it exists.
*/
int getIndex() {
result = Synth::convertUnspecifiedElementToRaw(this).(Raw::UnspecifiedElement).getIndex()
}
/**
* Holds if `getIndex()` exists.
*/
final predicate hasIndex() { exists(getIndex()) }
/**
* Gets the error of this unspecified element.
*/
string getError() {
result = Synth::convertUnspecifiedElementToRaw(this).(Raw::UnspecifiedElement).getError()
}
}
}

File diff suppressed because it is too large Load Diff

View File

@@ -2,6 +2,7 @@
import codeql.swift.elements
import codeql.swift.elements.expr.ArithmeticOperation
import codeql.swift.elements.expr.BitwiseOperation
import codeql.swift.elements.expr.LogicalOperation
import codeql.swift.elements.decl.MethodDecl
import codeql.swift.elements.decl.ClassOrStructDecl

View File

@@ -0,0 +1,21 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>Constant salts should not be used for password hashing. Data hashed using constant salts are vulnerable to dictionary attacks, enabling attackers to recover the original input.</p>
</overview>
<recommendation>
<p>Use randomly generated salts to securely hash input data.</p>
</recommendation>
<example>
<p>The following example shows a few cases of hashing input data. In the 'BAD' cases, the salt is constant, making the generated hashes vulnerable to dictionary attacks. In the 'GOOD' cases, the salt is randomly generated, which protects the hashed data against recovery.</p>
<sample src="ConstantSalt.swift" />
</example>
<references>
<li><a href="https://www.okta.com/blog/2019/03/what-are-salted-passwords-and-password-hashing/">What are Salted Passwords and Password Hashing?</a></li>
</references>
</qhelp>

View File

@@ -0,0 +1,63 @@
/**
* @name Use of constant salts
* @description Using constant salts for password hashing is not secure because potential attackers can precompute the hash value via dictionary attacks.
* @kind path-problem
* @problem.severity error
* @security-severity 7.5
* @precision high
* @id swift/constant-salt
* @tags security
* external/cwe/cwe-760
*/
import swift
import codeql.swift.dataflow.DataFlow
import codeql.swift.dataflow.TaintTracking
import codeql.swift.dataflow.FlowSteps
import DataFlow::PathGraph
/**
* A constant salt is created through either a byte array or string literals.
*/
class ConstantSaltSource extends Expr {
ConstantSaltSource() {
this = any(ArrayExpr arr | arr.getType().getName() = "Array<UInt8>") or
this instanceof StringLiteralExpr
}
}
/**
* A class for all ways to use a constant salt.
*/
class ConstantSaltSink extends Expr {
ConstantSaltSink() {
// `salt` arg in `init` is a sink
exists(ClassOrStructDecl c, AbstractFunctionDecl f, CallExpr call, int arg |
c.getFullName() = ["HKDF", "PBKDF1", "PBKDF2", "Scrypt"] and
c.getAMember() = f and
f.getName().matches("%init(%salt:%") and
call.getStaticTarget() = f and
f.getParam(pragma[only_bind_into](arg)).getName() = "salt" and
call.getArgument(pragma[only_bind_into](arg)).getExpr() = this
)
}
}
/**
* A taint configuration from the source of constants salts to expressions that use
* them to initialize password-based enecryption keys.
*/
class ConstantSaltConfig extends TaintTracking::Configuration {
ConstantSaltConfig() { this = "ConstantSaltConfig" }
override predicate isSource(DataFlow::Node node) { node.asExpr() instanceof ConstantSaltSource }
override predicate isSink(DataFlow::Node node) { node.asExpr() instanceof ConstantSaltSink }
}
// The query itself
from ConstantSaltConfig config, DataFlow::PathNode sourceNode, DataFlow::PathNode sinkNode
where config.hasFlowPath(sourceNode, sinkNode)
select sinkNode.getNode(), sourceNode, sinkNode,
"The value '" + sourceNode.getNode().toString() +
"' is used as a constant salt, which is insecure for hashing passwords."

View File

@@ -0,0 +1,22 @@
func encrypt(padding : Padding) {
// ...
// BAD: Using constant salts for hashing
let salt: Array<UInt8> = [0x2a, 0x3a, 0x80, 0x05]
let randomArray = (0..<10).map({ _ in UInt8.random(in: 0...UInt8.max) })
_ = try HKDF(password: randomArray, salt: salt, info: randomArray, keyLength: 0, variant: Variant.sha2)
_ = try PKCS5.PBKDF1(password: randomArray, salt: salt, iterations: 120120, keyLength: 0)
_ = try PKCS5.PBKDF2(password: randomArray, salt: salt, iterations: 120120, keyLength: 0)
_ = try Scrypt(password: randomArray, salt: salt, dkLen: 64, N: 16384, r: 8, p: 1)
// GOOD: Using randomly generated salts for hashing
let salt = (0..<10).map({ _ in UInt8.random(in: 0...UInt8.max) })
let randomArray = (0..<10).map({ _ in UInt8.random(in: 0...UInt8.max) })
_ = try HKDF(password: randomArray, salt: salt, info: randomArray, keyLength: 0, variant: Variant.sha2)
_ = try PKCS5.PBKDF1(password: randomArray, salt: salt, iterations: 120120, keyLength: 0)
_ = try PKCS5.PBKDF2(password: randomArray, salt: salt, iterations: 120120, keyLength: 0)
_ = try Scrypt(password: randomArray, salt: salt, dkLen: 64, N: 16384, r: 8, p: 1)
// ...
}

View File

@@ -29,4 +29,6 @@ predicate toBeTested(Element e) {
)
)
)
or
toBeTested(e.(UnspecifiedElement).getParent())
}

View File

@@ -229,3 +229,44 @@
| expressions.swift:154:22:154:56 | \\...[...] | KeyPathApplicationExpr |
| expressions.swift:154:33:154:33 | keyPathB | DeclRefExpr |
| expressions.swift:154:52:154:55 | #keyPath(...) | KeyPathExpr |
| expressions.swift:158:3:158:3 | _ | DiscardAssignmentExpr |
| expressions.swift:158:3:158:8 | ... = ... | AssignExpr |
| expressions.swift:158:7:158:7 | .~(_:) | MethodRefExpr |
| expressions.swift:158:7:158:7 | Int.Type | TypeExpr |
| expressions.swift:158:7:158:8 | call to ~(_:) | PrefixUnaryExpr |
| expressions.swift:158:8:158:8 | 1 | IntegerLiteralExpr |
| expressions.swift:159:3:159:3 | _ | DiscardAssignmentExpr |
| expressions.swift:159:3:159:11 | ... = ... | AssignExpr |
| expressions.swift:159:7:159:7 | 1 | IntegerLiteralExpr |
| expressions.swift:159:7:159:11 | ... .&(_:_:) ... | BinaryExpr |
| expressions.swift:159:9:159:9 | .&(_:_:) | MethodRefExpr |
| expressions.swift:159:9:159:9 | Int.Type | TypeExpr |
| expressions.swift:159:11:159:11 | 2 | IntegerLiteralExpr |
| expressions.swift:160:3:160:3 | _ | DiscardAssignmentExpr |
| expressions.swift:160:3:160:11 | ... = ... | AssignExpr |
| expressions.swift:160:7:160:7 | 1 | IntegerLiteralExpr |
| expressions.swift:160:7:160:11 | ... .\|(_:_:) ... | BinaryExpr |
| expressions.swift:160:9:160:9 | .\|(_:_:) | MethodRefExpr |
| expressions.swift:160:9:160:9 | Int.Type | TypeExpr |
| expressions.swift:160:11:160:11 | 2 | IntegerLiteralExpr |
| expressions.swift:161:3:161:3 | _ | DiscardAssignmentExpr |
| expressions.swift:161:3:161:11 | ... = ... | AssignExpr |
| expressions.swift:161:7:161:7 | 1 | IntegerLiteralExpr |
| expressions.swift:161:7:161:11 | ... .^(_:_:) ... | BinaryExpr |
| expressions.swift:161:9:161:9 | .^(_:_:) | MethodRefExpr |
| expressions.swift:161:9:161:9 | Int.Type | TypeExpr |
| expressions.swift:161:11:161:11 | 2 | IntegerLiteralExpr |
| expressions.swift:162:3:162:3 | _ | DiscardAssignmentExpr |
| expressions.swift:162:3:162:12 | ... = ... | AssignExpr |
| expressions.swift:162:7:162:7 | 1 | IntegerLiteralExpr |
| expressions.swift:162:7:162:12 | ... .<<(_:_:) ... | BinaryExpr |
| expressions.swift:162:9:162:9 | .<<(_:_:) | MethodRefExpr |
| expressions.swift:162:9:162:9 | Int.Type | TypeExpr |
| expressions.swift:162:12:162:12 | 0 | IntegerLiteralExpr |
| expressions.swift:163:3:163:3 | _ | DiscardAssignmentExpr |
| expressions.swift:163:3:163:12 | ... = ... | AssignExpr |
| expressions.swift:163:7:163:7 | 1 | IntegerLiteralExpr |
| expressions.swift:163:7:163:12 | ... .>>(_:_:) ... | BinaryExpr |
| expressions.swift:163:9:163:9 | .>>(_:_:) | MethodRefExpr |
| expressions.swift:163:9:163:9 | Int.Type | TypeExpr |
| expressions.swift:163:12:163:12 | 0 | IntegerLiteralExpr |

View File

@@ -152,4 +152,13 @@ func test(a : A, keyPathInt : WritableKeyPath<A, Int>, keyPathB : WritableKeyPat
var apply_keyPathInt = a[keyPath: keyPathInt]
var apply_keyPathB = a[keyPath: keyPathB]
var nested_apply = a[keyPath: keyPathB][keyPath: \B.x]
}
}
func bitwise() {
_ = ~1
_ = 1 & 2
_ = 1 | 2
_ = 1 ^ 2
_ = 1 << 0
_ = 1 >> 0
}

View File

@@ -0,0 +1,9 @@
| wrong.swift:3:1:3:23 | missing extended_type_decl from ExtensionDecl | getProperty: | extended_type_decl | getError: | element was unspecified by the extractor |
| wrong.swift:9:9:9:9 | missing fallthrough_dest from FallthroughStmt | getProperty: | fallthrough_dest | getError: | element was unspecified by the extractor |
| wrong.swift:9:9:9:9 | missing fallthrough_source from FallthroughStmt | getProperty: | fallthrough_source | getError: | element was unspecified by the extractor |
| wrong.swift:12:18:12:21 | missing element from EnumElementPattern | getProperty: | element | getError: | element was unspecified by the extractor |
| wrong.swift:14:18:14:26 | missing element from EnumElementPattern | getProperty: | element | getError: | element was unspecified by the extractor |
| wrong.swift:19:18:19:19 | missing element from EnumElementPattern | getProperty: | element | getError: | element was unspecified by the extractor |
| wrong.swift:22:13:22:13 | missing fallthrough_dest from FallthroughStmt | getProperty: | fallthrough_dest | getError: | element was unspecified by the extractor |
| wrong.swift:22:13:22:13 | missing fallthrough_source from FallthroughStmt | getProperty: | fallthrough_source | getError: | element was unspecified by the extractor |
| wrong.swift:26:18:26:19 | missing element from EnumElementPattern | getProperty: | element | getError: | element was unspecified by the extractor |

View File

@@ -0,0 +1,11 @@
// generated by codegen/codegen.py
import codeql.swift.elements
import TestUtils
from UnspecifiedElement x, string getProperty, string getError
where
toBeTested(x) and
not x.isUnknown() and
getProperty = x.getProperty() and
getError = x.getError()
select x, "getProperty:", getProperty, "getError:", getError

View File

@@ -0,0 +1,7 @@
// generated by codegen/codegen.py
import codeql.swift.elements
import TestUtils
from UnspecifiedElement x
where toBeTested(x) and not x.isUnknown()
select x, x.getIndex()

View File

@@ -0,0 +1,9 @@
| wrong.swift:3:1:3:23 | missing extended_type_decl from ExtensionDecl | extension |
| wrong.swift:9:9:9:9 | missing fallthrough_dest from FallthroughStmt | fallthrough |
| wrong.swift:9:9:9:9 | missing fallthrough_source from FallthroughStmt | fallthrough |
| wrong.swift:12:18:12:21 | missing element from EnumElementPattern | (no string representation) |
| wrong.swift:14:18:14:26 | missing element from EnumElementPattern | (no string representation) |
| wrong.swift:19:18:19:19 | missing element from EnumElementPattern | (no string representation) |
| wrong.swift:22:13:22:13 | missing fallthrough_dest from FallthroughStmt | fallthrough |
| wrong.swift:22:13:22:13 | missing fallthrough_source from FallthroughStmt | fallthrough |
| wrong.swift:26:18:26:19 | missing element from EnumElementPattern | (no string representation) |

View File

@@ -0,0 +1,7 @@
// generated by codegen/codegen.py
import codeql.swift.elements
import TestUtils
from UnspecifiedElement x
where toBeTested(x) and not x.isUnknown()
select x, x.getParent()

View File

@@ -0,0 +1,30 @@
//codeql-extractor-expected-status: 1
extension Undefined { }
enum Enum {
case A, B
func test(e: Enum) {
fallthrough
switch e {
case .A():
break
case .B(let x):
let _ = x
break
case Int:
break
case .C:
break
default:
fallthrough
}
switch undefined {
case .Whatever:
break
}
}
}

View File

@@ -0,0 +1 @@
extractor-tests/generated/UnspecifiedElement/UnspecifiedElement.ql

Some files were not shown because too many files have changed in this diff Show More