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

Merge branch 'master' into js/invalid-entity-transcoding

Browse Source
This commit is contained in:
Max Schaefer
2018-11-30 16:49:20 +00:00
committed by GitHub
parent 10166be535 939db5a7cd
commit 52b8a6bb56
89 changed files with 1313 additions and 65 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
change-notes/1.20/analysis-javascript.md
View File

@@ -6,8 +6,8 @@
| **Query** | **Tags** | **Purpose** |
|-----------------------------------------------|------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| Double escaping or unescaping (`js/double-escaping') | correctness, security, external/cwe/cwe-116 | Highlights potential double escaping or unescaping of special characters, indicating a possible violation of [CWE-116](https://cwe.mitre.org/data/definitions/116.html). Results are shown on LGTM by default. |
| Double escaping or unescaping (`js/double-escaping`) | correctness, security, external/cwe/cwe-116 | Highlights potential double escaping or unescaping of special characters, indicating a possible violation of [CWE-116](https://cwe.mitre.org/data/definitions/116.html). Results are shown on LGTM by default. |
| Useless comparison test (`js/useless-comparison-test`) | correctness | Highlights code that is unreachable due to a numeric comparison that is always true or always false. |
## Changes to existing queries

2
cpp/ql/src/JPL_C/LOC-2/Rule 03/ExitNonterminatingLoop.ql
View File

@@ -4,6 +4,8 @@
* @kind problem
* @id cpp/jpl-c/exit-nonterminating-loop
* @problem.severity warning
* @tags correctness
* external/jpl
*/
import cpp

2
cpp/ql/src/JPL_C/LOC-2/Rule 03/LoopBounds.ql
View File

@@ -5,6 +5,8 @@
* @kind problem
* @id cpp/jpl-c/loop-bounds
* @problem.severity warning
* @tags correctness
* external/jpl
*/
import cpp

4
cpp/ql/src/JPL_C/LOC-2/Rule 04/Recursion.ql
View File

@@ -4,6 +4,10 @@
* @kind problem
* @id cpp/jpl-c/recursion
* @problem.severity warning
* @tags maintainability
* readability
* testability
* external/jpl
*/
import cpp

4
cpp/ql/src/JPL_C/LOC-2/Rule 05/HeapMemory.ql
View File

@@ -3,7 +3,9 @@
* @description Dynamic memory allocation (using malloc() or calloc()) should be confined to the initialization routines of a program.
* @kind problem
* @id cpp/jpl-c/heap-memory
* @problem.severity warning
* @problem.severity recommendation
* @tags resources
* external/jpl
*/
import cpp

3
cpp/ql/src/JPL_C/LOC-2/Rule 07/ThreadSafety.ql
View File

@@ -4,6 +4,9 @@
* @kind problem
* @id cpp/jpl-c/thread-safety
* @problem.severity warning
* @tags correctness
* concurrency
* external/jpl
*/
import cpp

3
cpp/ql/src/JPL_C/LOC-2/Rule 09/AvoidNestedSemaphores.ql
View File

@@ -4,6 +4,9 @@
* @kind problem
* @id cpp/jpl-c/avoid-nested-semaphores
* @problem.severity warning
* @tags correctness
* concurrency
* external/jpl
*/
import Semaphores

4
cpp/ql/src/JPL_C/LOC-2/Rule 09/AvoidSemaphores.ql
View File

@@ -3,7 +3,9 @@
* @description The use of semaphores or locks to access shared data should be avoided.
* @kind problem
* @id cpp/jpl-c/avoid-semaphores
* @problem.severity warning
* @problem.severity recommendation
* @tags concurrency
* external/jpl
*/
import Semaphores

3
cpp/ql/src/JPL_C/LOC-2/Rule 09/OutOfOrderLocks.ql
View File

@@ -4,6 +4,9 @@
* @kind problem
* @id cpp/jpl-c/out-of-order-locks
* @problem.severity warning
* @tags correctness
* concurrency
* external/jpl
*/
import Semaphores

3
cpp/ql/src/JPL_C/LOC-2/Rule 09/ReleaseLocksWhenAcquired.ql
View File

@@ -4,6 +4,9 @@
* @kind problem
* @id cpp/jpl-c/release-locks-when-acquired
* @problem.severity warning
* @tags correctness
* concurrency
* external/jpl
*/
import Semaphores

3
cpp/ql/src/JPL_C/LOC-2/Rule 11/SimpleControlFlowGoto.ql
View File

@@ -4,6 +4,9 @@
* @kind problem
* @id cpp/jpl-c/simple-control-flow-goto
* @problem.severity warning
* @tags maintainability
* readability
* external/jpl
*/
import cpp

4
cpp/ql/src/JPL_C/LOC-2/Rule 11/SimpleControlFlowJmp.ql
View File

@@ -4,6 +4,10 @@
* @kind problem
* @id cpp/jpl-c/simple-control-flow-jmp
* @problem.severity warning
* @tags correctness
* portability
* readability
* external/jpl
*/
import cpp

5
cpp/ql/src/JPL_C/LOC-2/Rule 12/EnumInitialization.ql
View File

@@ -3,7 +3,10 @@
* @description In an enumerator list, the = construct should not be used to explicitly initialize members other than the first, unless all items are explicitly initialized. An exception is the pattern to use the last element of an enumerator list to get the number of possible values.
* @kind problem
* @id cpp/jpl-c/enum-initialization
* @problem.severity warning
* @problem.severity recommendation
* @tags maintainability
* readability
* external/jpl
*/
import cpp

2
cpp/ql/src/JPL_C/LOC-3/Rule 13/ExternDeclsInHeader.ql
View File

@@ -4,6 +4,8 @@
* @kind problem
* @id cpp/jpl-c/extern-decls-in-header
* @problem.severity warning
* @tags maintainability
* external/jpl
*/
import cpp

6
cpp/ql/src/JPL_C/LOC-3/Rule 13/LimitedScopeFile.ql
View File

@@ -3,7 +3,11 @@
* @description Global variables that are not accessed outside their own file should be made static to promote information hiding.
* @kind problem
* @id cpp/jpl-c/limited-scope-file
* @problem.severity warning
* @problem.severity recommendation
* @precision low
* @tags maintainability
* modularity
* external/jpl
*/
import cpp

5
cpp/ql/src/JPL_C/LOC-3/Rule 13/LimitedScopeFunction.ql
View File

@@ -3,7 +3,10 @@
* @description Global and file-scope variables that are accessed by only one function should be scoped within that function.
* @kind problem
* @id cpp/jpl-c/limited-scope-function
* @problem.severity warning
* @problem.severity recommendation
* @precision low
* @tags maintainability
* external/jpl
*/
import cpp

5
cpp/ql/src/JPL_C/LOC-3/Rule 13/LimitedScopeLocalHidesGlobal.ql
View File

@@ -3,7 +3,10 @@
* @description A local variable or parameter that hides a global variable of the same name.
* @kind problem
* @id cpp/jpl-c/limited-scope-local-hides-global
* @problem.severity warning
* @problem.severity recommendation
* @tags maintainability
* readability
* external/jpl
*/
import cpp

3
cpp/ql/src/JPL_C/LOC-3/Rule 14/CheckingReturnValues.ql
View File

@@ -4,6 +4,9 @@
* @kind problem
* @id cpp/jpl-c/checking-return-values
* @problem.severity warning
* @tags correctness
* reliability
* external/jpl
*/
import cpp

3
cpp/ql/src/JPL_C/LOC-3/Rule 15/CheckingParameterValues.ql
View File

@@ -4,6 +4,9 @@
* @kind problem
* @id cpp/jpl-c/checking-parameter-values
* @problem.severity warning
* @tags correctness
* reliability
* external/jpl
*/
import JPL_C.Tasks

3
cpp/ql/src/JPL_C/LOC-3/Rule 16/UseOfAssertionsConstant.ql
View File

@@ -4,6 +4,9 @@
* @kind problem
* @id cpp/jpl-c/use-of-assertions-constant
* @problem.severity warning
* @tags maintainability
* reliability
* external/jpl
*/
import semmle.code.cpp.commons.Assertions

5
cpp/ql/src/JPL_C/LOC-3/Rule 16/UseOfAssertionsDensity.ql
View File

@@ -3,7 +3,10 @@
* @description All functions of more than 10 lines should have at least one assertion.
* @kind problem
* @id cpp/jpl-c/use-of-assertions-density
* @problem.severity warning
* @problem.severity recommendation
* @tags maintainability
* reliability
* external/jpl
*/
import semmle.code.cpp.commons.Assertions

2
cpp/ql/src/JPL_C/LOC-3/Rule 16/UseOfAssertionsNonBoolean.ql
View File

@@ -4,6 +4,8 @@
* @kind problem
* @id cpp/jpl-c/use-of-assertions-non-boolean
* @problem.severity warning
* @tags correctness
* external/jpl
*/
import semmle.code.cpp.commons.Assertions

2
cpp/ql/src/JPL_C/LOC-3/Rule 16/UseOfAssertionsSideEffect.ql
View File

@@ -4,6 +4,8 @@
* @kind problem
* @id cpp/jpl-c/use-of-assertions-side-effect
* @problem.severity warning
* @tags correctness
* external/jpl
*/
import semmle.code.cpp.commons.Assertions

5
cpp/ql/src/JPL_C/LOC-3/Rule 17/BasicIntTypes.ql
View File

@@ -3,7 +3,10 @@
* @description Typedefs that indicate size and signedness should be used in place of the basic types.
* @kind problem
* @id cpp/jpl-c/basic-int-types
* @problem.severity warning
* @problem.severity recommendation
* @tags maintainability
* readability
* external/jpl
*/
import cpp

5
cpp/ql/src/JPL_C/LOC-3/Rule 18/CompoundExpressions.ql
View File

@@ -3,7 +3,10 @@
* @description In compound expressions with multiple sub-expressions the intended order of evaluation shall be made explicit with parentheses.
* @kind problem
* @id cpp/jpl-c/compound-expressions
* @problem.severity warning
* @problem.severity recommendation
* @tags maintainability
* readability
* external/jpl
*/
import cpp

3
cpp/ql/src/JPL_C/LOC-3/Rule 19/NoBooleanSideEffects.ql
View File

@@ -4,6 +4,9 @@
* @kind problem
* @id cpp/jpl-c/no-boolean-side-effects
* @problem.severity warning
* @tags correctness
* readability
* external/jpl
*/
import cpp

5
cpp/ql/src/JPL_C/LOC-4/Rule 20/PreprocessorUse.ql
View File

@@ -3,7 +3,10 @@
* @description The use of the preprocessor must be limited to inclusion of header files and simple macro definitions.
* @kind problem
* @id cpp/jpl-c/preprocessor-use
* @problem.severity warning
* @problem.severity recommendation
* @tags maintainability
* readability
* external/jpl
*/
import cpp

5
cpp/ql/src/JPL_C/LOC-4/Rule 20/PreprocessorUseIfdef.ql
View File

@@ -3,7 +3,10 @@
* @description The use of conditional compilation directives must be kept to a minimum -- e.g. for header guards only.
* @kind problem
* @id cpp/jpl-c/preprocessor-use-ifdef
* @problem.severity warning
* @problem.severity recommendation
* @tags maintainability
* readability
* external/jpl
*/
import cpp

5
cpp/ql/src/JPL_C/LOC-4/Rule 20/PreprocessorUsePartial.ql
View File

@@ -3,7 +3,10 @@
* @description Macros must expand to complete syntactic units -- "#define MY_IF if(" is not legal.
* @kind problem
* @id cpp/jpl-c/preprocessor-use-partial
* @problem.severity warning
* @problem.severity recommendation
* @tags maintainability
* readability
* external/jpl
*/
import cpp

5
cpp/ql/src/JPL_C/LOC-4/Rule 20/PreprocessorUseUndisciplined.ql
View File

@@ -3,7 +3,10 @@
* @description Macros are not allowed to use complex preprocessor features like variable argument lists and token pasting.
* @kind problem
* @id cpp/jpl-c/preprocessor-use-undisciplined
* @problem.severity warning
* @problem.severity recommendation
* @tags maintainability
* readability
* external/jpl
*/
import cpp

5
cpp/ql/src/JPL_C/LOC-4/Rule 21/MacroInBlock.ql
View File

@@ -3,7 +3,10 @@
* @description Macros shall not be #define'd within a function or a block.
* @kind problem
* @id cpp/jpl-c/macro-in-block
* @problem.severity warning
* @problem.severity recommendation
* @tags maintainability
* readability
* external/jpl
*/
import cpp

5
cpp/ql/src/JPL_C/LOC-4/Rule 22/UseOfUndef.ql
View File

@@ -3,7 +3,10 @@
* @description #undef shall not be used.
* @kind problem
* @id cpp/jpl-c/use-of-undef
* @problem.severity warning
* @problem.severity recommendation
* @tags maintainability
* readability
* external/jpl
*/
import cpp

3
cpp/ql/src/JPL_C/LOC-4/Rule 23/MismatchedIfdefs.ql
View File

@@ -4,6 +4,9 @@
* @kind problem
* @id cpp/jpl-c/mismatched-ifdefs
* @problem.severity warning
* @tags maintainability
* readability
* external/jpl
*/
import cpp

5
cpp/ql/src/JPL_C/LOC-4/Rule 24/MultipleStmtsPerLine.ql
View File

@@ -3,7 +3,10 @@
* @description Putting more than one statement on a single line hinders program understanding.
* @kind problem
* @id cpp/jpl-c/multiple-stmts-per-line
* @problem.severity warning
* @problem.severity recommendation
* @tags maintainability
* readability
* external/jpl
*/
import cpp

5
cpp/ql/src/JPL_C/LOC-4/Rule 24/MultipleVarDeclsPerLine.ql
View File

@@ -3,7 +3,10 @@
* @description There should be no more than one variable declaration per line.
* @kind problem
* @id cpp/jpl-c/multiple-var-decls-per-line
* @problem.severity warning
* @problem.severity recommendation
* @tags maintainability
* readability
* external/jpl
*/
import cpp

5
cpp/ql/src/JPL_C/LOC-4/Rule 25/FunctionSizeLimits.ql
View File

@@ -3,7 +3,10 @@
* @description Function length should be limited to what can be printed on a single sheet of paper (60 lines). Number of parameters is limited to 6 or fewer.
* @kind problem
* @id cpp/jpl-c/function-size-limits
* @problem.severity warning
* @problem.severity recommendation
* @tags maintainability
* readability
* external/jpl
*/
import cpp

5
cpp/ql/src/JPL_C/LOC-4/Rule 26/DeclarationPointerNesting.ql
View File

@@ -3,7 +3,10 @@
* @description The declaration of an object should contain no more than two levels of indirection.
* @kind problem
* @id cpp/jpl-c/declaration-pointer-nesting
* @problem.severity warning
* @problem.severity recommendation
* @tags maintainability
* readability
* external/jpl
*/
import cpp

5
cpp/ql/src/JPL_C/LOC-4/Rule 27/PointerDereferenceInStmt.ql
View File

@@ -3,7 +3,10 @@
* @description Statements should contain no more than two levels of dereferencing per object.
* @kind problem
* @id cpp/jpl-c/pointer-dereference-in-stmt
* @problem.severity warning
* @problem.severity recommendation
* @tags maintainability
* readability
* external/jpl
*/
import cpp

5
cpp/ql/src/JPL_C/LOC-4/Rule 28/HiddenPointerDereferenceMacro.ql
View File

@@ -3,7 +3,10 @@
* @description Pointer dereference operations should not be hidden in macro definitions.
* @kind problem
* @id cpp/jpl-c/hidden-pointer-dereference-macro
* @problem.severity warning
* @problem.severity recommendation
* @tags maintainability
* readability
* external/jpl
*/
import cpp

5
cpp/ql/src/JPL_C/LOC-4/Rule 28/HiddenPointerIndirectionTypedef.ql
View File

@@ -3,7 +3,10 @@
* @description Pointer indirection may not be hidden by typedefs -- "typedef int* IntPtr;" is not allowed.
* @kind problem
* @id cpp/jpl-c/hidden-pointer-indirection-typedef
* @problem.severity warning
* @problem.severity recommendation
* @tags maintainability
* readability
* external/jpl
*/
import cpp

6
cpp/ql/src/JPL_C/LOC-4/Rule 29/NonConstFunctionPointer.ql
View File

@@ -3,7 +3,11 @@
* @description Non-constant pointers to functions should not be used.
* @kind problem
* @id cpp/jpl-c/non-const-function-pointer
* @problem.severity warning
* @problem.severity recommendation
* @precision low
* @tags maintainability
* readability
* external/jpl
*/
import cpp

3
cpp/ql/src/JPL_C/LOC-4/Rule 30/FunctionPointerConversions.ql
View File

@@ -4,6 +4,9 @@
* @kind problem
* @id cpp/jpl-c/function-pointer-conversions
* @problem.severity warning
* @precision low
* @tags correctness
* external/jpl
*/
import cpp

5
cpp/ql/src/JPL_C/LOC-4/Rule 31/IncludesFirst.ql
View File

@@ -3,7 +3,10 @@
* @description #include directives in a file shall only be preceded by other preprocessor directives or comments.
* @kind problem
* @id cpp/jpl-c/includes-first
* @problem.severity warning
* @problem.severity recommendation
* @tags maintainability
* readability
* external/jpl
*/
import cpp

5
cpp/ql/src/external/DuplicateBlock.ql vendored
View File

@@ -4,6 +4,11 @@
* @kind problem
* @id cpp/duplicate-block
* @problem.severity recommendation
* @precision medium
* @tags testability
* maintainability
* duplicate-code
* non-attributable
*/
import CodeDuplication

2
cpp/ql/src/semmle/code/cpp/controlflow/Dominance.qll
View File

@@ -126,6 +126,7 @@ predicate bbIPostDominates(BasicBlock pDom, BasicBlock node) = idominance(bb_exi
* Holds if `dominator` is a strict dominator of `node` in the control-flow
* graph of basic blocks. Being strict means that `dominator != node`.
*/
pragma[nomagic] // magic prevents fastTC
predicate bbStrictlyDominates(BasicBlock dominator, BasicBlock node) {
bbIDominates+(dominator, node)
}
@@ -134,6 +135,7 @@ predicate bbStrictlyDominates(BasicBlock dominator, BasicBlock node) {
* Holds if `postDominator` is a strict post-dominator of `node` in the control-flow
* graph of basic blocks. Being strict means that `postDominator != node`.
*/
pragma[nomagic] // magic prevents fastTC
predicate bbStrictlyPostDominates(BasicBlock postDominator, BasicBlock node) {
bbIPostDominates+(postDominator, node)
}

3
cpp/ql/test/query-tests/JPL_C/LOC-4/Rule 29/NonConstFunctionPointer/NonConstFunctionPointer.expected Normal file
View File

@@ -0,0 +1,3 @@
| test.c:18:2:18:10 | call to expression | This call does not go through a const function pointer. |
| test.c:19:2:19:10 | call to expression | This call does not go through a const function pointer. |
| test.c:20:2:20:10 | call to expression | This call does not go through a const function pointer. |

1
cpp/ql/test/query-tests/JPL_C/LOC-4/Rule 29/NonConstFunctionPointer/NonConstFunctionPointer.qlref Normal file
View File

@@ -0,0 +1 @@
JPL_C/LOC-4/Rule 29/NonConstFunctionPointer.ql

21
cpp/ql/test/query-tests/JPL_C/LOC-4/Rule 29/NonConstFunctionPointer/test.c Normal file
View File

@@ -0,0 +1,21 @@
// test.c
void myFunc1();
void myFunc2();
typedef void (*voidFunPointer)();
void test()
{
void (*funPtr1)() = &myFunc1;
const void (*funPtr2)() = &myFunc1;
const voidFunPointer funPtr3 = &myFunc1;
funPtr1 = &myFunc2;
funPtr2 = &myFunc2;
//funPtr3 = &myFunc2; --- this would be a compilation error
funPtr1(); // BAD
funPtr2(); // BAD
funPtr3(); // GOOD [FALSE POSITIVE]
}

6
cpp/ql/test/query-tests/JPL_C/LOC-4/Rule 30/FunctionPointerConversions/FunctionPointerConversions.expected Normal file
View File

@@ -0,0 +1,6 @@
| test.c:11:16:11:23 | & ... | Function pointer converted to int *, which is not an integral type. |
| test.c:12:18:12:25 | & ... | Function pointer converted to void *, which is not an integral type. |
| test.c:17:11:17:17 | funPtr1 | Function pointer converted to int *, which is not an integral type. |
| test.c:18:12:18:18 | funPtr1 | Function pointer converted to void *, which is not an integral type. |
| test.c:29:18:29:24 | funPtr1 | Function pointer converted to int *, which is not an integral type. |
| test.c:30:20:30:26 | funPtr1 | Function pointer converted to void *, which is not an integral type. |

1
cpp/ql/test/query-tests/JPL_C/LOC-4/Rule 30/FunctionPointerConversions/FunctionPointerConversions.qlref Normal file
View File

@@ -0,0 +1 @@
JPL_C/LOC-4/Rule 30/FunctionPointerConversions.ql

32
cpp/ql/test/query-tests/JPL_C/LOC-4/Rule 30/FunctionPointerConversions/test.c Normal file
View File

@@ -0,0 +1,32 @@
// test.c
void myFunc1();
typedef void (*voidFunPtr)();
void test()
{
void (*funPtr1)() = &myFunc1; // GOOD
voidFunPtr funPtr2 = &myFunc1; // GOOD
int *intPtr = &myFunc1; // BAD (function pointer -> int pointer)
void *voidPtr = &myFunc1; // BAD (function pointer -> void pointer)
int i = &myFunc1; // GOOD (permitted)
funPtr1 = funPtr1; // GOOD
funPtr2 = funPtr1; // GOOD
intPtr = funPtr1; // BAD (function pointer -> int pointer)
voidPtr = funPtr1; // BAD (function pointer -> void pointer)
i = funPtr1; // GOOD (permitted)
funPtr1 = funPtr2; // GOOD
funPtr2 = funPtr2; // GOOD
intPtr = funPtr2; // BAD (function pointer -> int pointer) [NOT DETECTED]
voidPtr = funPtr2; // BAD (function pointer -> void pointer) [NOT DETECTED]
i = funPtr2; // GOOD (permitted)
funPtr1 = (void (*)())funPtr1; // GOOD
funPtr2 = (voidFunPtr)funPtr1; // GOOD
intPtr = (int *)funPtr1; // BAD (function pointer -> int pointer)
voidPtr = (void *)funPtr1; // BAD (function pointer -> void pointer)
i = (int)funPtr1; // GOOD (permitted)
}

5
csharp/ql/src/semmle/code/csharp/commons/Assertions.qll
View File

@@ -55,7 +55,8 @@ class Assertion extends MethodCall {
bb = this.getAControlFlowNode().getBasicBlock() |
result = bb.getASuccessor*()
) and
result.getASuccessor() = jb
result.getASuccessor() = jb and
not jb.dominates(result)
}
pragma[nomagic]
@@ -64,6 +65,8 @@ class Assertion extends MethodCall {
forall(BasicBlock pred |
pred = jb.getAPredecessor() |
pred = this.getAPossiblyDominatedPredecessor(jb)
or
jb.dominates(pred)
)
}

5
csharp/ql/src/semmle/code/csharp/controlflow/ControlFlowElement.qll
View File

@@ -106,7 +106,8 @@ class ControlFlowElement extends ExprOrStmtParent, @control_flow_element {
this.immediatelyControls(mid, s) |
result = mid.getASuccessor*()
) and
result.getASuccessor() = controlled
result.getASuccessor() = controlled and
not controlled.dominates(result)
}
pragma[nomagic]
@@ -115,6 +116,8 @@ class ControlFlowElement extends ExprOrStmtParent, @control_flow_element {
forall(BasicBlock pred |
pred = controlled.getAPredecessor() |
pred = this.getAPossiblyControlledPredecessor(controlled, s)
or
controlled.dominates(pred)
)
}

6
csharp/ql/src/semmle/code/csharp/controlflow/ControlFlowGraph.qll
View File

@@ -353,6 +353,12 @@ module ControlFlow {
)
}
/** Gets a comma-separated list of strings for each split in this node, if any. */
string getSplitsString() {
result = splits.toString() and
result != ""
}
/** Gets a split for this control flow node, if any. */
Split getASplit() {
result = splits.getASplit()

23
csharp/ql/src/semmle/code/csharp/controlflow/Guards.qll
View File

@@ -285,7 +285,7 @@ class AccessOrCallExpr extends Expr {
Declaration getTarget() { result = target }
/**
* Gets the (non-trivial) SSA definition corresponding to the longest
* Gets a (non-trivial) SSA definition corresponding to the longest
* qualifier chain of this expression, if any.
*
* This includes the case where this expression is itself an access to an
@@ -299,13 +299,11 @@ class AccessOrCallExpr extends Expr {
* x.Foo().Bar(); // SSA qualifier: SSA definition for `x`
* x; // SSA qualifier: SSA definition for `x`
* ```
*
* An expression can have more than one SSA qualifier in the presence
* of control flow splitting.
*/
Ssa::Definition getSsaQualifier() { result = getSsaQualifier(this) }
/**
* Holds if this expression has an SSA qualifier.
*/
predicate hasSsaQualifier() { exists(this.getSsaQualifier()) }
Ssa::Definition getAnSsaQualifier() { result = getAnSsaQualifier(this) }
}
private Declaration getDeclarationTarget(Expr e) {
@@ -313,11 +311,11 @@ private Declaration getDeclarationTarget(Expr e) {
result = e.(Call).getTarget()
}
private Ssa::Definition getSsaQualifier(Expr e) {
private Ssa::Definition getAnSsaQualifier(Expr e) {
e = getATrackedRead(result)
or
not e = getATrackedRead(_) and
result = getSsaQualifier(e.(QualifiableExpr).getQualifier())
result = getAnSsaQualifier(e.(QualifiableExpr).getQualifier())
}
private AssignableRead getATrackedRead(Ssa::Definition def) {
@@ -688,10 +686,9 @@ module Internal {
predicate isGuardedBy(AccessOrCallExpr guarded, Guard g, AccessOrCallExpr sub, AbstractValue v) {
isGuardedBy1(guarded, g, sub, v) and
sub = g.getAChildExpr*() and
(
not guarded.hasSsaQualifier() and not sub.hasSsaQualifier()
or
guarded.getSsaQualifier() = sub.getSsaQualifier()
forall(Ssa::Definition def |
def = sub.getAnSsaQualifier() |
def = guarded.getAnSsaQualifier()
)
}
}

35
csharp/ql/src/semmle/code/csharp/dataflow/SSA.qll
View File

@@ -1975,6 +1975,25 @@ module Ssa {
private import SsaImpl
private string getSplitString(Definition def) {
exists(BasicBlock bb, int i, ControlFlow::Node cfn |
definesAt(def, bb, i, _) and
result = cfn.(ControlFlow::Nodes::ElementNode).getSplitsString()
|
cfn = bb.getNode(i)
or
not exists(bb.getNode(i)) and
cfn = bb.getFirstNode()
)
}
private string getToStringPrefix(Definition def) {
result = "[" + getSplitString(def) + "] "
or
not exists(getSplitString(def)) and
result = ""
}
/**
* A static single assignment (SSA) definition. Either an explicit variable
* definition (`ExplicitDefinition`), an implicit variable definition
@@ -2311,9 +2330,9 @@ module Ssa {
override string toString() {
if this.getADefinition() instanceof AssignableDefinitions::ImplicitParameterDefinition then
result = "SSA param(" + this.getSourceVariable() + ")"
result = getToStringPrefix(this) + "SSA param(" + this.getSourceVariable() + ")"
else
result = "SSA def(" + this.getSourceVariable() + ")"
result = getToStringPrefix(this) + "SSA def(" + this.getSourceVariable() + ")"
}
override Location getLocation() {
@@ -2354,9 +2373,9 @@ module Ssa {
override string toString() {
if this.getSourceVariable().getAssignable() instanceof LocalScopeVariable then
result = "SSA capture def(" + this.getSourceVariable() + ")"
result = getToStringPrefix(this) + "SSA capture def(" + this.getSourceVariable() + ")"
else
result = "SSA entry def(" + this.getSourceVariable() + ")"
result = getToStringPrefix(this) + "SSA entry def(" + this.getSourceVariable() + ")"
}
override Location getLocation() {
@@ -2391,7 +2410,7 @@ module Ssa {
}
override string toString() {
result = "SSA call def(" + getSourceVariable() + ")"
result = getToStringPrefix(this) + "SSA call def(" + getSourceVariable() + ")"
}
override Location getLocation() {
@@ -2410,7 +2429,7 @@ module Ssa {
}
override string toString() {
result = "SSA qualifier def(" + getSourceVariable() + ")"
result = getToStringPrefix(this) + "SSA qualifier def(" + getSourceVariable() + ")"
}
override Location getLocation() {
@@ -2435,7 +2454,7 @@ module Ssa {
}
override string toString() {
result = "SSA untracked def(" + getSourceVariable() + ")"
result = getToStringPrefix(this) + "SSA untracked def(" + getSourceVariable() + ")"
}
override Location getLocation() {
@@ -2497,7 +2516,7 @@ module Ssa {
}
override string toString() {
result = "SSA phi(" + getSourceVariable() + ")"
result = getToStringPrefix(this) + "SSA phi(" + getSourceVariable() + ")"
}
/*

3
csharp/ql/test/library-tests/controlflow/guards/BooleanGuardedExpr.expected
View File

@@ -62,6 +62,8 @@
| Guards.cs:194:31:194:31 | access to parameter s | Guards.cs:193:14:193:25 | call to method NullTest3 | Guards.cs:193:24:193:24 | access to parameter s | false |
| Guards.cs:196:31:196:31 | access to parameter s | Guards.cs:195:13:195:27 | call to method NotNullTest4 | Guards.cs:195:26:195:26 | access to parameter s | true |
| Guards.cs:198:31:198:31 | access to parameter s | Guards.cs:197:14:197:29 | call to method NullTestWrong | Guards.cs:197:28:197:28 | access to parameter s | false |
| Guards.cs:205:13:205:13 | access to parameter o | Guards.cs:203:13:203:21 | ... != ... | Guards.cs:203:13:203:13 | access to parameter o | true |
| Guards.cs:208:17:208:17 | access to parameter o | Guards.cs:203:13:203:21 | ... != ... | Guards.cs:203:13:203:13 | access to parameter o | true |
| Splitting.cs:13:17:13:17 | access to parameter o | Splitting.cs:12:17:12:25 | ... != ... | Splitting.cs:12:17:12:17 | access to parameter o | true |
| Splitting.cs:23:24:23:24 | access to parameter o | Splitting.cs:22:17:22:25 | ... != ... | Splitting.cs:22:17:22:17 | access to parameter o | true |
| Splitting.cs:25:13:25:13 | access to parameter o | Splitting.cs:22:17:22:25 | ... != ... | Splitting.cs:22:17:22:17 | access to parameter o | false |
@@ -80,3 +82,4 @@
| Splitting.cs:117:9:117:9 | access to parameter o | Splitting.cs:116:22:116:30 | ... != ... | Splitting.cs:116:22:116:22 | access to parameter o | true |
| Splitting.cs:119:13:119:13 | access to parameter o | Splitting.cs:116:22:116:30 | ... != ... | Splitting.cs:116:22:116:22 | access to parameter o | true |
| Splitting.cs:120:16:120:16 | access to parameter o | Splitting.cs:116:22:116:30 | ... != ... | Splitting.cs:116:22:116:22 | access to parameter o | true |
| Splitting.cs:132:25:132:25 | access to parameter b | Splitting.cs:130:21:130:21 | access to parameter b | Splitting.cs:130:21:130:21 | access to parameter b | false |

5
csharp/ql/test/library-tests/controlflow/guards/GuardedExpr.expected
View File

@@ -155,6 +155,10 @@
| Guards.cs:196:31:196:31 | access to parameter s | Guards.cs:195:13:195:27 | call to method NotNullTest4 | Guards.cs:195:26:195:26 | access to parameter s | true |
| Guards.cs:196:31:196:31 | access to parameter s | Guards.cs:195:26:195:26 | access to parameter s | Guards.cs:195:26:195:26 | access to parameter s | non-null |
| Guards.cs:198:31:198:31 | access to parameter s | Guards.cs:197:14:197:29 | call to method NullTestWrong | Guards.cs:197:28:197:28 | access to parameter s | false |
| Guards.cs:205:13:205:13 | access to parameter o | Guards.cs:203:13:203:13 | access to parameter o | Guards.cs:203:13:203:13 | access to parameter o | non-null |
| Guards.cs:205:13:205:13 | access to parameter o | Guards.cs:203:13:203:21 | ... != ... | Guards.cs:203:13:203:13 | access to parameter o | true |
| Guards.cs:208:17:208:17 | access to parameter o | Guards.cs:203:13:203:13 | access to parameter o | Guards.cs:203:13:203:13 | access to parameter o | non-null |
| Guards.cs:208:17:208:17 | access to parameter o | Guards.cs:203:13:203:21 | ... != ... | Guards.cs:203:13:203:13 | access to parameter o | true |
| Splitting.cs:13:17:13:17 | access to parameter o | Splitting.cs:12:17:12:17 | access to parameter o | Splitting.cs:12:17:12:17 | access to parameter o | non-null |
| Splitting.cs:13:17:13:17 | access to parameter o | Splitting.cs:12:17:12:25 | ... != ... | Splitting.cs:12:17:12:17 | access to parameter o | true |
| Splitting.cs:23:24:23:24 | access to parameter o | Splitting.cs:22:17:22:17 | access to parameter o | Splitting.cs:22:17:22:17 | access to parameter o | non-null |
@@ -191,3 +195,4 @@
| Splitting.cs:119:13:119:13 | access to parameter o | Splitting.cs:116:22:116:30 | ... != ... | Splitting.cs:116:22:116:22 | access to parameter o | true |
| Splitting.cs:120:16:120:16 | access to parameter o | Splitting.cs:116:22:116:22 | access to parameter o | Splitting.cs:116:22:116:22 | access to parameter o | non-null |
| Splitting.cs:120:16:120:16 | access to parameter o | Splitting.cs:116:22:116:30 | ... != ... | Splitting.cs:116:22:116:22 | access to parameter o | true |
| Splitting.cs:132:25:132:25 | access to parameter b | Splitting.cs:130:21:130:21 | access to parameter b | Splitting.cs:130:21:130:21 | access to parameter b | false |

12
csharp/ql/test/library-tests/controlflow/guards/Guards.cs
View File

@@ -197,4 +197,16 @@ public class Guards
if (!NullTestWrong(s))
Console.WriteLine(s); // not null guarded
}
void M17(object o, string[] args)
{
if (o != null)
{
o.ToString(); // null guarded
foreach (var arg in args)
{
o.ToString(); // null guarded
}
}
}
}

6
csharp/ql/test/library-tests/controlflow/guards/Implications.expected
View File

@@ -200,6 +200,8 @@
| Guards.cs:195:13:195:27 | call to method NotNullTest4 | true | Guards.cs:195:26:195:26 | access to parameter s | non-null |
| Guards.cs:197:13:197:29 | !... | false | Guards.cs:197:14:197:29 | call to method NullTestWrong | true |
| Guards.cs:197:13:197:29 | !... | true | Guards.cs:197:14:197:29 | call to method NullTestWrong | false |
| Guards.cs:203:13:203:21 | ... != ... | false | Guards.cs:203:13:203:13 | access to parameter o | null |
| Guards.cs:203:13:203:21 | ... != ... | true | Guards.cs:203:13:203:13 | access to parameter o | non-null |
| Splitting.cs:12:17:12:25 | ... != ... | false | Splitting.cs:12:17:12:17 | access to parameter o | null |
| Splitting.cs:12:17:12:25 | ... != ... | true | Splitting.cs:12:17:12:17 | access to parameter o | non-null |
| Splitting.cs:22:17:22:25 | ... != ... | false | Splitting.cs:22:17:22:17 | access to parameter o | null |
@@ -222,3 +224,7 @@
| Splitting.cs:105:22:105:30 | ... != ... | true | Splitting.cs:105:22:105:22 | access to parameter o | non-null |
| Splitting.cs:116:22:116:30 | ... != ... | false | Splitting.cs:116:22:116:22 | access to parameter o | null |
| Splitting.cs:116:22:116:30 | ... != ... | true | Splitting.cs:116:22:116:22 | access to parameter o | non-null |
| Splitting.cs:128:17:128:25 | ... != ... | false | Splitting.cs:128:17:128:17 | access to local variable o | null |
| Splitting.cs:128:17:128:25 | ... != ... | true | Splitting.cs:128:17:128:17 | access to local variable o | non-null |
| Splitting.cs:133:17:133:17 | access to local variable o | non-null | Splitting.cs:132:21:132:29 | call to method M11 | non-null |
| Splitting.cs:133:17:133:17 | access to local variable o | null | Splitting.cs:132:21:132:29 | call to method M11 | null |

2
csharp/ql/test/library-tests/controlflow/guards/NullGuardedExpr.expected
View File

@@ -39,6 +39,8 @@
| Guards.cs:192:31:192:31 | access to parameter s |
| Guards.cs:194:31:194:31 | access to parameter s |
| Guards.cs:196:31:196:31 | access to parameter s |
| Guards.cs:205:13:205:13 | access to parameter o |
| Guards.cs:208:17:208:17 | access to parameter o |
| Splitting.cs:13:17:13:17 | access to parameter o |
| Splitting.cs:23:24:23:24 | access to parameter o |
| Splitting.cs:35:13:35:13 | access to parameter o |

16
csharp/ql/test/library-tests/controlflow/guards/Splitting.cs
View File

@@ -119,4 +119,20 @@ public class Splitting
o.ToString(); // null guarded
return o.ToString(); // null guarded
}
public void M12(int i, bool b)
{
object o = null;
do
{
if (o != null)
{
if (b)
return;
o = M11(b, o);
o.GetHashCode(); // not null guarded
}
}
while (i > 0);
}
}

4
csharp/ql/test/library-tests/dataflow/ssa/SsaAdjacentUncertainRead.expected
View File

@@ -69,7 +69,7 @@ WARNING: Predicate getAFirstUncertainRead has been deprecated and may be removed
| Capture.cs:236:9:236:12 | SSA call def(i) | Capture.cs:237:34:237:34 | access to local variable i |
| Consistency.cs:7:25:7:25 | SSA param(b) | Consistency.cs:11:17:11:17 | access to parameter b |
| Consistency.cs:15:17:15:21 | SSA def(i) | Consistency.cs:16:17:16:17 | access to local variable i |
| Consistency.cs:15:17:15:21 | SSA def(i) | Consistency.cs:16:17:16:17 | access to local variable i |
| Consistency.cs:15:17:15:21 | [finally: exception(Exception)] SSA def(i) | Consistency.cs:16:17:16:17 | access to local variable i |
| Consistency.cs:25:29:25:29 | SSA def(c) | Consistency.cs:26:13:26:13 | access to local variable c |
| Consistency.cs:25:29:25:29 | SSA qualifier def(c.Field) | Consistency.cs:26:13:26:19 | access to field Field |
| Consistency.cs:32:9:32:29 | SSA def(c) | Consistency.cs:33:9:33:9 | access to parameter c |
@@ -318,7 +318,7 @@ WARNING: Predicate getAFirstUncertainRead has been deprecated and may be removed
| Test.cs:46:10:46:10 | SSA entry def(this.field) | Test.cs:56:13:56:17 | access to field field |
| Test.cs:46:16:46:18 | SSA param(in) | Test.cs:48:13:48:15 | access to parameter in |
| Test.cs:57:9:57:17 | SSA def(this.field) | Test.cs:58:13:58:17 | access to field field |
| Test.cs:68:45:68:45 | SSA def(e) | Test.cs:70:17:70:17 | access to local variable e |
| Test.cs:68:45:68:45 | [exception: DivideByZeroException] SSA def(e) | Test.cs:70:17:70:17 | access to local variable e |
| Tuples.cs:10:9:10:54 | SSA def(b) | Tuples.cs:12:13:12:13 | access to local variable b |
| Tuples.cs:10:9:10:54 | SSA def(s) | Tuples.cs:13:13:13:13 | access to local variable s |
| Tuples.cs:10:9:10:54 | SSA def(x) | Tuples.cs:11:13:11:13 | access to local variable x |

4
csharp/ql/test/library-tests/dataflow/ssa/SsaDef.expected
View File

@@ -84,7 +84,7 @@
| Capture.cs:229:13:229:13 | i | Capture.cs:236:9:236:12 | SSA call def(i) |
| Consistency.cs:7:25:7:25 | b | Consistency.cs:7:25:7:25 | SSA param(b) |
| Consistency.cs:15:17:15:17 | i | Consistency.cs:15:17:15:21 | SSA def(i) |
| Consistency.cs:15:17:15:17 | i | Consistency.cs:15:17:15:21 | SSA def(i) |
| Consistency.cs:15:17:15:17 | i | Consistency.cs:15:17:15:21 | [finally: exception(Exception)] SSA def(i) |
| Consistency.cs:25:29:25:29 | c | Consistency.cs:25:29:25:29 | SSA def(c) |
| Consistency.cs:26:13:26:19 | c.Field | Consistency.cs:25:29:25:29 | SSA qualifier def(c.Field) |
| Consistency.cs:30:30:30:30 | c | Consistency.cs:32:9:32:29 | SSA def(c) |
@@ -342,7 +342,7 @@
| Test.cs:46:29:46:32 | out | Test.cs:56:9:56:19 | SSA phi(out) |
| Test.cs:56:13:56:17 | this.field | Test.cs:46:10:46:10 | SSA entry def(this.field) |
| Test.cs:56:13:56:17 | this.field | Test.cs:57:9:57:17 | SSA def(this.field) |
| Test.cs:68:45:68:45 | e | Test.cs:68:45:68:45 | SSA def(e) |
| Test.cs:68:45:68:45 | e | Test.cs:68:45:68:45 | [exception: DivideByZeroException] SSA def(e) |
| Tuples.cs:10:14:10:14 | x | Tuples.cs:10:9:10:54 | SSA def(x) |
| Tuples.cs:10:14:10:14 | x | Tuples.cs:14:9:14:32 | SSA def(x) |
| Tuples.cs:10:14:10:14 | x | Tuples.cs:23:9:23:37 | SSA def(x) |

4
csharp/ql/test/library-tests/dataflow/ssa/SsaDefLastRead.expected
View File

@@ -53,7 +53,7 @@
| Capture.cs:229:13:229:13 | i | Capture.cs:236:9:236:12 | SSA call def(i) | Capture.cs:237:34:237:34 | access to local variable i |
| Consistency.cs:7:25:7:25 | b | Consistency.cs:7:25:7:25 | SSA param(b) | Consistency.cs:11:17:11:17 | access to parameter b |
| Consistency.cs:15:17:15:17 | i | Consistency.cs:15:17:15:21 | SSA def(i) | Consistency.cs:16:17:16:17 | access to local variable i |
| Consistency.cs:15:17:15:17 | i | Consistency.cs:15:17:15:21 | SSA def(i) | Consistency.cs:16:17:16:17 | access to local variable i |
| Consistency.cs:15:17:15:17 | i | Consistency.cs:15:17:15:21 | [finally: exception(Exception)] SSA def(i) | Consistency.cs:16:17:16:17 | access to local variable i |
| Consistency.cs:25:29:25:29 | c | Consistency.cs:25:29:25:29 | SSA def(c) | Consistency.cs:27:13:27:13 | access to local variable c |
| Consistency.cs:26:13:26:19 | c.Field | Consistency.cs:25:29:25:29 | SSA qualifier def(c.Field) | Consistency.cs:27:13:27:19 | access to field Field |
| Consistency.cs:44:11:44:11 | s | Consistency.cs:44:11:44:11 | SSA def(s) | Consistency.cs:46:13:46:13 | access to local variable s |
@@ -235,7 +235,7 @@
| Test.cs:46:16:46:18 | in | Test.cs:46:16:46:18 | SSA param(in) | Test.cs:48:13:48:15 | access to parameter in |
| Test.cs:56:13:56:17 | this.field | Test.cs:46:10:46:10 | SSA entry def(this.field) | Test.cs:56:13:56:17 | access to field field |
| Test.cs:56:13:56:17 | this.field | Test.cs:57:9:57:17 | SSA def(this.field) | Test.cs:58:13:58:17 | access to field field |
| Test.cs:68:45:68:45 | e | Test.cs:68:45:68:45 | SSA def(e) | Test.cs:70:17:70:17 | access to local variable e |
| Test.cs:68:45:68:45 | e | Test.cs:68:45:68:45 | [exception: DivideByZeroException] SSA def(e) | Test.cs:70:17:70:17 | access to local variable e |
| Tuples.cs:10:14:10:14 | x | Tuples.cs:10:9:10:54 | SSA def(x) | Tuples.cs:11:13:11:13 | access to local variable x |
| Tuples.cs:10:14:10:14 | x | Tuples.cs:14:9:14:32 | SSA def(x) | Tuples.cs:15:13:15:13 | access to local variable x |
| Tuples.cs:10:14:10:14 | x | Tuples.cs:23:9:23:37 | SSA def(x) | Tuples.cs:24:13:24:13 | access to local variable x |

4
csharp/ql/test/library-tests/dataflow/ssa/SsaExplicitDef.expected
View File

@@ -56,7 +56,7 @@
| Capture.cs:229:13:229:13 | i | Capture.cs:235:21:235:25 | SSA def(i) | Capture.cs:235:21:235:25 | ... = ... |
| Consistency.cs:7:25:7:25 | b | Consistency.cs:7:25:7:25 | SSA param(b) | Consistency.cs:7:25:7:25 | b |
| Consistency.cs:15:17:15:17 | i | Consistency.cs:15:17:15:21 | SSA def(i) | Consistency.cs:15:17:15:21 | Int32 i = ... |
| Consistency.cs:15:17:15:17 | i | Consistency.cs:15:17:15:21 | SSA def(i) | Consistency.cs:15:17:15:21 | Int32 i = ... |
| Consistency.cs:15:17:15:17 | i | Consistency.cs:15:17:15:21 | [finally: exception(Exception)] SSA def(i) | Consistency.cs:15:17:15:21 | Int32 i = ... |
| Consistency.cs:25:29:25:29 | c | Consistency.cs:25:29:25:29 | SSA def(c) | Consistency.cs:25:29:25:29 | Consistency c |
| Consistency.cs:30:30:30:30 | c | Consistency.cs:32:9:32:29 | SSA def(c) | Consistency.cs:32:9:32:29 | ... = ... |
| Consistency.cs:38:13:38:13 | i | Consistency.cs:39:28:39:32 | SSA def(i) | Consistency.cs:39:28:39:32 | ... = ... |
@@ -209,7 +209,7 @@
| Test.cs:46:29:46:32 | out | Test.cs:50:13:50:20 | SSA def(out) | Test.cs:50:13:50:20 | ... = ... |
| Test.cs:46:29:46:32 | out | Test.cs:54:13:54:20 | SSA def(out) | Test.cs:54:13:54:20 | ... = ... |
| Test.cs:56:13:56:17 | this.field | Test.cs:57:9:57:17 | SSA def(this.field) | Test.cs:57:9:57:17 | ... = ... |
| Test.cs:68:45:68:45 | e | Test.cs:68:45:68:45 | SSA def(e) | Test.cs:68:45:68:45 | DivideByZeroException e |
| Test.cs:68:45:68:45 | e | Test.cs:68:45:68:45 | [exception: DivideByZeroException] SSA def(e) | Test.cs:68:45:68:45 | DivideByZeroException e |
| Tuples.cs:10:14:10:14 | x | Tuples.cs:10:9:10:54 | SSA def(x) | Tuples.cs:10:9:10:54 | ... = ... |
| Tuples.cs:10:14:10:14 | x | Tuples.cs:14:9:14:32 | SSA def(x) | Tuples.cs:14:9:14:32 | ... = ... |
| Tuples.cs:10:14:10:14 | x | Tuples.cs:23:9:23:37 | SSA def(x) | Tuples.cs:23:9:23:37 | ... = ... |

4
csharp/ql/test/library-tests/dataflow/ssa/SsaRead.expected
View File

@@ -65,7 +65,7 @@
| Capture.cs:229:13:229:13 | i | Capture.cs:236:9:236:12 | SSA call def(i) | Capture.cs:237:34:237:34 | access to local variable i |
| Consistency.cs:7:25:7:25 | b | Consistency.cs:7:25:7:25 | SSA param(b) | Consistency.cs:11:17:11:17 | access to parameter b |
| Consistency.cs:15:17:15:17 | i | Consistency.cs:15:17:15:21 | SSA def(i) | Consistency.cs:16:17:16:17 | access to local variable i |
| Consistency.cs:15:17:15:17 | i | Consistency.cs:15:17:15:21 | SSA def(i) | Consistency.cs:16:17:16:17 | access to local variable i |
| Consistency.cs:15:17:15:17 | i | Consistency.cs:15:17:15:21 | [finally: exception(Exception)] SSA def(i) | Consistency.cs:16:17:16:17 | access to local variable i |
| Consistency.cs:25:29:25:29 | c | Consistency.cs:25:29:25:29 | SSA def(c) | Consistency.cs:26:13:26:13 | access to local variable c |
| Consistency.cs:25:29:25:29 | c | Consistency.cs:25:29:25:29 | SSA def(c) | Consistency.cs:27:13:27:13 | access to local variable c |
| Consistency.cs:26:13:26:19 | c.Field | Consistency.cs:25:29:25:29 | SSA qualifier def(c.Field) | Consistency.cs:26:13:26:19 | access to field Field |
@@ -356,7 +356,7 @@
| Test.cs:46:16:46:18 | in | Test.cs:46:16:46:18 | SSA param(in) | Test.cs:48:13:48:15 | access to parameter in |
| Test.cs:56:13:56:17 | this.field | Test.cs:46:10:46:10 | SSA entry def(this.field) | Test.cs:56:13:56:17 | access to field field |
| Test.cs:56:13:56:17 | this.field | Test.cs:57:9:57:17 | SSA def(this.field) | Test.cs:58:13:58:17 | access to field field |
| Test.cs:68:45:68:45 | e | Test.cs:68:45:68:45 | SSA def(e) | Test.cs:70:17:70:17 | access to local variable e |
| Test.cs:68:45:68:45 | e | Test.cs:68:45:68:45 | [exception: DivideByZeroException] SSA def(e) | Test.cs:70:17:70:17 | access to local variable e |
| Tuples.cs:10:14:10:14 | x | Tuples.cs:10:9:10:54 | SSA def(x) | Tuples.cs:11:13:11:13 | access to local variable x |
| Tuples.cs:10:14:10:14 | x | Tuples.cs:14:9:14:32 | SSA def(x) | Tuples.cs:15:13:15:13 | access to local variable x |
| Tuples.cs:10:14:10:14 | x | Tuples.cs:23:9:23:37 | SSA def(x) | Tuples.cs:24:13:24:13 | access to local variable x |

4
csharp/ql/test/library-tests/dataflow/ssa/SsaUltimateDef.expected
View File

@@ -95,7 +95,7 @@
| Capture.cs:229:13:229:13 | i | Capture.cs:236:9:236:12 | SSA call def(i) | Capture.cs:236:9:236:12 | SSA call def(i) |
| Consistency.cs:7:25:7:25 | b | Consistency.cs:7:25:7:25 | SSA param(b) | Consistency.cs:7:25:7:25 | SSA param(b) |
| Consistency.cs:15:17:15:17 | i | Consistency.cs:15:17:15:21 | SSA def(i) | Consistency.cs:15:17:15:21 | SSA def(i) |
| Consistency.cs:15:17:15:17 | i | Consistency.cs:15:17:15:21 | SSA def(i) | Consistency.cs:15:17:15:21 | SSA def(i) |
| Consistency.cs:15:17:15:17 | i | Consistency.cs:15:17:15:21 | [finally: exception(Exception)] SSA def(i) | Consistency.cs:15:17:15:21 | [finally: exception(Exception)] SSA def(i) |
| Consistency.cs:25:29:25:29 | c | Consistency.cs:25:29:25:29 | SSA def(c) | Consistency.cs:25:29:25:29 | SSA def(c) |
| Consistency.cs:26:13:26:19 | c.Field | Consistency.cs:25:29:25:29 | SSA qualifier def(c.Field) | Consistency.cs:25:29:25:29 | SSA qualifier def(c.Field) |
| Consistency.cs:30:30:30:30 | c | Consistency.cs:32:9:32:29 | SSA def(c) | Consistency.cs:32:9:32:29 | SSA def(c) |
@@ -455,7 +455,7 @@
| Test.cs:46:29:46:32 | out | Test.cs:56:9:56:19 | SSA phi(out) | Test.cs:54:13:54:20 | SSA def(out) |
| Test.cs:56:13:56:17 | this.field | Test.cs:46:10:46:10 | SSA entry def(this.field) | Test.cs:46:10:46:10 | SSA entry def(this.field) |
| Test.cs:56:13:56:17 | this.field | Test.cs:57:9:57:17 | SSA def(this.field) | Test.cs:57:9:57:17 | SSA def(this.field) |
| Test.cs:68:45:68:45 | e | Test.cs:68:45:68:45 | SSA def(e) | Test.cs:68:45:68:45 | SSA def(e) |
| Test.cs:68:45:68:45 | e | Test.cs:68:45:68:45 | [exception: DivideByZeroException] SSA def(e) | Test.cs:68:45:68:45 | [exception: DivideByZeroException] SSA def(e) |
| Tuples.cs:10:14:10:14 | x | Tuples.cs:10:9:10:54 | SSA def(x) | Tuples.cs:10:9:10:54 | SSA def(x) |
| Tuples.cs:10:14:10:14 | x | Tuples.cs:14:9:14:32 | SSA def(x) | Tuples.cs:14:9:14:32 | SSA def(x) |
| Tuples.cs:10:14:10:14 | x | Tuples.cs:23:9:23:37 | SSA def(x) | Tuples.cs:23:9:23:37 | SSA def(x) |

1
javascript/config/suites/javascript/correctness-more
View File

@@ -15,3 +15,4 @@
+ semmlecode-javascript-queries/Statements/ReturnOutsideFunction.ql: /Correctness/Statements
+ semmlecode-javascript-queries/Statements/SuspiciousUnusedLoopIterationVariable.ql: /Correctness/Statements
+ semmlecode-javascript-queries/Statements/UselessConditional.ql: /Correctness/Statements
+ semmlecode-javascript-queries/Statements/UselessComparisonTest.ql: /Correctness/Statements

47
javascript/ql/src/Statements/UselessComparisonTest.qhelp Normal file
View File

@@ -0,0 +1,47 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>
If a condition always evaluates to true or always evaluates to false, this often indicates
incomplete code or a latent bug, and it should be examined carefully.
</p>
</overview>
<recommendation>
<p>
Examine the surrounding code to determine why the condition is redundant. If it is no
longer needed, remove it.
</p>
<p>
If the check is needed to guard against <code>NaN</code> values, insert a comment explaning the possibility of <code>NaN</code>.
</p>
</recommendation>
<example>
<p>
The following example finds the index of an element in a given slice of the array:
</p>
<sample src="examples/UselessComparisonTest.js" />
<p>
The condition <code>i &lt; end</code> at the end is always false, however. The code can be clarified if the
redundant condition is removed:
</p>
<sample src="examples/UselessComparisonTestGood.js" />
</example>
<references>
<li>Mozilla Developer Network: <a href="https://developer.mozilla.org/en-US/docs/Glossary/Truthy">Truthy</a>,
<a href="https://developer.mozilla.org/en-US/docs/Glossary/Falsy">Falsy</a>.</li>
</references>
</qhelp>

60
javascript/ql/src/Statements/UselessComparisonTest.ql Normal file
View File

@@ -0,0 +1,60 @@
/**
* @name Useless comparison test
* @description A comparison that always evaluates to true or always evaluates to false may
* indicate faulty logic and dead code.
* @kind problem
* @problem.severity warning
* @id js/useless-comparison-test
* @tags correctness
* @precision high
*/
import javascript
/**
* Holds if there are any contradictory guard nodes in `container`.
*
* We use this to restrict reachability analysis to a small set of containers.
*/
predicate hasContradictoryGuardNodes(StmtContainer container) {
exists (ConditionGuardNode guard |
RangeAnalysis::isContradictoryGuardNode(guard) and
container = guard.getContainer())
}
/**
* Holds if `block` is reachable and is in a container with contradictory guard nodes.
*/
predicate isReachable(BasicBlock block) {
exists (StmtContainer container |
hasContradictoryGuardNodes(container) and
block = container.getEntryBB())
or
isReachable(block.getAPredecessor()) and
not RangeAnalysis::isContradictoryGuardNode(block.getANode())
}
/**
* Holds if `block` is unreachable, but could be reached if `guard` was not contradictory.
*/
predicate isBlockedByContradictoryGuardNodes(BasicBlock block, ConditionGuardNode guard) {
RangeAnalysis::isContradictoryGuardNode(guard) and
isReachable(block.getAPredecessor()) and // the guard itself is reachable
block = guard.getBasicBlock()
or
isBlockedByContradictoryGuardNodes(block.getAPredecessor(), guard) and
not isReachable(block)
}
/**
* Holds if the given guard node is contradictory and causes an expression or statement to be unreachable.
*/
predicate isGuardNodeWithDeadCode(ConditionGuardNode guard) {
exists (BasicBlock block |
isBlockedByContradictoryGuardNodes(block, guard) and
block.getANode() instanceof ExprOrStmt)
}
from ConditionGuardNode guard
where isGuardNodeWithDeadCode(guard)
select guard.getTest(), "The condition '" + guard.getTest() + "' is always " + guard.getOutcome().booleanNot() + "."

12
javascript/ql/src/Statements/examples/UselessComparisonTest.js Normal file
View File

@@ -0,0 +1,12 @@
function findValue(values, x, start, end) {
let i;
for (i = start; i < end; ++i) {
if (values[i] === x) {
return i;
}
}
if (i < end) {
return i;
}
return -1;
}

8
javascript/ql/src/Statements/examples/UselessComparisonTestGood.js Normal file
View File

@@ -0,0 +1,8 @@
function findValue(values, x, start, end) {
for (let i = start; i < end; ++i) {
if (values[i] === x) {
return i;
}
}
return -1;
}

1
javascript/ql/src/javascript.qll
View File

@@ -36,6 +36,7 @@ import semmle.javascript.NPM
import semmle.javascript.Paths
import semmle.javascript.Promises
import semmle.javascript.CanonicalNames
import semmle.javascript.RangeAnalysis
import semmle.javascript.Regexp
import semmle.javascript.SSA
import semmle.javascript.StandardLibrary

631
javascript/ql/src/semmle/javascript/RangeAnalysis.qll Normal file
View File

@@ -0,0 +1,631 @@
import javascript
/*
* The range analysis is based on Difference Bound constraints, that is, inequalities of form:
*
* a - b <= c
*
* or equivalently,
*
* a <= b + c
*
* where a and b are variables in the constraint system, and c is an integer constant.
*
* Such constraints obey a transitive law. Given two constraints,
*
* a - x <= c1
* x - b <= c2
*
* adding the two inequalities yields the obvious transitive conclusion:
*
* a - b <= c1 + c2
*
* We view the system of constraints as a weighted graph, where `a - b <= c`
* corresponds to the edge `a -> b` with weight `c`.
*
* Paths in this graph corresponds to the additional inequalities we can derive from the constraint set.
* A negative-weight cycle represents a contradiction, such as `a <= a - 1`.
*
*
* CONTROL FLOW:
*
* Each constraint is associated with a CFG node where that constraint is known to be valid.
* The constraint is only valid within the dominator subtree of that node.
*
* The transitive rule additionally requires that, in order to compose two edges, one of
* their CFG nodes must dominate the other, and the resulting edge becomes associated with the
* dominated CFG node (i.e. the most restrictive scope). This ensures constraints
* cannot be taken out of context.
*
* If a negative-weight cycle can be constructed from the edges "in scope" at a given CFG node
* (i.e. associated with a dominator of the node), that node is unreachable.
*
*
* DUAL CONSTRAINTS:
*
* For every data flow node `a` we have two constraint variables, `+a` and `-a` (or just `a` and `-a`)
* representing the numerical value of `a` and its negation. Negations let us reason about the sum of
* two variables. For example:
*
* a + b <= 10 becomes a - (-b) <= 10
*
* It also lets us reason about the upper and lower bounds of a single variable:
*
* a <= 10 becomes a + a <= 20 becomes a - (-a) <= 20
* a >= 10 becomes -a <= -10 becomes (-a) - a <= -20
*
* For the graph analogy to include the relationship between `a` and `-a`, all constraints
* imply their dual constraint:
*
* a - b <= c implies (-b) - (-a) <= c
*
* That is, for every edge from a -> b, there is an edge with the same weight from (-b) -> (-a).
*
*
* PATH FINDING:
*
* See `extendedEdge` predicate for details about how we find negative-weight paths in the graph.
*
*
* CAVEATS:
*
* - We assume !(x <= y) means x > y, ignoring NaN, unless a nearby comment or identifier mentions NaN.
*
* - We assume integer arithmetic is exact, ignoring values above 2^53.
*
*/
/**
* Contains predicates for reasoning about the relative numeric value of expressions.
*/
module RangeAnalysis {
/**
* Holds if the given node is relevant for range analysis.
*/
private predicate isRelevant(DataFlow::Node node) {
node = any(Comparison cmp).getAnOperand().flow()
or
node = any(ConditionGuardNode guard).getTest().flow()
or
exists (DataFlow::Node succ | isRelevant(succ) |
succ = node.getASuccessor()
or
linearDefinitionStep(succ, node, _, _)
or
exists (BinaryExpr bin | bin instanceof AddExpr or bin instanceof SubExpr |
succ.asExpr() = bin and
bin.getAnOperand().flow() = node))
}
/**
* Holds if the given node has a unique data flow predecessor.
*/
pragma[noinline]
private predicate hasUniquePredecessor(DataFlow::Node node) {
isRelevant(node) and
strictcount(node.getAPredecessor()) = 1
}
/**
* Gets the definition of `node`, without unfolding phi nodes.
*/
DataFlow::Node getDefinition(DataFlow::Node node) {
if hasUniquePredecessor(node) then
result = getDefinition(node.getAPredecessor())
else
result = node
}
/**
* Gets a data flow node holding the result of the add/subtract operation in
* the given increment/decrement expression.
*/
private DataFlow::Node updateExprResult(UpdateExpr expr) {
exists (SsaExplicitDefinition def | def.getDef() = expr |
result = DataFlow::ssaDefinitionNode(def))
or
expr.isPrefix() and
result = expr.flow()
}
/**
* Gets a data flow node holding the result of the given componund assignment.
*/
private DataFlow::Node compoundAssignResult(CompoundAssignExpr expr) {
exists (SsaExplicitDefinition def | def.getDef() = expr |
result = DataFlow::ssaDefinitionNode(def))
or
result = expr.flow()
}
/**
* A 30-bit integer.
*
* Adding two such integers is guaranteed not to overflow. We simply omit constraints
* whose parameters would exceed this range.
*/
private class Bias extends int {
bindingset[this]
Bias() {
-536870912 < this and this < 536870912
}
}
/**
* Holds if `r` can be modelled as `r = root * sign + bias`.
*
* Only looks "one step", that is, does not follow data flow and does not recursively
* unfold nested arithmetic expressions.
*/
private predicate linearDefinitionStep(DataFlow::Node r, DataFlow::Node root, int sign, Bias bias) {
not exists(r.asExpr().getIntValue()) and
(
exists (AddExpr expr | r.asExpr() = expr |
// r = root + k
root = expr.getLeftOperand().flow() and
bias = expr.getRightOperand().getIntValue() and
sign = 1
or
// r = k + root
bias = expr.getLeftOperand().getIntValue() and
root = expr.getRightOperand().flow() and
sign = 1)
or
exists (SubExpr expr | r.asExpr() = expr |
// r = root - k
root = expr.getLeftOperand().flow() and
bias = -expr.getRightOperand().getIntValue() and
sign = 1
or
// r = k - root
bias = expr.getLeftOperand().getIntValue() and
root = expr.getRightOperand().flow() and
sign = -1)
or
exists (NegExpr expr | r.asExpr() = expr |
// r = -root
root = expr.getOperand().flow() and
bias = 0 and
sign = -1)
or
exists (UpdateExpr update | r = updateExprResult(update) |
// r = ++root
root = update.getOperand().flow() and
sign = 1 and
if update instanceof IncExpr then
bias = 1
else
bias = -1)
or
exists (UpdateExpr update | r.asExpr() = update | // Return value of x++ is just x (coerced to an int)
// r = root++
root = update.getOperand().flow() and
not update.isPrefix() and
sign = 1 and
bias = 0)
or
exists (CompoundAssignExpr assign | r = compoundAssignResult(assign) |
root = assign.getLhs().flow() and
sign = 1 and
(
// r = root += k
assign instanceof AssignAddExpr and
bias = assign.getRhs().getIntValue()
or
// r = root -= k
assign instanceof AssignSubExpr and
bias = -assign.getRhs().getIntValue()
))
)
}
/**
* Holds if `r` can be modelled as `r = root * sign + bias`.
*/
predicate linearDefinition(DataFlow::Node r, DataFlow::Node root, int sign, Bias bias) {
if hasUniquePredecessor(r) then
linearDefinition(r.getAPredecessor(), root, sign, bias)
else if linearDefinitionStep(r, _, _, _) then
exists (DataFlow::Node pred, int sign1, int bias1, int sign2, int bias2 |
// r = pred * sign1 + bias1
linearDefinitionStep(r, pred, sign1, bias1) and
// pred = root * sign2 + bias2
linearDefinition(pred, root, sign2, bias2) and
// r = (root * sign2 + bias2) * sign1 + bias1
sign = sign1 * sign2 and
bias = bias1 + sign1 * bias2)
else (
isRelevant(r) and
root = r and
sign = 1 and
bias = 0
)
}
/**
* Holds if `r` can be modelled as `r = xroot * xsign + yroot * ysign + bias`.
*/
predicate linearDefinitionSum(DataFlow::Node r, DataFlow::Node xroot, int xsign, DataFlow::Node yroot, int ysign, Bias bias) {
if hasUniquePredecessor(r) then
linearDefinitionSum(r.getAPredecessor(), xroot, xsign, yroot, ysign, bias)
else if exists(r.asExpr().getIntValue()) then
none() // do not model constants as sums
else (
exists (AddExpr add, int bias1, int bias2 | r.asExpr() = add |
// r = r1 + r2
linearDefinition(add.getLeftOperand().flow(), xroot, xsign, bias1) and
linearDefinition(add.getRightOperand().flow(), yroot, ysign, bias2) and
bias = bias1 + bias2)
or
exists (SubExpr sub, int bias1, int bias2 | r.asExpr() = sub |
// r = r1 - r2
linearDefinition(sub.getLeftOperand().flow(), xroot, xsign, bias1) and
linearDefinition(sub.getRightOperand().flow(), yroot, -ysign, -bias2) and // Negate right-hand operand
bias = bias1 + bias2)
or
linearDefinitionSum(r.asExpr().(NegExpr).getOperand().flow(), xroot, -xsign, yroot, -ysign, -bias)
)
}
/**
* Holds if the given comparison can be modelled as `A <op> B + bias` where `<op>` is the comparison operator,
* and `A` is `a * asign` and likewise `B` is `b * bsign`.
*/
predicate linearComparison(Comparison comparison, DataFlow::Node a, int asign, DataFlow::Node b, int bsign, Bias bias) {
exists(Expr left, Expr right, int bias1, int bias2 | left = comparison.getLeftOperand() and right = comparison.getRightOperand() |
// A <= B + c
linearDefinition(left.flow(), a, asign, bias1) and
linearDefinition(right.flow(), b, bsign, bias2) and
bias = bias2 - bias1
or
// A - B + c1 <= c2 becomes A <= B + (c2 - c1)
linearDefinitionSum(left.flow(), a, asign, b, -bsign, bias1) and
right.getIntValue() = bias2 and
bias = bias2 - bias1
or
// c1 <= -A + B + c2 becomes A <= B + (c2 - c1)
left.getIntValue() = bias1 and
linearDefinitionSum(right.flow(), a, -asign, b, bsign, bias2) and
bias = bias2 - bias1
)
}
/**
* Holds if the given container has a comment or identifier mentioning `NaN`.
*/
predicate hasNaNIndicator(StmtContainer container) {
exists (Comment comment |
comment.getText().regexpMatch("(?s).*N[aA]N.*") and
comment.getFile() = container.getFile() and
(
comment.getLocation().getStartLine() >= container.getLocation().getStartLine() and
comment.getLocation().getEndLine() <= container.getLocation().getEndLine()
or
comment.getNextToken() = container.getFirstToken()
))
or
exists (Identifier id | id.getName() = "NaN" or id.getName() = "isNaN" |
id.getContainer() = container)
}
/**
* Holds if `guard` asserts that the outcome of `A <op> B + bias` is true, where `<op>` is a comparison operator.
*/
predicate linearComparisonGuard(ConditionGuardNode guard, DataFlow::Node a, int asign, string operator, DataFlow::Node b, int bsign, Bias bias) {
exists (Comparison compare | compare = getDefinition(guard.getTest().flow()).asExpr() |
linearComparison(compare, a, asign, b, bsign, bias) and
(
guard.getOutcome() = true and operator = compare.getOperator()
or
not hasNaNIndicator(guard.getContainer()) and
guard.getOutcome() = false and operator = negateOperator(compare.getOperator())
)
)
}
/**
* Gets the binary operator whose return value is the opposite of `operator` (excluding NaN comparisons).
*/
private string negateOperator(string operator) {
operator = "==" and result = "!=" or
operator = "===" and result = "!==" or
operator = "<" and result = ">=" or
operator = ">" and result = "<=" or
operator = negateOperator(result)
}
/**
* Holds if immediately after `cfg` it becomes known that `A <= B + c`.
*
* These are the initial inputs to the difference bound constraint system.
*
* The dual constraint `-B <= -A + c` is not included in this predicate.
*/
predicate comparisonEdge(ControlFlowNode cfg, DataFlow::Node a, int asign, DataFlow::Node b, int bsign, Bias bias, boolean sharp) {
// A <= B + c
linearComparisonGuard(cfg, a, asign, "<=", b, bsign, bias) and
sharp = false
or
// A < B + c
linearComparisonGuard(cfg, a, asign, "<", b, bsign, bias) and
sharp = true
or
// A <= B + c iff B >= A - c
linearComparisonGuard(cfg, b, bsign, ">=", a, asign, -bias) and
sharp = false
or
// A < B + c iff B > A - c
linearComparisonGuard(cfg, b, bsign, ">", a, asign, -bias) and
sharp = true
or
sharp = false and
exists (string operator | operator = "==" or operator = "===" |
// A == B + c iff A <= B + c and B <= A - c
linearComparisonGuard(cfg, a, asign, operator, b, bsign, bias)
or
linearComparisonGuard(cfg, b, bsign, operator, a, asign, -bias)
)
}
/**
* Holds if `node` is a phi node with `left` and `right` has the only two inputs.
*
* Note that this predicate is symmetric: when it holds for (left, right) it also holds for (right, left).
*/
predicate binaryPhiNode(DataFlow::Node node, DataFlow::Node left, DataFlow::Node right) {
exists (SsaPhiNode phi | node = DataFlow::ssaDefinitionNode(phi) |
isRelevant(node) and
strictcount(phi.getAnInput()) = 2 and
left = DataFlow::ssaDefinitionNode(phi.getAnInput()) and
right = DataFlow::ssaDefinitionNode(phi.getAnInput()) and
left != right)
}
/**
* Holds if `A <= B + c` can be determined based on a phi node.
*/
predicate phiEdge(ControlFlowNode cfg, DataFlow::Node a, int asign, DataFlow::Node b, int bsign, Bias c) {
exists (DataFlow::Node phi, DataFlow::Node left, DataFlow::Node right |
binaryPhiNode(phi, left, right) and
cfg = phi.getBasicBlock()
|
// Both inputs are defined in terms of the same root:
// phi = PHI(root + bias1, root + bias2)
exists (DataFlow::Node root, int sign, Bias bias1, Bias bias2 |
linearDefinition(left, root, sign, bias1) and
linearDefinition(right, root, sign, bias2) and
bias1 < bias2 and
// root + bias1 <= phi <= root + bias2
(
// root <= phi - bias1
a = root and asign = 1 and
b = phi and bsign = 1 and
c = -bias1
or
// phi <= root + bias2
a = phi and asign = 1 and
b = root and bsign = 1 and
c = bias2
)
)
or
// One input is defined in terms of the phi node itself:
// phi = PHI(phi + increment, x)
exists (int increment, DataFlow::Node root, int sign, Bias bias |
linearDefinition(left, phi, 1, increment) and
linearDefinition(right, root, sign, bias) and
(
// If increment is positive (or zero):
// phi >= right' + bias
increment >= 0 and
a = root and asign = sign and
b = phi and bsign = 1 and
c = -bias
or
// If increment is negative (or zero):
// phi <= right' + bias
increment <= 0 and
a = phi and asign = 1 and
b = root and bsign = sign and
c = bias
)
)
)
}
/**
* Holds if a comparison implies that `A <= B + c`.
*
* Comparisons where one operand is really a constant are converted into a unary constraint.
*/
predicate foldedComparisonEdge(ControlFlowNode cfg, DataFlow::Node a, int asign, DataFlow::Node b, int bsign, Bias c, boolean sharp) {
// A <= B + c (where A and B are not constants)
comparisonEdge(cfg, a, asign, b, bsign, c, sharp) and
not exists(a.asExpr().getIntValue()) and
not exists(b.asExpr().getIntValue())
or
// A - k <= c becomes A - (-A) <= 2*(k + c)
exists (DataFlow::Node k, int ksign, Bias kbias, Bias value |
comparisonEdge(cfg, a, asign, k, ksign, kbias, sharp) and
value = k.asExpr().getIntValue() and
b = a and
bsign = -asign and
c = 2 * (value * ksign + kbias))
or
// k - A <= c becomes -A - A <= 2(-k + c)
exists (DataFlow::Node k, int ksign, Bias kbias, Bias value |
comparisonEdge(cfg, k, ksign, b, bsign, kbias, sharp) and
value = k.asExpr().getIntValue() and
a = b and
asign = -bsign and
c = 2 * (-value * ksign + kbias))
or
// For completeness, generate a contradictory constraint for trivially false conditions.
exists (DataFlow::Node k, int ksign, Bias bias, int avalue, int kvalue |
comparisonEdge(cfg, a, asign, k, ksign, bias, sharp) and
avalue = a.asExpr().getIntValue() * asign and
kvalue = k.asExpr().getIntValue() * ksign and
(avalue > kvalue + bias or sharp = true and avalue = kvalue + bias) and
a = b and
asign = bsign and
c = -1)
}
/**
* The set of initial edges including those from dual constraints.
*/
predicate seedEdge(ControlFlowNode cfg, DataFlow::Node a, int asign, DataFlow::Node b, int bsign, Bias c, boolean sharp) {
foldedComparisonEdge(cfg, a, asign, b, bsign, c, sharp)
or
phiEdge(cfg, a, asign, b, bsign, c) and sharp = false
}
private predicate seedEdgeWithDual(ControlFlowNode cfg, DataFlow::Node a, int asign, DataFlow::Node b, int bsign, Bias c, boolean sharp) {
// A <= B + c
seedEdge(cfg, a, asign, b, bsign, c, sharp)
or
// -B <= -A + c (dual constraint)
seedEdge(cfg, b, -bsign, a, -asign, c, sharp)
}
/**
* Adds a negative and positive integer, but only if they are within in the same
* order of magnitude.
*/
bindingset[x, sharpx, y, sharpy]
private int wideningAddition(int x, boolean sharpx, int y, boolean sharpy) {
(x < 0 or x = 0 and sharpx = true) and
(y > 0 or y = 0 and sharpy = false) and
(
x <= 0 and x >= 0
or
y <= 0 and y >= 0
or
// If non-zero, check that the values are within a factor 16 of each other
x.abs().bitShiftRight(4) < y.abs() and
y.abs().bitShiftRight(4) < x.abs()
) and
result = x + y
}
/**
* Applies a restricted transitive rule to the edge set.
*
* In particular, we apply the transitive rule only where a negative edge followed by a non-negative edge.
* For example:
*
* A --(-1)--> B --(+3)--> C
*
* yields:
*
* A --(+2)--> C
*
* In practice, the restriction to edges of different sign prevent the
* quadratic blow-up you would normally get from a transitive closure.
*
* It also prevents the relation from becoming infinite in case
* there are negative-weight cycles, where the transitive weights would
* otherwise diverge towards minus infinity.
*
* Moreover, the rule is enough to guarantee the following property:
*
* A negative-weight path from X to Y exists iff a path of negative-weight edges exists from X to Y.
*
* This means negative-weight cycles (contradictions) can be detected using simple cycle detection.
*/
pragma[noopt]
private predicate extendedEdge(DataFlow::Node a, int asign, DataFlow::Node b, int bsign, Bias c, boolean sharp, ControlFlowNode cfg) {
seedEdgeWithDual(cfg, a, asign, b, bsign, c, sharp)
or
// One of the two CFG nodes must dominate the other, and `cfg` must be bound to the dominated one.
exists (ControlFlowNode cfg1, ControlFlowNode cfg2 |
// They are in the same basic block
extendedEdgeCandidate(a, asign, b, bsign, c, sharp, cfg1, cfg2) and
exists (BasicBlock bb, int i, int j |
bb.getNode(i) = cfg1 and
bb.getNode(j) = cfg2 and
if i < j then
cfg = cfg2
else
cfg = cfg1)
or
// They are in different basic blocks
extendedEdgeCandidate(a, asign, b, bsign, c, sharp, cfg1, cfg2) and
exists (BasicBlock cfg1BB, ReachableBasicBlock cfg1RBB, BasicBlock cfg2BB, ReachableBasicBlock cfg2RBB |
cfg1BB = cfg1.getBasicBlock() and
cfg1RBB = cfg1BB.(ReachableBasicBlock) and
cfg2BB = cfg2.getBasicBlock() and
cfg2RBB = cfg2BB.(ReachableBasicBlock) and
(
cfg1RBB.strictlyDominates(cfg2BB) and
cfg = cfg2
or
cfg2RBB.strictlyDominates(cfg1RBB) and
cfg = cfg1
))
) and
cfg instanceof ControlFlowNode
}
/**
* Holds if an extended edge from `A` to `B` can potentially be generates from two edges, from `cfg1` and `cfg2`, respectively.
*
* This does not check for dominance between `cfg1` and `cfg2`.
*/
pragma[nomagic]
private predicate extendedEdgeCandidate(DataFlow::Node a, int asign, DataFlow::Node b, int bsign, Bias c, boolean sharp, ControlFlowNode cfg1, ControlFlowNode cfg2) {
exists (DataFlow::Node mid, int midx, Bias c1, Bias c2, boolean sharp1, boolean sharp2 |
extendedEdge(a, asign, mid, midx, c1, sharp1, cfg1) and
extendedEdge(mid, midx, b, bsign, c2, sharp2, cfg2) and
(a != mid or asign != midx) and
(b != mid or bsign != midx) and
sharp = sharp1.booleanOr(sharp2) and
c = wideningAddition(c1, sharp1, c2, sharp2))
}
/**
* Holds if there is a negative-weight edge from src to dst.
*/
private predicate negativeEdge(DataFlow::Node a, int asign, DataFlow::Node b, int bsign, ControlFlowNode cfg) {
exists (int weight, boolean sharp | extendedEdge(a, asign, b, bsign, weight, sharp, cfg) |
weight = 0 and sharp = true // a strict "< 0" edge counts as negative
or
weight < 0)
}
/**
* Holds if `src` can reach `dst` using only negative-weight edges.
*
* The initial outgoing edge from `src` must be derived at `cfg`.
*/
pragma[noopt]
private predicate reachableByNegativeEdges(DataFlow::Node a, int asign, DataFlow::Node b, int bsign, ControlFlowNode cfg) {
negativeEdge(a, asign, b, bsign, cfg)
or
exists(DataFlow::Node mid, int midx, ControlFlowNode midcfg |
reachableByNegativeEdges(a, asign, mid, midx, cfg) and
negativeEdge(mid, midx, b, bsign, midcfg) and
exists (BasicBlock bb, int i, int j |
bb.getNode(i) = midcfg and
bb.getNode(j) = cfg and
i <= j))
or
// Same as above, but where CFG nodes are in different basic blocks
exists(DataFlow::Node mid, int midx, ControlFlowNode midcfg, BasicBlock midBB, ReachableBasicBlock midRBB, BasicBlock cfgBB |
reachableByNegativeEdges(a, asign, mid, midx, cfg) and
negativeEdge(mid, midx, b, bsign, midcfg) and
midBB = midcfg.getBasicBlock() and
midRBB = midBB.(ReachableBasicBlock) and
cfgBB = cfg.getBasicBlock() and
midRBB.strictlyDominates(cfgBB)
)
}
/**
* Holds if the condition asserted at `guard` is contradictory, that is, its condition always has the
* opposite of the expected outcome.
*/
predicate isContradictoryGuardNode(ConditionGuardNode guard) {
exists (DataFlow::Node a, int asign | reachableByNegativeEdges(a, asign, a, asign, guard))
}
}

10
javascript/ql/src/semmle/javascript/frameworks/Express.qll
View File

@@ -127,10 +127,12 @@ module Express {
Expr getRouteHandlerExpr(int index) {
// The first argument is a URI pattern if it is a string. If it could possibly be
// a function, we consider it to be a route handler, otherwise a URI pattern.
if getArgument(0).analyze().getAType() = TTFunction() then
result = getArgument(index)
else
(index >= 0 and result = getArgument(index + 1))
exists (AnalyzedNode firstArg | firstArg = getArgument(0).analyze() |
if firstArg.getAType() = TTFunction() then
result = getArgument(index)
else
(index >= 0 and result = getArgument(index + 1))
)
}
/** Gets an argument that represents a route handler being registered. */

0
javascript/ql/test/library-tests/RangeAnalysis/DeadBranch.expected Normal file
View File

35
javascript/ql/test/library-tests/RangeAnalysis/DeadBranch.ql Normal file
View File

@@ -0,0 +1,35 @@
import javascript
class AssertionComment extends LineComment {
boolean isOK;
AssertionComment() {
isOK = true and getText().trim().regexpMatch("OK.*")
or
isOK = false and getText().trim().regexpMatch("NOT OK.*")
}
ConditionGuardNode getAGuardNode() {
result.getLocation().getStartLine() = this.getLocation().getStartLine() and
result.getFile() = this.getFile()
}
Expr getTestExpr() { result = getAGuardNode().getTest() }
string getMessage() {
not exists(getAGuardNode()) and result = "Error: no guard node on this line"
or
isOK = true and
exists (ConditionGuardNode guard | guard = getAGuardNode() |
RangeAnalysis::isContradictoryGuardNode(guard) and
result = "Error: analysis claims " + getTestExpr() + " is always " + guard.getOutcome().booleanNot()
)
or
isOK = false and
not RangeAnalysis::isContradictoryGuardNode(getAGuardNode()) and
result = "Error: " + getTestExpr() + " is always true or always false"
}
}
from AssertionComment assertion
select assertion, assertion.getMessage()

8
javascript/ql/test/library-tests/RangeAnalysis/constants.js Normal file
View File

@@ -0,0 +1,8 @@
function f() {
if (1 > 0) {} // NOT OK - always true
if (1 - 1 >= 0) {} // NOT OK - always true
let one = 1;
let two = 2;
if (two > one) {} // NOT OK - always true
if (two <= one) {} // NOT OK - always false
}

9
javascript/ql/test/library-tests/RangeAnalysis/expressions.js Normal file
View File

@@ -0,0 +1,9 @@
function f(arr) {
if (arr.length > 2) {} // OK
let x = arr.length || 0;
if (x > 2) {} // OK
let y = arr.length && 3;
if (y > 2) {} // OK
}

3
javascript/ql/test/library-tests/RangeAnalysis/length.js Normal file
View File

@@ -0,0 +1,3 @@
function f(arr) {
if (arr.length > 2) {} // OK
}

40
javascript/ql/test/library-tests/RangeAnalysis/loop.js Normal file
View File

@@ -0,0 +1,40 @@
function increasing(start, end) {
for (var i = start; i < end; ++i) {
if (i >= start) {} // NOT OK - always true
if (i < start) {} // NOT OK - always false
if (i < end) {} // NOT OK - always true
if (i >= end) {} // NOT OK - always false
if (i + 1 > start) {} // NOT OK - always true
if (i - 1 < start) {} // OK
}
}
function decreasing(start, end) {
for (var i = start; i > end; --i) {
if (i <= start) {} // NOT OK - always true
if (i > start) {} // NOT OK - always false
if (i + 1 > start) {} // OK
if (i - 1 < start) {} // NOT OK - always true
}
}
function middleIncrement(start, end) {
for (var i = start; i < end;) {
if (i >= start) {} // OK - always true but we don't catch it
if (i < start) {} // OK - always false but we don't catch it
if (i < end) {} // NOT OK - always true
if (i >= end) {} // NOT OK - always false
if (something()) {
i += 2;
}
if (i >= start) {} // OK - always true but we don't catch it
if (i < start) {} // OK - always false but we don't catch it
if (i < end) {} // OK
if (i >= end) {} // OK
}
}

33
javascript/ql/test/library-tests/RangeAnalysis/plus.js Normal file
View File

@@ -0,0 +1,33 @@
function f(x,y) {
if (x + 1 < y) {
if (x < y) {} // NOT OK - always true
if (x + 1 < y) {} // NOT OK - always true
if (x + 1 <= y) {} // NOT OK - always true
if (x > y) {} // NOT OK - always false
if (x >= y) {} // NOT OK - always false
if (x + 1 >= y) {} // NOT OK - always false
if (x + 2 < y) {} // OK
if (x < y - 1) {} // NOT OK - always true
}
if (x < y + 1) {
if (x < y) {} // OK
if (x + 1 < y) {} // OK
if (x - 1 < y) {} // NOT OK - always true
}
}
function g(x) {
let z = x + 1;
z += 1;
++z;
z++;
if (z < x + 4) {} // NOT OK - always false
if (z > x + 4) {} // NOT OK - always false
if (z < x + 5) {} // NOT OK - always true
if (z > x + 5) {} // NOT OK - always false
}
function h(x) {
let y = x++;
if (x > y) {} // NOT OK - always true
}

64
javascript/ql/test/library-tests/RangeAnalysis/tst.js Normal file
View File

@@ -0,0 +1,64 @@
function f(x,y) {
if (x < y) {
if (x < y) {} // NOT OK - always true
if (x <= y) {} // NOT OK - always true
if (x > y) {} // NOT OK - always false
if (x >= y) {} // NOT OK - always false
if (x === y) {} // NOT OK - always false
} else {
if (x < y) {} // NOT OK - always false
if (x <= y) {} // OK - could be equal
if (x > y) {} // OK
if (x >= y) {} // NOT OK - always true
if (x === y) {} // OK
}
}
function g(x,y) {
if (x <= y) {
if (x < y) {} // OK
if (x <= y) {} // NOT OK - always true
if (x > y) {} // NOT OK - always false
if (x >= y) {} // OK - could be equal
if (x === y) {} // OK
} else {
if (x < y) {} // NOT OK - always false
if (x <= y) {} // NOT OK - always false
if (x > y) {} // NOT OK - always true
if (x >= y) {} // NOT OK - always true
if (x === y) {} // NOT OK - always false
}
}
function loop(start, end) {
var i;
for (i = start; i < end; i++) {
if (i < end) {} // NOT OK - always true
if (i >= end) {} // NOT OK - always false
}
if (i >= end) {} // NOT OK - always true
if (i < end) {} // NOT OK - always false
}
function h(x, y) {
if (x - y < 0) {
if (x < y) {} // NOT OK - always true
if (x >= y) {} // NOT OK - always false
}
if (0 < x - y) { // y < x
if (x <= y) {} // NOT OK - always false
if (x > y) {} // NOT OK - always true
}
}
function nan(x) {
// This is a NaN comment.
if (x - 1 < x) {} // OK
}
/**
* This is a NAN comment.
*/
function nan2(x) {
if (x - 1 < x) {} // OK
}

3
javascript/ql/test/query-tests/Statements/UselessComparisonTest/UselessComparisonTest.expected Normal file
View File

@@ -0,0 +1,3 @@
| constant.js:2:7:2:11 | 1 > 2 | The condition '1 > 2' is always false. |
| constant.js:3:7:3:11 | 1 > 0 | The condition '1 > 0' is always true. |
| example.js:8:7:8:13 | i < end | The condition 'i < end' is always false. |

1
javascript/ql/test/query-tests/Statements/UselessComparisonTest/UselessComparisonTest.qlref Normal file
View File

@@ -0,0 +1 @@
Statements/UselessComparisonTest.ql

4
javascript/ql/test/query-tests/Statements/UselessComparisonTest/constant.js Normal file
View File

@@ -0,0 +1,4 @@
function f() {
if (1 > 2) {} else {} // NOT OK - always false
if (1 > 0) {} else {} // NOT OK - always true
}

12
javascript/ql/test/query-tests/Statements/UselessComparisonTest/example.js Normal file
View File

@@ -0,0 +1,12 @@
function findValue(values, x, start, end) {
let i;
for (i = start; i < end; ++i) {
if (values[i] === x) {
return i;
}
}
if (i < end) {
return i;
}
return -1;
}

8
javascript/ql/test/query-tests/Statements/UselessComparisonTest/exampleGood.js Normal file
View File

@@ -0,0 +1,8 @@
function findValue(values, x, start, end) {
for (let i = start; i < end; ++i) {
if (values[i] === x) {
return i;
}
}
return -1;
}