hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-20 22:44:52 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge branch 'master' into rdmarsh/cpp/memset-decltype-changenote

Browse Source
This commit is contained in:
Jonas Jensen
2018-10-30 14:05:26 +01:00
committed by GitHub
parent 7c2bcf6fa6 336f605153
commit 784b2749e6
1353 changed files with 66259 additions and 27561 deletions
Download Patch File Download Diff File Expand all files Collapse all files

17
.lgtm.yml Executable file
View File

@@ -0,0 +1,17 @@
path_classifiers:
library:
- javascript/externs
test:
- csharp/ql/src
- csharp/ql/test
- javascript/ql/src
- javascript/ql/test
queries:
- include: "*"
extraction:
python:
python_setup:
version: 3

1
CODEOWNERS
View File

@@ -1,3 +1,4 @@
/csharp/ @Semmle/cs
/java/ @Semmle/java
/javascript/ @Semmle/js
/cpp/ @Semmle/cpp-analysis

14
change-notes/1.19/analysis-cpp.md
View File

@@ -6,15 +6,25 @@
| **Query** | **Tags** | **Purpose** |
|-----------------------------|-----------|--------------------------------------------------------------------|
| *@name of query (Query ID)* | *Tags* |*Aim of the new query and whether it is enabled by default or not* |
| Cast between `HRESULT` and a Boolean type (`cpp/hresult-boolean-conversion`) | external/cwe/cwe-253 | Finds logic errors caused by mistakenly treating the Windows `HRESULT` type as a Boolean instead of testing it with the appropriate macros. Enabled by default. |
| Setting a DACL to `NULL` in a `SECURITY_DESCRIPTOR` (`cpp/unsafe-dacl-security-descriptor`) | external/cwe/cwe-732 | This query finds code that creates world-writable objects on Windows by setting their DACL to `NULL`. Enabled by default. |
| Cast from `char*` to `wchar_t*` | security, external/cwe/cwe-704 | Detects potentially dangerous casts from `char*` to `wchar_t*`. Enabled by default on LGTM. |
| Dead code due to `goto` or `break` statement (`cpp/dead-code-goto`) | maintainability, external/cwe/cwe-561 | Detects dead code following a goto or break statement. Enabled by default on LGTM. |
## Changes to existing queries
| **Query** | **Expected impact** | **Change** |
|----------------------------|------------------------|------------------------------------------------------------------|
| Resource not released in destructor | Fewer false positive results | Placement new is now excluded from the query. |
| Empty branch of conditional | Fewer false positive results | The query now recognizes commented blocks more reliably. |
| Resource not released in destructor | Fewer false positive results | Placement new is now excluded from the query. Also fixed an issue where false positives could occur if the destructor body was not in the snapshot. |
| Missing return statement (`cpp/missing-return`) | Visible by default | The precision of this query has been increased from 'medium' to 'high', which makes it visible by default in LGTM. It was 'medium' in release 1.17 and 1.18 because it had false positives due to an extractor bug that was fixed in 1.18. |
| Missing return statement | Fewer false positive results | The query is now produces correct results when a function returns a template-dependent type. |
| Call to memory access function may overflow buffer | More correct results | Array indexing with a negative index is now detected by this query. |
| Suspicious call to memset | Fewer false positive results | Types involving decltype are now correctly compared. |
| Suspicious add with sizeof | Fewer false positive results | Arithmetic with void pointers (where allowed) is now excluded from this query. |
| Wrong type of arguments to formatting function | Fewer false positive results | False positive results involving typedefs have been removed. Expected argument types are determined more accurately, especially for wide string and pointer types. Custom (non-standard) formatting functions are also identified more accurately. |
## Changes to QL libraries
* Added a hash consing library for structural comparison of expressions.
* `getBufferSize` now detects variable size structs more reliably.

22
change-notes/1.19/analysis-csharp.md Normal file
View File

@@ -0,0 +1,22 @@
# Improvements to C# analysis
## General improvements
* Control flow graph improvements:
* The control flow graph construction now takes simple Boolean conditions on local scope variables into account. For example, in `if (b) x = 0; if (b) x = 1;`, the control flow graph will reflect that taking the `true` (resp. `false`) branch in the first condition implies taking the same branch in the second condition. In effect, the first assignment to `x` will now be identified as being dead.
* Code that is only reachable from a constant failing assertion, such as `Debug.Assert(false)`, is considered to be unreachable.
## New queries
| **Query** | **Tags** | **Purpose** |
|-----------------------------|-----------|--------------------------------------------------------------------|
| *@name of query (Query ID)* | *Tags* |*Aim of the new query and whether it is enabled by default or not* |
## Changes to existing queries
| Inconsistent lock sequence (`cs/inconsistent-lock-sequence`) | More results | This query now finds inconsistent lock sequences globally across calls. |
| Local scope variable shadows member (`cs/local-shadows-member`) | Fewer results | Results have been removed where a constructor parameter shadows a member, because the parameter is probably used to initialize the member. |
| *@name of query (Query ID)*| *Impact on results* | *How/why the query has changed* |
## Changes to QL libraries

28
change-notes/1.19/analysis-java.md Normal file
View File

@@ -0,0 +1,28 @@
# Improvements to Java analysis
## General improvements
## New queries
| **Query** | **Tags** | **Purpose** |
|-----------------------------|-----------|--------------------------------------------------------------------|
| Missing catch of NumberFormatException (`java/uncaught-number-format-exception`) | reliability, external/cwe/cwe-248 | Finds calls to `Integer.parseInt` and similar string-to-number conversions that might raise a `NumberFormatException` without a corresponding `catch`-clause. |
## Changes to existing queries
| **Query** | **Expected impact** | **Change** |
|----------------------------|------------------------|------------------------------------------------------------------|
| Array index out of bounds (`java/index-out-of-bounds`) | Fewer false positive results | False positives involving arrays with a length evenly divisible by 3 or some greater number and an index being increased with a similar stride length are no longer reported. |
| Query built from user-controlled sources (`java/sql-injection`) | More results | Sql injection sinks from the Spring JDBC, MyBatis, and Hibernate frameworks are now reported. |
| Query built without neutralizing special characters (`java/concatenated-sql-query`) | More results | Sql injection sinks from the Spring JDBC, MyBatis, and Hibernate frameworks are now reported. |
| Unreachable catch clause (`java/unreachable-catch-clause`) | Fewer false positive results | This rule now accounts for calls to generic methods that throw generic exceptions. |
| Useless comparison test (`java/constant-comparison`) | Fewer false positive results | Constant comparisons guarding `java.util.ConcurrentModificationException` are no longer reported, as they are intended to always be false in the absence of API misuse. |
## Changes to QL libraries
* The default set of taint sources in the `FlowSources` library is extended to
cover parameters annotated with Spring framework annotations indicating
remote user input from servlets. This affects all security queries, which
will yield additional results on projects using the Spring Web framework.
* The `ParityAnalysis` library is replaced with the more general `ModulusAnalysis` library, which improves the range analysis.

20
change-notes/1.19/analysis-javascript.md
View File

@@ -8,16 +8,22 @@
* Support for popular libraries has been improved. Consequently, queries may produce more results on code bases that use the following features:
- file system access, for example through [fs-extra](https://github.com/jprichardson/node-fs-extra) or [globby](https://www.npmjs.com/package/globby)
- outbound network access, for example through the [fetch API](https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API)
- the [Google Cloud Spanner](https://cloud.google.com/spanner) database
* The type inference now handles nested imports (that is, imports not appearing at the toplevel). This may yield fewer false-positive results on projects that use this non-standard language feature.
## New queries
| **Query** | **Tags** | **Purpose** |
|-----------------------------------------------|------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| Enabling Node.js integration for Electron web content renderers (`js/enabling-electron-renderer-node-integration`) | security, frameworks/electron, external/cwe/cwe-094 | Highlights Electron web content renderer preferences with Node.js integration enabled, indicating a violation of [CWE-94](https://cwe.mitre.org/data/definitions/94.html). Results are not shown on LGTM by default. |
| Stored cross-site scripting (`js/stored-xss`) | security, external/cwe/cwe-079, external/cwe/cwe-116 | Highlights uncontrolled stored values flowing into HTML content, indicating a violation of [CWE-079](https://cwe.mitre.org/data/definitions/79.html). Results shown on LGTM by default. |
| Replacement of a substring with itself (`js/identity-replacement`) | correctness, security, external/cwe/cwe-116 | Highlights string replacements that replace a string with itself, which usually indicates a mistake. Results shown on LGTM by default. |
| File data in outbound network request | security, external/cwe/cwe-200 | Highlights locations where file data is sent in a network request. Results are not shown on LGTM by default. |
| Host header poisoning in email generation | security, external/cwe/cwe-640 | Highlights code that generates emails with links that can be hijacked by HTTP host header poisoning, indicating a violation of [CWE-640](https://cwe.mitre.org/data/definitions/640.html). Results shown on LGTM by default. |
| Replacement of a substring with itself (`js/identity-replacement`) | correctness, security, external/cwe/cwe-116 | Highlights string replacements that replace a string with itself, which usually indicates a mistake. Results shown on LGTM by default. |
| Stored cross-site scripting (`js/stored-xss`) | security, external/cwe/cwe-079, external/cwe/cwe-116 | Highlights uncontrolled stored values flowing into HTML content, indicating a violation of [CWE-079](https://cwe.mitre.org/data/definitions/79.html). Results shown on LGTM by default. |
| Unclear precedence of nested operators (`js/unclear-operator-precedence`) | maintainability, correctness, external/cwe/cwe-783 | Highlights nested binary operators whose relative precedence is easy to misunderstand. Results shown on LGTM by default. |
| User-controlled data in file | security, external/cwe/cwe-912 | Highlights locations where user-controlled data is written to a file. Results are not shown on LGTM by default. |
## Changes to existing queries
@@ -29,5 +35,15 @@
| Remote property injection | Fewer results | The precision of this rule has been revised to "medium". Results are no longer shown on LGTM by default. |
| Missing CSRF middleware | Fewer false-positive results | This rule now recognizes additional CSRF protection middlewares. |
| Server-side URL redirect | More results | This rule now recognizes redirection calls in more cases. |
| Unused variable, import, function or class | Fewer results | This rule now flags import statements with multiple unused imports once. |
| User-controlled bypass of security check | Fewer results | This rule no longer flags conditions that guard early returns. The precision of this rule has been revised to "medium". Results are no longer shown on LGTM by default. |
| Whitespace contradicts operator precedence | Fewer false-positive results | This rule no longer flags operators with asymmetric whitespace. |
| Unused import | Fewer false-positive results | This rule no longer flags imports used by the `transform-react-jsx` Babel plugin. |
## Changes to QL libraries
* The flow configuration framework now supports distinguishing and tracking different kinds of taint, specified by an extensible class `FlowLabel` (which can also be referred to by its alias `TaintKind`).
* The `DataFlow::ThisNode` class now corresponds to the implicit receiver parameter of a function, as opposed to an indivdual `this` expression. This means `getALocalSource` now maps all `this` expressions within a given function to the same source. The data-flow node associated with a `ThisExpr` can no longer be cast to `DataFlow::SourceNode` or `DataFlow::ThisNode` - it is recomended to use `getALocalSource` before casting or instead of casting.
* `ReactComponent::getAThisAccess` has been renamed to `getAThisNode`. The old name is still usable but is deprecated. It no longer gets individual `this` expressions, but the `ThisNode` mentioned above.

1
cpp/config/suites/c/correctness
View File

@@ -6,6 +6,7 @@
+ semmlecode-cpp-queries/Likely Bugs/Arithmetic/IntMultToLong.ql: /Correctness/Dangerous Conversions
+ semmlecode-cpp-queries/Likely Bugs/Conversion/NonzeroValueCastToPointer.ql: /Correctness/Dangerous Conversions
+ semmlecode-cpp-queries/Likely Bugs/Conversion/ImplicitDowncastFromBitfield.ql: /Correctness/Dangerous Conversions
+ semmlecode-cpp-queries/Security/CWE/CWE-253/HResultBooleanConversion.ql: /Correctness/Dangerous Conversions
# Consistent Use
+ semmlecode-cpp-queries/Critical/ReturnValueIgnored.ql: /Correctness/Consistent Use
+ semmlecode-cpp-queries/Likely Bugs/InconsistentCheckReturnNull.ql: /Correctness/Consistent Use

1
cpp/config/suites/cpp/correctness
View File

@@ -7,6 +7,7 @@
+ semmlecode-cpp-queries/Likely Bugs/Conversion/NonzeroValueCastToPointer.ql: /Correctness/Dangerous Conversions
+ semmlecode-cpp-queries/Likely Bugs/Conversion/ImplicitDowncastFromBitfield.ql: /Correctness/Dangerous Conversions
+ semmlecode-cpp-queries/Likely Bugs/Conversion/CastArrayPointerArithmetic.ql: /Correctness/Dangerous Conversions
+ semmlecode-cpp-queries/Security/CWE/CWE-253/HResultBooleanConversion.ql: /Correctness/Dangerous Conversions
# Consistent Use
+ semmlecode-cpp-queries/Critical/ReturnValueIgnored.ql: /Correctness/Consistent Use
+ semmlecode-cpp-queries/Likely Bugs/InconsistentCheckReturnNull.ql: /Correctness/Consistent Use

3
cpp/config/suites/security/cwe-253 Normal file
View File

@@ -0,0 +1,3 @@
# CWE-253: Incorrect Check of Function Return Value
+ semmlecode-cpp-queries/Security/CWE/CWE-253/HResultBooleanConversion.ql: /CWE/CWE-253
@name Cast between HRESULT and a Boolean type (CWE-253)

3
cpp/config/suites/security/cwe-428 Normal file
View File

@@ -0,0 +1,3 @@
# CWE-428: Unquoted Search Path or Element
+ semmlecode-cpp-queries/Security/CWE/CWE-428/UnsafeCreateProcessCall.ql: /CWE/CWE-428
@name NULL application name with an unquoted path in call to CreateProcess (CWE-428)

2
cpp/config/suites/security/cwe-732
View File

@@ -1,3 +1,5 @@
# CWE-732: Incorrect Permission Assignment for Critical Resource
+ semmlecode-cpp-queries/Security/CWE/CWE-732/DoNotCreateWorldWritable.ql: /CWE/CWE-732
@name File created without restricting permissions (CWE-732)
+ semmlecode-cpp-queries/Security/CWE/CWE-732/UnsafeDaclSecurityDescriptor.ql: /CWE/CWE-732
@name Setting a DACL to NULL in a SECURITY_DESCRIPTOR (CWE-732)

2
cpp/config/suites/security/default
View File

@@ -13,11 +13,13 @@
@import "cwe-170"
@import "cwe-190"
@import "cwe-242"
@import "cwe-253"
@import "cwe-290"
@import "cwe-311"
@import "cwe-327"
@import "cwe-367"
@import "cwe-416"
@import "cwe-428"
@import "cwe-457"
@import "cwe-468"
@import "cwe-676"

27
cpp/ql/src/Best Practices/Likely Errors/EmptyBlock.ql
View File

@@ -29,6 +29,10 @@ class AffectedFile extends File {
}
}
/**
* A block, or an element we might find textually within a block that is
* not a child of it in the AST.
*/
class BlockOrNonChild extends Element {
BlockOrNonChild() {
( this instanceof Block
@@ -68,6 +72,9 @@ class BlockOrNonChild extends Element {
}
}
/**
* A block that contains a non-child element.
*/
predicate emptyBlockContainsNonchild(Block b) {
emptyBlock(_, b) and
exists(BlockOrNonChild c, AffectedFile file |
@@ -78,7 +85,27 @@ predicate emptyBlockContainsNonchild(Block b) {
)
}
/**
* A block that is entirely on one line, which also contains a comment. Chances
* are the comment is intended to refer to the block.
*/
predicate lineComment(Block b) {
emptyBlock(_, b) and
exists(Location bLocation, File f, int line |
bLocation = b.getLocation() and
f = bLocation.getFile() and
line = bLocation.getStartLine() and
line = bLocation.getEndLine() and
exists(Comment c, Location cLocation |
cLocation = c.getLocation() and
cLocation.getFile() = f and
cLocation.getStartLine() = line
)
)
}
from ControlStructure s, Block eb
where emptyBlock(s, eb)
and not emptyBlockContainsNonchild(eb)
and not lineComment(eb)
select eb, "Empty block without comment"

7
cpp/ql/src/Critical/DeadCodeGoto.cpp Normal file
View File

@@ -0,0 +1,7 @@
goto err1;
free(pointer); // BAD: this line is unreachable
err1: return -1;
free(pointer); // GOOD: this line is reachable
goto err2;
err2: return -1;

28
cpp/ql/src/Critical/DeadCodeGoto.qhelp Normal file
View File

@@ -0,0 +1,28 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>
Code immediately following a <code>goto</code> or <code>break</code> statement will not be executed,
unless there is a label or switch case. When the code is necessary, this leads to logical errors or
resource leaks. If the code is unnecessary, it may confuse readers.
</p>
</overview>
<recommendation>
<p>
If the unreachable code is necessary, move the <code>goto</code> or <code>break</code> statement to
after the code. Otherwise, delete the unreachable code.
</p>
</recommendation>
<example><sample src="DeadCodeGoto.cpp" />
</example>
<references>
<li>
The CERT C Secure Coding Standard: <a href="https://wiki.sei.cmu.edu/confluence/display/c/MSC12-C.+Detect+and+remove+code+that+has+no+effect+or+is+never+executed">MSC12-C. Detect and remove code that has no effect or is never executed</a>.
</li>
</references>
</qhelp>

33
cpp/ql/src/Critical/DeadCodeGoto.ql Normal file
View File

@@ -0,0 +1,33 @@
/**
* @name Dead code due to goto or break statement
* @description A goto or break statement is followed by unreachable code.
* @kind problem
* @problem.severity warning
* @precision high
* @id cpp/dead-code-goto
* @tags maintainability
* external/cwe/cwe-561
*/
import cpp
Stmt getNextRealStmt(Block b, int i) {
result = b.getStmt(i + 1) and
not result instanceof EmptyStmt
or
b.getStmt(i + 1) instanceof EmptyStmt and
result = getNextRealStmt(b, i + 1)
}
from JumpStmt js, Block b, int i, Stmt s
where b.getStmt(i) = js
and s = getNextRealStmt(b, i)
// the next statement isn't jumped to
and not s instanceof LabelStmt
and not s instanceof SwitchCase
// the next statement isn't breaking out of a switch
and not s.(BreakStmt).getBreakable() instanceof SwitchStmt
// the next statement isn't a loop that can be jumped into
and not exists (LabelStmt ls | s.(Loop).getStmt().getAChild*() = ls)
and not exists (SwitchCase sc | s.(Loop).getStmt().getAChild*() = sc)
select js, "This statement makes $@ unreachable.", s, s.toString()

10
cpp/ql/src/Critical/OverflowDestination.cpp
View File

@@ -1,13 +1,13 @@
int main(int argc, char* argv[]) {
char param[SIZE];
char param[20];
char *arg1;
char arg1[10];
char arg2[20];
arg1 = argv[1];
//wrong: only uses the size of the source (argv[1]) when using strncpy
strncpy(param, argv[1], strlen(arg1));
strncpy(param, arg1, strlen(arg1));
//correct: uses the size of the destination array as well
strncpy(param, argv[1], min(strlen(arg1, sizeof(param) -1)));
strncpy(param, arg1, min(strlen(arg1), sizeof(param) -1));
}

65
cpp/ql/src/Critical/OverflowDestination.ql
View File

@@ -5,26 +5,38 @@
* @kind problem
* @id cpp/overflow-destination
* @problem.severity warning
* @precision low
* @tags reliability
* security
* external/cwe/cwe-119
* external/cwe/cwe-131
*/
import cpp
import semmle.code.cpp.pointsto.PointsTo
import semmle.code.cpp.security.TaintTracking
predicate sourceSized(FunctionCall fc)
/**
* Holds if `fc` is a call to a copy operation where the size argument contains
* a reference to the source argument. For example:
* ```
* memcpy(dest, src, sizeof(src));
* ```
*/
predicate sourceSized(FunctionCall fc, Expr src)
{
exists(string name |
(name = "strncpy" or name = "strncat" or name = "memcpy" or name = "memmove") and
fc.getTarget().hasQualifiedName(name))
and
exists(Expr dest, Expr src, Expr size, Variable v |
exists(Expr dest, Expr size, Variable v |
fc.getArgument(0) = dest and fc.getArgument(1) = src and fc.getArgument(2) = size and
src = v.getAnAccess() and size.getAChild+() = v.getAnAccess() and
// exception: `dest` is also referenced in the size argument
not exists(Variable other |
dest = other.getAnAccess() and size.getAChild+() = other.getAnAccess())
and
// exception: `src` and `dest` are both arrays of the same type and size
not exists(ArrayType srctype, ArrayType desttype |
dest.getType().getUnderlyingType() = desttype and
src.getType().getUnderlyingType() = srctype and
@@ -32,48 +44,7 @@ predicate sourceSized(FunctionCall fc)
desttype.getArraySize() = srctype.getArraySize()))
}
class VulnerableArgument extends PointsToExpr
{
VulnerableArgument() { sourceSized(this.getParent()) }
override predicate interesting() { sourceSized(this.getParent()) }
}
predicate taintingFunction(Function f, int buf)
{
(f.hasQualifiedName("read") and buf = 1) or
(f.hasQualifiedName("fgets") and buf = 0) or
(f.hasQualifiedName("fread") and buf = 0)
}
// Taint `argv[i]`, for all i, but also `*argv`, etc.
predicate commandLineArg(Expr e)
{
exists(Function f, Parameter argv, VariableAccess access |
f.hasQualifiedName("main") and f.getParameter(1) = argv and
argv.getAnAccess() = access and access.isRValue() and
pointer(access, e))
}
predicate tainted(Expr e)
{
exists(FunctionCall fc, int arg |
taintingFunction(fc.getTarget(), arg) and
e = fc.getArgument(arg))
or
e.(FunctionCall).getTarget().hasQualifiedName("getenv")
or
commandLineArg(e)
}
class TaintedArgument extends PointsToExpr
{
TaintedArgument() { tainted(this) }
override predicate interesting() { tainted(this) }
}
from FunctionCall fc, VulnerableArgument vuln, TaintedArgument tainted
where sourceSized(fc)
and fc.getArgument(1) = vuln
and vuln.pointsTo() = tainted.pointsTo()
and vuln.confidence() > 0.01
from FunctionCall fc, Expr vuln, Expr taintSource
where sourceSized(fc, vuln)
and tainted(taintSource, vuln)
select fc, "To avoid overflow, this operation should be bounded by destination-buffer size, not source-buffer size."

4
cpp/ql/src/Likely Bugs/Arithmetic/PointlessComparison.ql
View File

@@ -22,13 +22,15 @@ import UnsignedGEZero
// #define PRINTMSG(val,msg) { if (val >= PRINTLEVEL) printf(msg); }
//
// So to reduce the number of false positives, we do not report a result if
// the comparison is in a macro expansion.
// the comparison is in a macro expansion. Similarly for template
// instantiations.
from
ComparisonOperation cmp, SmallSide ss,
float left, float right, boolean value,
string reason
where
not cmp.isInMacroExpansion() and
not cmp.isFromTemplateInstantiation(_) and
reachablePointlessComparison(cmp, left, right, value, ss) and
// a comparison between an enum and zero is always valid because whether

1
cpp/ql/src/Likely Bugs/Arithmetic/UnsignedGEZero.qll
View File

@@ -50,5 +50,6 @@ predicate unsignedGEZero(UnsignedGEZero ugez, string msg) {
ugez.getLocation().getStartLine() = mi.getLocation().getStartLine() and
ugez.getLocation().getStartColumn() = mi.getLocation().getStartColumn()
) and
not ugez.isFromTemplateInstantiation(_) and
msg = "Pointless comparison of unsigned value to zero."
}

2
cpp/ql/src/Likely Bugs/Format/NonConstantFormat.ql
View File

@@ -25,7 +25,7 @@ predicate stringType(Type t, Type charType) {
charType = t.(ArrayType).getBaseType()
) and (
charType.getUnspecifiedType() instanceof CharType or
charType.getUnspecifiedType() instanceof WideCharType
charType.getUnspecifiedType() instanceof Wchar_t
)
)
or

125
cpp/ql/src/Likely Bugs/Format/WrongTypeFormatArguments.ql
View File

@@ -25,7 +25,8 @@ private predicate formattingFunctionCallExpectedType(FormattingFunctionCall ffc,
ffc.getTarget() = f and
f.getFormatParameterIndex() = i and
ffc.getArgument(i) = fl and
fl.getConversionType(pos) = expected
fl.getConversionType(pos) = expected and
count(fl.getConversionType(pos)) = 1
)
}
@@ -66,30 +67,15 @@ predicate formatOtherArgType(FormattingFunctionCall ffc, int pos, Type expected,
class ExpectedType extends Type
{
ExpectedType() {
formatArgType(_, _, this, _, _) or
formatOtherArgType(_, _, this, _, _) or
exists(ExpectedType t |
this = t.(PointerType).getBaseType()
exists(Type t |
(
formatArgType(_, _, t, _, _) or
formatOtherArgType(_, _, t, _, _)
) and this = t.getUnspecifiedType()
)
}
}
/**
* Gets an 'interesting' type that can be reached from `t` by removing
* typedefs and specifiers. Note that this does not always mean removing
* all typedefs and specifiers as `Type.getUnspecifiedType()` would, for
* example if the interesting type is itself a typedef.
*/
ExpectedType getAnUnderlyingExpectedType(Type t) {
(
result = t
) or (
result = getAnUnderlyingExpectedType(t.(TypedefType).getBaseType())
) or (
result = getAnUnderlyingExpectedType(t.(SpecifiedType).getBaseType())
)
}
/**
* Holds if it is safe to display a value of type `actual` when `printf`
* expects a value of type `expected`.
@@ -100,59 +86,48 @@ ExpectedType getAnUnderlyingExpectedType(Type t) {
* are converted to `double`.
*/
predicate trivialConversion(ExpectedType expected, Type actual) {
formatArgType(_, _, expected, _, actual) and
exists(Type actualU |
actualU = actual.getUnspecifiedType() and
exists(Type exp, Type act |
formatArgType(_, _, exp, _, act) and
expected = exp.getUnspecifiedType() and
actual = act.getUnspecifiedType()
) and (
(
(
// allow a pointer type to be displayed with `%p`
expected instanceof VoidPointerType and actualU instanceof PointerType
) or (
// allow a function pointer type to be displayed with `%p`
expected instanceof VoidPointerType and actualU instanceof FunctionPointerType and expected.getSize() = actual.getSize()
) or (
// allow an `enum` type to be displayed with `%i`, `%c` etc
expected instanceof IntegralType and actualU instanceof Enum
) or (
// allow any `char *` type to be displayed with `%s`
expected instanceof CharPointerType and actualU instanceof CharPointerType
) or (
// allow `wchar_t *`, or any pointer to an integral type of the same size, to be displayed
// with `%ws`
expected.(PointerType).getBaseType().hasName("wchar_t") and
exists(Wchar_t t |
actual.getUnspecifiedType().(PointerType).getBaseType().(IntegralType).getSize() = t.getSize()
)
) or (
// allow an `int` (or anything promoted to `int`) to be displayed with `%c`
expected instanceof CharType and actualU instanceof IntType
) or (
// allow an `int` (or anything promoted to `int`) to be displayed with `%wc`
expected instanceof Wchar_t and actualU instanceof IntType
) or (
expected instanceof UnsignedCharType and actualU instanceof IntType
) or (
// allow the underlying type of a `size_t` (e.g. `unsigned long`) for
// `%zu`, since this is the type of a `sizeof` expression
expected instanceof Size_t and
actual.getUnspecifiedType() = expected.getUnspecifiedType()
) or (
// allow the underlying type of a `ssize_t` (e.g. `long`) for `%zd`
expected instanceof Ssize_t and
actual.getUnspecifiedType() = expected.getUnspecifiedType()
) or (
// allow any integral type of the same size
// (this permits signedness changes)
expected.(IntegralType).getSize() = actualU.(IntegralType).getSize()
) or (
// allow a pointer to any integral type of the same size
// (this permits signedness changes)
expected.(PointerType).getBaseType().(IntegralType).getSize() = actualU.(PointerType).getBaseType().(IntegralType).getSize()
) or (
// allow expected, or a typedef or specified version of expected
expected = getAnUnderlyingExpectedType(actual)
// allow a pointer type to be displayed with `%p`
expected instanceof VoidPointerType and actual instanceof PointerType
) or (
// allow a function pointer type to be displayed with `%p`
expected instanceof VoidPointerType and actual instanceof FunctionPointerType and expected.getSize() = actual.getSize()
) or (
// allow an `enum` type to be displayed with `%i`, `%c` etc
expected instanceof IntegralType and actual instanceof Enum
) or (
// allow any `char *` type to be displayed with `%s`
expected instanceof CharPointerType and actual instanceof CharPointerType
) or (
// allow `wchar_t *`, or any pointer to an integral type of the same size, to be displayed
// with `%ws`
expected.(PointerType).getBaseType().hasName("wchar_t") and
exists(Wchar_t t |
actual.getUnspecifiedType().(PointerType).getBaseType().(IntegralType).getSize() = t.getSize()
)
) or (
// allow an `int` (or anything promoted to `int`) to be displayed with `%c`
expected instanceof CharType and actual instanceof IntType
) or (
// allow an `int` (or anything promoted to `int`) to be displayed with `%wc`
expected instanceof Wchar_t and actual instanceof IntType
) or (
expected instanceof UnsignedCharType and actual instanceof IntType
) or (
// allow any integral type of the same size
// (this permits signedness changes)
expected.(IntegralType).getSize() = actual.(IntegralType).getSize()
) or (
// allow a pointer to any integral type of the same size
// (this permits signedness changes)
expected.(PointerType).getBaseType().(IntegralType).getSize() = actual.(PointerType).getBaseType().(IntegralType).getSize()
) or (
expected = actual
)
)
}
@@ -164,16 +139,16 @@ int sizeof_IntType() {
exists(IntType it | result = it.getSize())
}
from FormattingFunctionCall ffc, int n, Expr arg, ExpectedType expected, Type actual
from FormattingFunctionCall ffc, int n, Expr arg, Type expected, Type actual
where (
(
formatArgType(ffc, n, expected, arg, actual) and
not trivialConversion(expected, actual)
not trivialConversion(expected.getUnspecifiedType(), actual.getUnspecifiedType())
)
or
(
formatOtherArgType(ffc, n, expected, arg, actual) and
not actual.getUnderlyingType().(IntegralType).getSize() = sizeof_IntType()
not actual.getUnspecifiedType().(IntegralType).getSize() = sizeof_IntType()
)
)
and not arg.isAffectedByMacro()

7
cpp/ql/src/Likely Bugs/Likely Typos/inconsistentLoopDirection.c Normal file
View File

@@ -0,0 +1,7 @@
void f()
{
for (signed char i = 0; i < 100; i--)
{
// code ...
}
}

27
cpp/ql/src/Likely Bugs/Likely Typos/inconsistentLoopDirection.qhelp Normal file
View File

@@ -0,0 +1,27 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>A <code>for-loop</code> iteration expression goes backwards with respect of the initialization statement and condition expression.</p>
<p>This warning indicates that a <code>for-loop</code> might not function as intended.</p>
</overview>
<recommendation>
<p>To fix this issue, check that the loop condition is correct and change the iteration expression to match.</p>
</recommendation>
<example>
<p>In the following example, the initialization statement (<code>i = 0</code>) and the condition expression (<code>i &lt; 100</code>) indicate that the intended iteration expression should have been incrementing, but instead a postfix decrement operator is used (<code>i--</code>).</p>
<sample src="inconsistentLoopDirection.c" />
<p>To fix this issue, change the iteration expression to match the direction of the initialization statement and the condition expression: <code>i++</code>.</p>
</example>
<references>
<li><a href="https://docs.microsoft.com/en-us/previous-versions/visualstudio/visual-studio-2012/58teb7hd(v=vs.110)">warning C6293: Ill-defined for-loop: counts down from minimum</a>
</li>
</references>
</qhelp>

116
cpp/ql/src/Likely Bugs/Likely Typos/inconsistentLoopDirection.ql Normal file
View File

@@ -0,0 +1,116 @@
/**
* @name Inconsistent direction of for loop
* @description A for-loop iteration expression goes backward with respect of the initialization statement and condition expression.
* @kind problem
* @problem.severity error
* @precision high
* @id cpp/inconsistent-loop-direction
* @tags correctness
* external/cwe/cwe-835
* external/microsoft/6293
* @msrc.severity important
*/
import cpp
import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis
import semmle.code.cpp.dataflow.DataFlow
predicate illDefinedDecrForStmt( ForStmt forstmt, Variable v, Expr initialCondition, Expr terminalCondition ) {
v.getAnAssignedValue() = initialCondition
and
exists(
RelationalOperation rel |
rel = forstmt.getCondition() |
terminalCondition = rel.getGreaterOperand()
and v.getAnAccess() = rel.getLesserOperand()
and
DataFlow::localFlowStep(DataFlow::exprNode(initialCondition), DataFlow::exprNode(rel.getLesserOperand()))
)
and
exists(
DecrementOperation dec |
dec = forstmt.getUpdate().(DecrementOperation)
and dec.getAnOperand() = v.getAnAccess()
)
and
(
( upperBound(initialCondition) < lowerBound(terminalCondition) )
or
( forstmt.conditionAlwaysFalse() or forstmt.conditionAlwaysTrue() )
)
}
predicate illDefinedIncrForStmt( ForStmt forstmt, Variable v, Expr initialCondition, Expr terminalCondition ) {
v.getAnAssignedValue() = initialCondition
and
exists(
RelationalOperation rel |
rel = forstmt.getCondition() |
terminalCondition = rel.getLesserOperand()
and v.getAnAccess() = rel.getGreaterOperand()
and
DataFlow::localFlowStep(DataFlow::exprNode(initialCondition), DataFlow::exprNode(rel.getGreaterOperand()))
)
and
exists( IncrementOperation incr |
incr = forstmt.getUpdate().(IncrementOperation)
and
incr.getAnOperand() = v.getAnAccess()
)
and
(
( upperBound(terminalCondition) < lowerBound(initialCondition))
or
( forstmt.conditionAlwaysFalse() or forstmt.conditionAlwaysTrue())
)
}
predicate illDefinedForStmtWrongDirection( ForStmt forstmt, Variable v, Expr initialCondition, Expr terminalCondition
, boolean isIncr ) {
( illDefinedDecrForStmt( forstmt, v, initialCondition, terminalCondition) and isIncr = false )
or
( illDefinedIncrForStmt( forstmt, v, initialCondition, terminalCondition) and isIncr = true)
}
bindingset[b]
private string forLoopdirection(boolean b){
if( b = true ) then result = "upward"
else result = "downward"
}
bindingset[b]
private string forLoopTerminalConditionRelationship(boolean b){
if( b = true ) then result = "lower"
else result = "higher"
}
predicate illDefinedForStmt( ForStmt for, string message ) {
exists(
boolean isIncr,
Variable v,
Expr initialCondition,
Expr terminalCondition |
illDefinedForStmtWrongDirection(for, v, initialCondition, terminalCondition, isIncr)
and
if( for.conditionAlwaysFalse() ) then
(
message = "Ill-defined for-loop: a loop using variable \"" + v + "\" counts "
+ forLoopdirection(isIncr) + " from a value ("+ initialCondition +"), but the terminal condition is always false."
)
else if
(
for.conditionAlwaysTrue() ) then (
message = "Ill-defined for-loop: a loop using variable \"" + v + "\" counts "
+ forLoopdirection(isIncr) + " from a value ("+ initialCondition +"), but the terminal condition is always true."
)
else
(
message = "Ill-defined for-loop: a loop using variable \"" + v + "\" counts "
+ forLoopdirection(isIncr) + " from a value ("+ initialCondition +"), but the terminal condition is "
+ forLoopTerminalConditionRelationship(isIncr) + " (" + terminalCondition + ")."
)
)
}
from ForStmt forstmt, string message
where illDefinedForStmt(forstmt, message)
select forstmt, message

1
cpp/ql/src/Likely Bugs/OO/NonVirtualDestructor.ql
View File

@@ -7,6 +7,7 @@
* @id cpp/non-virtual-destructor
* @problem.severity warning
* @tags reliability
* @deprecated
*/
// This query is deprecated, and replaced by jsf/4.10 Classes/AV Rule 78.ql, which has far fewer false positives on typical code.

19
cpp/ql/src/Likely Bugs/UseInOwnInitializer.ql
View File

@@ -11,10 +11,21 @@
import cpp
from Initializer init, Variable v, VariableAccess va
where init.getDeclaration() = v
and va.getTarget() = v
and va.getParent*() = init
class VariableAccessInInitializer extends VariableAccess {
Variable var;
Initializer init;
VariableAccessInInitializer() {
init.getDeclaration() = var and
init.getExpr().getAChild*() = this
}
predicate initializesItself(Variable v, Initializer i) {
v = var and i = init and var = this.getTarget()
}
}
from Initializer init, Variable v, VariableAccessInInitializer va
where va.initializesItself(v, init)
and (
va.hasLValueToRValueConversion() or
exists (Assignment assn | assn.getLValue() = va) or

4
cpp/ql/src/META-INF/MANIFEST.MF
View File

@@ -2,7 +2,7 @@ Manifest-Version: 1.0
Bundle-ManifestVersion: 2
Bundle-Name: Semmle C/C++ Default Queries
Bundle-SymbolicName: com.semmle.plugin.semmlecode.cpp.queries;singleton:=true
Bundle-Version: 1.18.1.qualifier
Bundle-Version: 1.18.1
Bundle-Vendor: Semmle Ltd.
Bundle-ActivationPolicy: lazy
Require-Bundle: com.semmle.plugin.qdt.ui;bundle-version="[1.18.1.qualifier,1.18.1.qualifier]"
Require-Bundle: com.semmle.plugin.qdt.ui;bundle-version="[1.18.1,1.18.1]"

8
cpp/ql/src/Security/CWE/CWE-253/HResultBooleanConversion.qhelp
View File

@@ -4,13 +4,13 @@
<qhelp>
<overview>
<p>This query indicates that an <code>HRESULT</code> is being cast to a boolean type or vice versa.</p>
<p>The typical success value (<code>S_OK</code>) of an <code>HRESULT</code> equals 0. However, 0 indicates failure for a boolean type.</p>
<p>Casting an <code>HRESULT</code> to a boolean type and then using it in a test expression will yield an incorrect result.</p>
<p>This query indicates that an <code>HRESULT</code> is being cast to a Boolean type or vice versa.</p>
<p>The typical success value (<code>S_OK</code>) of an <code>HRESULT</code> equals 0. However, 0 indicates failure for a Boolean type.</p>
<p>Casting an <code>HRESULT</code> to a Boolean type and then using it in a test expression will yield an incorrect result.</p>
</overview>
<recommendation>
<p>To check if a call that returns an HRESULT succeeded use the <code>FAILED</code> macro.</p>
<p>To check if a call that returns an <code>HRESULT</code> succeeded use the <code>FAILED</code> macro.</p>
</recommendation>
<example>

9
cpp/ql/src/Security/CWE/CWE-253/HResultBooleanConversion.ql
View File

@@ -1,8 +1,6 @@
/**
* @name Cast between semantically different integer types: HRESULT to/from a Boolean type
* @description Cast between semantically different integer types: HRESULT to/from a Boolean type.
* Boolean types indicate success by a non-zero value, whereas success (S_OK) in HRESULT is indicated by a value of 0.
* Casting an HRESULT to/from a Boolean type and then using it in a test expression will yield an incorrect result.
* @name Cast between HRESULT and a Boolean type
* @description Casting an HRESULT to/from a Boolean type and then using it in a test expression will yield an incorrect result because success (S_OK) in HRESULT is indicated by a value of 0.
* @kind problem
* @id cpp/hresult-boolean-conversion
* @problem.severity error
@@ -50,6 +48,7 @@ where exists
ctls.getControllingExpr() = e1
and e1.getType().(TypedefType).hasName("HRESULT")
and not isHresultBooleanConverted(e1)
and not ctls instanceof SwitchStmt // not controlled by a boolean condition
and msg = "Direct usage of a type " + e1.getType().toString() + " as a conditional expression"
)
or
@@ -68,4 +67,4 @@ where exists
)
and not isHresultBooleanConverted(e1)
)
select e1, msg
select e1, msg

11
cpp/ql/src/Security/CWE/CWE-428/UnsafeCreateProcessCall.cpp Normal file
View File

@@ -0,0 +1,11 @@
STARTUPINFOW si;
PROCESS_INFORMATION pi;
// ...
CreateProcessW( // BUG
NULL, // lpApplicationName
(LPWSTR)L"C:\\Program Files\\MyApp", // lpCommandLine
NULL, NULL, FALSE, 0, NULL, NULL, &si, &pi);
// ...

46
cpp/ql/src/Security/CWE/CWE-428/UnsafeCreateProcessCall.qhelp Normal file
View File

@@ -0,0 +1,46 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>This query indicates that there is a call to a function of the <code>CreateProcess*</code> family of functions, which introduces a security vulnerability.</p>
</overview>
<recommendation>
<p>Do not use <code>NULL</code> for the <code>lpApplicationName</code> argument to the <code>CreateProcess*</code> function.</p>
<p>If you pass <code>NULL</code> for <code>lpApplicationName</code>, use quotation marks around the executable path in <code>lpCommandLine</code>.</p>
</recommendation>
<example>
<p>In the following example, <code>CreateProcessW</code> is called with a <code>NULL</code> value for <code>lpApplicationName</code>,
and the value for <code>lpCommandLine</code> that represent the application path is not quoted and has spaces in it.</p>
<p>If an attacker has access to the file system, they can elevate privileges by creating a file such as <code>C:\Program.exe</code> that will be executed instead of the intended application.</p>
<sample src="UnsafeCreateProcessCall.cpp" />
<p>To fix this issue, specify a valid string for <code>lpApplicationName</code>, or quote the path for <code>lpCommandLine</code>. For example:</p>
<p><code>(LPWSTR)L"\"C:\\Program Files\\MyApp\"", // lpCommandLine</code></p>
</example>
<references>
<li>
<a href="https://docs.microsoft.com/en-us/windows/desktop/api/processthreadsapi/nf-processthreadsapi-createprocessa">CreateProcessA function (Microsoft documentation).</a>
</li>
<li>
<a href="https://docs.microsoft.com/en-us/windows/desktop/api/processthreadsapi/nf-processthreadsapi-createprocessw">CreateProcessW function (Microsoft documentation).</a>
</li>
<li>
<a href="https://docs.microsoft.com/en-us/windows/desktop/api/processthreadsapi/nf-processthreadsapi-createprocessasusera">CreateProcessAsUserA function (Microsoft documentation).</a>
</li>
<li>
<a href="https://docs.microsoft.com/en-us/windows/desktop/api/processthreadsapi/nf-processthreadsapi-createprocessasuserw">CreateProcessAsUserW function (Microsoft documentation).</a>
</li>
<li>
<a href="https://docs.microsoft.com/en-us/windows/desktop/api/winbase/nf-winbase-createprocesswithlogonw">CreateProcessWithLogonW function (Microsoft documentation).</a>
</li>
<li>
<a href="https://docs.microsoft.com/en-us/windows/desktop/api/winbase/nf-winbase-createprocesswithtokenw">CreateProcessWithTokenW function (Microsoft documentation).</a>
</li>
</references>
</qhelp>

130
cpp/ql/src/Security/CWE/CWE-428/UnsafeCreateProcessCall.ql Normal file
View File

@@ -0,0 +1,130 @@
/**
* @name NULL application name with an unquoted path in call to CreateProcess
* @description Calling a function of the CreateProcess* family of functions, where the path contains spaces, introduces a security vulnerability.
* @id cpp/unsafe-create-process-call
* @kind problem
* @problem.severity error
* @precision medium
* @msrc.severity important
* @tags security
* external/cwe/cwe-428
* external/microsoft/C6277
*/
import cpp
import semmle.code.cpp.dataflow.DataFlow
import semmle.code.cpp.dataflow.DataFlow2
predicate isCreateProcessFunction(FunctionCall call, int applicationNameIndex, int commandLineIndex) {
(
call.getTarget().hasGlobalName("CreateProcessA")
and applicationNameIndex = 0
and commandLineIndex = 1
) or (
call.getTarget().hasGlobalName("CreateProcessW")
and applicationNameIndex = 0
and commandLineIndex = 1
) or (
call.getTarget().hasGlobalName("CreateProcessWithTokenW")
and applicationNameIndex = 2
and commandLineIndex = 3
) or (
call.getTarget().hasGlobalName("CreateProcessWithLogonW")
and applicationNameIndex = 4
and commandLineIndex = 5
) or (
call.getTarget().hasGlobalName("CreateProcessAsUserA")
and applicationNameIndex = 1
and commandLineIndex = 2
) or (
call.getTarget().hasGlobalName("CreateProcessAsUserW")
and applicationNameIndex = 1
and commandLineIndex = 2
)
}
/**
* A function call to CreateProcess (either wide-char or single byte string versions)
*/
class CreateProcessFunctionCall extends FunctionCall {
CreateProcessFunctionCall() {
isCreateProcessFunction( this, _, _)
}
int getApplicationNameArgumentId() {
isCreateProcessFunction( this, result, _)
}
int getCommandLineArgumentId() {
isCreateProcessFunction( this, _, result)
}
}
/**
* Dataflow that detects a call to CreateProcess with a NULL value for lpApplicationName argument
*/
class NullAppNameCreateProcessFunctionConfiguration extends DataFlow::Configuration {
NullAppNameCreateProcessFunctionConfiguration() {
this = "NullAppNameCreateProcessFunctionConfiguration"
}
override predicate isSource(DataFlow::Node source) {
nullValue(source.asExpr())
}
override predicate isSink(DataFlow::Node sink) {
exists(
CreateProcessFunctionCall call, Expr val |
val = sink.asExpr() |
val = call.getArgument(call.getApplicationNameArgumentId())
)
}
}
/**
* Dataflow that detects a call to CreateProcess with an unquoted commandLine argument
*/
class QuotedCommandInCreateProcessFunctionConfiguration extends DataFlow2::Configuration {
QuotedCommandInCreateProcessFunctionConfiguration() {
this = "QuotedCommandInCreateProcessFunctionConfiguration"
}
override predicate isSource(DataFlow2::Node source) {
exists( string s |
s = source.asExpr().getValue().toString()
and
not isQuotedOrNoSpaceApplicationNameOnCmd(s)
)
}
override predicate isSink(DataFlow2::Node sink) {
exists(
CreateProcessFunctionCall call, Expr val |
val = sink.asExpr() |
val = call.getArgument(call.getCommandLineArgumentId())
)
}
}
bindingset[s]
predicate isQuotedOrNoSpaceApplicationNameOnCmd(string s){
s.regexpMatch("\"([^\"])*\"(\\s|.)*") // The first element (path) is quoted
or
s.regexpMatch("[^\\s]+") // There are no spaces in the string
}
from CreateProcessFunctionCall call, string msg1, string msg2
where
exists( Expr source, Expr appName,
NullAppNameCreateProcessFunctionConfiguration nullAppConfig |
appName = call.getArgument(call.getApplicationNameArgumentId())
and nullAppConfig.hasFlow(DataFlow2::exprNode(source), DataFlow2::exprNode(appName))
and msg1 = call.toString() + " with lpApplicationName == NULL (" + appName + ")"
)
and
exists( Expr source, Expr cmd,
QuotedCommandInCreateProcessFunctionConfiguration quotedConfig |
cmd = call.getArgument(call.getCommandLineArgumentId())
and quotedConfig.hasFlow(DataFlow2::exprNode(source), DataFlow2::exprNode(cmd))
and msg2 = " and with an unquoted lpCommandLine (" + cmd + ") introduces a security vulnerability if the path contains spaces."
)
select call, msg1 + " " + msg2

4
cpp/ql/src/Security/CWE/CWE-676/DangerousUseOfCin.ql
View File

@@ -19,7 +19,7 @@ import cpp
class AnyCharPointerType extends PointerType {
AnyCharPointerType() {
this.getBaseType().getUnderlyingType() instanceof CharType or
this.getBaseType().getUnderlyingType() instanceof WideCharType
this.getBaseType().getUnderlyingType() instanceof Wchar_t
}
}
@@ -29,7 +29,7 @@ class AnyCharPointerType extends PointerType {
class AnyCharArrayType extends ArrayType {
AnyCharArrayType() {
this.getBaseType().getUnderlyingType() instanceof CharType or
this.getBaseType().getUnderlyingType() instanceof WideCharType
this.getBaseType().getUnderlyingType() instanceof Wchar_t
}
}

3
cpp/ql/src/Security/CWE/CWE-704/WcharCharConversion.cpp Normal file
View File

@@ -0,0 +1,3 @@
wchar_t* pSrc;
pSrc = (wchar_t*)"a"; // casting a byte-string literal "a" to a wide-character string

35
cpp/ql/src/Security/CWE/CWE-704/WcharCharConversion.qhelp Normal file
View File

@@ -0,0 +1,35 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>This rule indicates a potentially incorrect cast from an byte string (<code>char *</code>) to a wide-character string (<code>wchar_t *</code>).</p>
<p>This cast might yield strings that are not correctly terminated; including potential buffer overruns when using such strings with some dangerous APIs.</p>
</overview>
<recommendation>
<p>Do not explicitly cast byte strings to wide-character strings.</p>
<p>For string literals, prepend the literal string with the letter "L" to indicate that the string is a wide-character string (<code>wchar_t *</code>).</p>
<p>For converting a byte literal to a wide-character string literal, you would need to use the appropriate conversion function for the platform you are using. Please see the references section for options according to your platform.</p>
</recommendation>
<example>
<p>In the following example, an byte string literal (<code>"a"</code>) is cast to a wide-character string.</p>
<sample src="WcharCharConversion.cpp" />
<p>To fix this issue, prepend the literal with the letter "L" (<code>L"a"</code>) to define it as a wide-character string.</p>
</example>
<references>
<li>
General resources:
<a href="https://en.cppreference.com/w/cpp/string/multibyte/mbstowcs">std::mbstowcs</a>
</li>
<li>
Microsoft specific resources:
<a href="https://docs.microsoft.com/en-us/windows/desktop/Intl/security-considerations--international-features">Security Considerations: International Features</a>
</li>
</references>
</qhelp>

29
cpp/ql/src/Security/CWE/CWE-704/WcharCharConversion.ql Normal file
View File

@@ -0,0 +1,29 @@
/**
* @name Cast from char* to wchar_t*
* @description Casting a byte string to a wide-character string is likely
* to yield a string that is incorrectly terminated or aligned.
* This can lead to undefined behavior, including buffer overruns.
* @kind problem
* @id cpp/incorrect-string-type-conversion
* @problem.severity error
* @precision high
* @tags security
* external/cwe/cwe-704
* external/microsoft/c/c6276
*/
import cpp
class WideCharPointerType extends PointerType {
WideCharPointerType() {
this.getBaseType() instanceof WideCharType
}
}
from Expr e1, Cast e2
where
e2 = e1.getConversion() and
exists(WideCharPointerType w, CharPointerType c |
w = e2.getType().getUnspecifiedType().(PointerType) and
c = e1.getType().getUnspecifiedType().(PointerType)
)
select e1, "Conversion from " + e1.getType().toString() + " to " + e2.getType().toString() + ". Use of invalid string can lead to undefined behavior."

16
cpp/ql/src/jsf/4.07 Header Files/AV Rule 35.ql
View File

@@ -107,15 +107,17 @@ predicate defUndef(File f, string macroName) {
}
/**
* Header file `hf` looks like it contains an x-macro called
* `macroName`. That is, a macro that is used to interpret the
* Header file `hf` looks like it contains an x-macro.
* That is, a macro that is used to interpret the
* data in `hf`, usually defined just before including that file
* and undefined immediately afterwards.
*/
predicate hasXMacro(HeaderFile hf, string macroName) {
usesMacro(hf, macroName) and
forex(Include i | i.getIncludedFile() = hf |
defUndef(i.getFile(), macroName)
predicate hasXMacro(HeaderFile hf) {
exists(string macroName |
usesMacro(hf, macroName) and
forex(File f | f.getAnIncludedFile() = hf |
defUndef(f, macroName)
)
)
}
@@ -135,7 +137,7 @@ where not hf instanceof IncludeGuardedHeader
exists(UsingEntry ue | ue.getFile() = hf)
)
// Exclude files which look like they contain 'x-macros'
and not hasXMacro(hf, _)
and not hasXMacro(hf)
// Exclude files which are always #imported.
and not forex(Include i | i.getIncludedFile() = hf | i instanceof Import)
// Exclude files which are only included once.

11
cpp/ql/src/jsf/4.10 Classes/AV Rule 79.ql
View File

@@ -159,6 +159,17 @@ predicate unreleasedResource(Resource r, Expr acquire, File f, int acquireLine)
)
and f = acquire.getFile()
and acquireLine = acquire.getLocation().getStartLine()
// check that any destructor for this class has a block; if it doesn't,
// we must be missing information.
and forall(Class c, Destructor d |
r.getDeclaringType().isConstructedFrom*(c) and
d = c.getAMember() and
not d.isCompilerGenerated() and
not d.isDefaulted() and
not d.isDeleted() |
exists(d.getBlock())
)
}
predicate freedInSameMethod(Resource r, Expr acquire) {

20
cpp/ql/src/jsf/4.10 Classes/AV Rule 82.ql
View File

@@ -25,7 +25,7 @@ predicate pointerThis(Expr e) {
// `f(...)`
// (includes `this = ...`, where `=` is overloaded so a `FunctionCall`)
exists(FunctionCall fc | fc = e and callOnThis(fc) |
exists(fc.getTarget().getBlock()) implies returnsPointerThis(fc.getTarget())
returnsPointerThis(fc.getTarget())
) or
// `this = ...` (where `=` is not overloaded, so an `AssignExpr`)
@@ -38,22 +38,33 @@ predicate dereferenceThis(Expr e) {
// `f(...)`
// (includes `*this = ...`, where `=` is overloaded so a `FunctionCall`)
exists(FunctionCall fc | fc = e and callOnThis(fc) |
exists(fc.getTarget().getBlock()) implies returnsDereferenceThis(fc.getTarget())
returnsDereferenceThis(fc.getTarget())
) or
// `*this = ...` (where `=` is not overloaded, so an `AssignExpr`)
dereferenceThis(e.(AssignExpr).getLValue())
}
/**
* Holds if all `return` statements in `f` return `this`, possibly indirectly.
* This includes functions whose body is not in the database.
*/
predicate returnsPointerThis(Function f) {
forex(ReturnStmt s | s.getEnclosingFunction() = f |
f.getType().getUnspecifiedType() instanceof PointerType and
forall(ReturnStmt s | s.getEnclosingFunction() = f and reachable(s) |
// `return this`
pointerThis(s.getExpr())
)
}
/**
* Holds if all `return` statements in `f` return a reference to `*this`,
* possibly indirectly. This includes functions whose body is not in the
* database.
*/
predicate returnsDereferenceThis(Function f) {
forex(ReturnStmt s | s.getEnclosingFunction() = f |
f.getType().getUnspecifiedType() instanceof ReferenceType and
forall(ReturnStmt s | s.getEnclosingFunction() = f and reachable(s) |
// `return *this`
dereferenceThis(s.getExpr())
)
@@ -72,7 +83,6 @@ predicate assignOperatorWithWrongType(Operator op, string msg) {
predicate assignOperatorWithWrongResult(Operator op, string msg) {
op.hasName("operator=")
and not returnsDereferenceThis(op)
and exists(op.getBlock())
and not op.getType() instanceof VoidType
and not assignOperatorWithWrongType(op, _)
and msg = "Assignment operator in class " + op.getDeclaringType().getName() + " does not return a reference to *this."

22
cpp/ql/src/jsf/4.13 Functions/AV Rule 114.ql
View File

@@ -3,7 +3,7 @@
* @description All functions that are not void should return a value on every exit path.
* @kind problem
* @problem.severity error
* @precision medium
* @precision high
* @id cpp/missing-return
* @tags reliability
* readability
@@ -19,7 +19,11 @@ import cpp
predicate functionsMissingReturnStmt(Function f, ControlFlowNode blame) {
f.fromSource() and
not f.getType().getUnderlyingType().getUnspecifiedType() instanceof VoidType and
exists(Type returnType |
returnType = f.getType().getUnderlyingType().getUnspecifiedType() and
not returnType instanceof VoidType and
not returnType instanceof TemplateParameter
) and
exists(ReturnStmt s | f.getAPredecessor() = s | blame = s.getAPredecessor())}
/* If a function has a value-carrying return statement, but the extractor hit a snag
@@ -32,13 +36,11 @@ predicate functionImperfectlyExtracted(Function f) {
exists(ErrorExpr ee | ee.getEnclosingFunction() = f)
}
from Stmt stmt, string msg
from Stmt stmt, string msg, Function f, ControlFlowNode blame
where
exists(Function f, ControlFlowNode blame |
functionsMissingReturnStmt(f, blame) and
reachable(blame) and
not functionImperfectlyExtracted(f) and
(blame = stmt or blame.(Expr).getEnclosingStmt() = stmt) and
msg = "Function " + f.getName() + " should return a value of type " + f.getType().getName() + " but does not return a value here"
)
functionsMissingReturnStmt(f, blame) and
reachable(blame) and
not functionImperfectlyExtracted(f) and
(blame = stmt or blame.(Expr).getEnclosingStmt() = stmt) and
msg = "Function " + f.getName() + " should return a value of type " + f.getType().getName() + " but does not return a value here"
select stmt, msg

4
cpp/ql/src/semmle/code/cpp/Type.qll
View File

@@ -601,6 +601,10 @@ class VoidType extends BuiltInType {
/**
* The C/C++ wide character type.
*
* Note that on some platforms `wchar_t` doesn't exist as a built-in
* type but a typedef is provided. Consider using the `Wchar_t` QL
* class to include these types.
*/
class WideCharType extends IntegralType {

94
cpp/ql/src/semmle/code/cpp/commons/Buffer.qll
View File

@@ -1,27 +1,5 @@
import cpp
/**
* Holds if `sizeof(s)` occurs as part of the parameter of a dynamic
* memory allocation (`malloc`, `realloc`, etc.), except if `sizeof(s)`
* only ever occurs as the immediate parameter to allocations.
*
* For example, holds for `s` if it occurs as
* ```
* malloc(sizeof(s) + 100 * sizeof(char))
* ```
* but not if it only ever occurs as
* ```
* malloc(sizeof(s))
* ```
*/
private predicate isDynamicallyAllocatedWithDifferentSize(Class s) {
exists(SizeofTypeOperator sof |
sof.getTypeOperand().getUnspecifiedType() = s |
// Check all ancestor nodes except the immediate parent for
// allocations.
isStdLibAllocationExpr(sof.getParent().(Expr).getParent+())
)
}
import semmle.code.cpp.dataflow.DataFlow
/**
* Holds if `v` is a member variable of `c` that looks like it might be variable sized in practice. For
@@ -33,20 +11,46 @@ private predicate isDynamicallyAllocatedWithDifferentSize(Class s) {
* };
* ```
* This requires that `v` is an array of size 0 or 1, and `v` is the last member of `c`. In addition,
* there must be at least one instance where a `c` pointer is allocated with additional space.
* there must be at least one instance where a `c` pointer is allocated with additional space. For
* example, holds for `c` if it occurs as
* ```
* malloc(sizeof(c) + 100 * sizeof(char))
* ```
* but not if it only ever occurs as
* ```
* malloc(sizeof(c))
* ```
*/
predicate memberMayBeVarSize(Class c, MemberVariable v) {
exists(int i |
// `v` is the last field in `c`
i = max(int j | c.getCanonicalMember(j) instanceof Field | j) and
v = c.getCanonicalMember(i) and
v.getType().getUnspecifiedType().(ArrayType).getSize() <= 1
) and
isDynamicallyAllocatedWithDifferentSize(c)
// v is an array of size at most 1
v.getType().getUnspecifiedType().(ArrayType).getArraySize() <= 1
) and (
exists(SizeofOperator so |
// `sizeof(c)` is taken
so.(SizeofTypeOperator).getTypeOperand().getUnspecifiedType() = c or
so.(SizeofExprOperator).getExprOperand().getType().getUnspecifiedType() = c |
// arithmetic is performed on the result
so.getParent*() instanceof AddExpr
) or exists(AddressOfExpr aoe |
// `&(c.v)` is taken
aoe.getAddressable() = v
) or exists(BuiltInOperationOffsetOf oo |
// `offsetof(c, v)` using a builtin
oo.getAChild().(VariableAccess).getTarget() = v
)
)
}
/**
* Get the size in bytes of the buffer pointed to by an expression (if this can be determined).
*/
language[monotonicAggregates]
int getBufferSize(Expr bufferExpr, Element why) {
exists(Variable bufferVar | bufferVar = bufferExpr.(VariableAccess).getTarget() |
(
@@ -58,6 +62,10 @@ int getBufferSize(Expr bufferExpr, Element why) {
// buffer is an initialized array
// e.g. int buffer[] = {1, 2, 3};
why = bufferVar.getInitializer().getExpr() and
(
why instanceof AggregateLiteral or
why instanceof StringLiteral
) and
result = why.(Expr).getType().(ArrayType).getSize() and
not exists(bufferVar.getType().getUnspecifiedType().(ArrayType).getSize())
) or exists(Class parentClass, VariableAccess parentPtr |
@@ -71,19 +79,27 @@ int getBufferSize(Expr bufferExpr, Element why) {
bufferVar.getType().getSize() -
parentClass.getSize()
)
) or exists(Expr def |
// buffer is assigned with an allocation
definitionUsePair(_, def, bufferExpr) and
exprDefinition(_, def, why) and
isFixedSizeAllocationExpr(why, result)
) or exists(Expr def, Expr e, Element why2 |
// buffer is assigned with another buffer
definitionUsePair(_, def, bufferExpr) and
exprDefinition(_, def, e) and
result = getBufferSize(e, why2) and
(
) or (
// buffer is a fixed size dynamic allocation
isFixedSizeAllocationExpr(bufferExpr, result) and
why = bufferExpr
) or exists(DataFlow::ExprNode bufferExprNode |
// dataflow (all sources must be the same size)
bufferExprNode = DataFlow::exprNode(bufferExpr) and
result = min(Expr def |
DataFlow::localFlowStep(DataFlow::exprNode(def), bufferExprNode) |
getBufferSize(def, _)
) and result = max(Expr def |
DataFlow::localFlowStep(DataFlow::exprNode(def), bufferExprNode) |
getBufferSize(def, _)
) and
// find reason
exists(Expr def |
DataFlow::localFlowStep(DataFlow::exprNode(def), bufferExprNode) |
why = def or
why = why2
exists(getBufferSize(def, why))
)
) or exists(Type bufferType |
// buffer is the address of a variable

22
cpp/ql/src/semmle/code/cpp/commons/CommonType.qll
View File

@@ -1,7 +1,7 @@
import semmle.code.cpp.Type
/**
* The C/C++ char* type.
* The C/C++ `char*` type.
*/
class CharPointerType extends PointerType {
@@ -10,7 +10,7 @@ class CharPointerType extends PointerType {
}
/**
* The C/C++ int* type.
* The C/C++ `int*` type.
*/
class IntPointerType extends PointerType {
@@ -20,7 +20,7 @@ class IntPointerType extends PointerType {
/**
* The C/C++ void* type.
* The C/C++ `void*` type.
*/
class VoidPointerType extends PointerType {
@@ -29,7 +29,7 @@ class VoidPointerType extends PointerType {
}
/**
* The C/C++ size_t type.
* The C/C++ `size_t` type.
*/
class Size_t extends Type {
Size_t() {
@@ -39,7 +39,7 @@ class Size_t extends Type {
}
/**
* The C/C++ ssize_t type.
* The C/C++ `ssize_t` type.
*/
class Ssize_t extends Type {
Ssize_t() {
@@ -49,7 +49,7 @@ class Ssize_t extends Type {
}
/**
* The C/C++ ptrdiff_t type.
* The C/C++ `ptrdiff_t` type.
*/
class Ptrdiff_t extends Type {
Ptrdiff_t() {
@@ -59,7 +59,7 @@ class Ptrdiff_t extends Type {
}
/**
* The C/C++ intmax_t type.
* The C/C++ `intmax_t` type.
*/
class Intmax_t extends Type {
Intmax_t() {
@@ -69,7 +69,7 @@ class Intmax_t extends Type {
}
/**
* The C/C++ uintmax_t type.
* The C/C++ `uintmax_t` type.
*/
class Uintmax_t extends Type {
Uintmax_t() {
@@ -79,7 +79,11 @@ class Uintmax_t extends Type {
}
/**
* The C/C++ wchar_t type.
* The C/C++ `wchar_t` type.
*
* Note that on some platforms `wchar_t` doesn't exist as a built-in
* type but a typedef is provided. This QL class includes both cases
* (see also `WideCharType`).
*/
class Wchar_t extends Type {
Wchar_t() {

79
cpp/ql/src/semmle/code/cpp/commons/Printf.qll
View File

@@ -220,6 +220,32 @@ class FormatLiteral extends Literal {
getUse().getTarget().(FormattingFunction).isWideCharDefault()
}
/**
* Gets the default character type expected for `%s` by this format literal. Typically
* `char` or `wchar_t`.
*/
Type getDefaultCharType() {
result = getUse().getTarget().(FormattingFunction).getDefaultCharType()
}
/**
* Gets the non-default character type expected for `%S` by this format literal. Typically
* `wchar_t` or `char`. On some snapshots there may be multiple results where we can't tell
* which is correct for a particular function.
*/
Type getNonDefaultCharType() {
result = getUse().getTarget().(FormattingFunction).getNonDefaultCharType()
}
/**
* Gets the wide character type for this format literal. This is usually `wchar_t`. On some
* snapshots there may be multiple results where we can't tell which is correct for a
* particular function.
*/
Type getWideCharType() {
result = getUse().getTarget().(FormattingFunction).getWideCharType()
}
/**
* Holds if this `FormatLiteral` is in a context that supports
* Microsoft rules and extensions.
@@ -629,7 +655,6 @@ class FormatLiteral extends Literal {
result = getConversionType2(n) or
result = getConversionType3(n) or
result = getConversionType4(n) or
result = getConversionType5(n) or
result = getConversionType6(n) or
result = getConversionType7(n) or
result = getConversionType8(n) or
@@ -696,33 +721,35 @@ class FormatLiteral extends Literal {
}
/**
* Gets the 'effective' string type character, that is, 's' (meaning a char string) or
* 'S' (meaning a wide string).
* - in the base case this is the same as the format type character.
* - for a `wprintf` or similar function call, the meanings are reversed.
* - the size prefixes 'l'/'w' (long) and 'h' (short) override the
* type character to effectively 'S' or 's' respectively.
* Gets the string type required by the nth conversion specifier.
* - in the base case this is the default for the formatting function
* (e.g. `char` for `printf`, `wchar_t` for `wprintf`).
* - the `%S` format character reverses wideness.
* - the size prefixes 'l'/'w' and 'h' override the type character
* to wide or single-byte characters respectively.
*/
private string getEffectiveStringConversionChar(int n) {
exists(string len, string conv | this.parseConvSpec(n, _, _, _, _, _, len, conv) and (conv = "s" or conv = "S") |
(len = "l" and result = "S") or
(len = "w" and result = "S") or
(len = "h" and result = "s") or
(len != "l" and len != "w" and len != "h" and (result = "s" or result = "S") and (if isWideCharDefault() then result != conv else result = conv))
)
}
private Type getConversionType4(int n) {
exists(string cnv, CharType t | cnv = this.getEffectiveStringConversionChar(n) |
cnv="s" and t = result.(PointerType).getBaseType()
and not t.isExplicitlySigned()
and not t.isExplicitlyUnsigned()
)
}
private Type getConversionType5(int n) {
exists(string cnv | cnv = this.getEffectiveStringConversionChar(n) |
cnv="S" and result.(PointerType).getBaseType().hasName("wchar_t")
exists(string len, string conv |
this.parseConvSpec(n, _, _, _, _, _, len, conv) and
(
(
(conv = "s" or conv = "S") and
len = "h" and
result.(PointerType).getBaseType() instanceof PlainCharType
) or (
(conv = "s" or conv = "S") and
(len = "l" or len = "w") and
result.(PointerType).getBaseType() = getWideCharType()
) or (
conv = "s" and
(len != "l" and len != "w" and len != "h") and
result.(PointerType).getBaseType() = getDefaultCharType()
) or (
conv = "S" and
(len != "l" and len != "w" and len != "h") and
result.(PointerType).getBaseType() = getNonDefaultCharType()
)
)
)
}

8
cpp/ql/src/semmle/code/cpp/controlflow/SSAUtils.qll
View File

@@ -124,11 +124,17 @@ cached library class SSAHelper extends int {
* Modern Compiler Implementation by Andrew Appel.
*/
private predicate frontier_phi_node(LocalScopeVariable v, BasicBlock b) {
exists(BasicBlock x | dominanceFrontier(x, b) and ssa_defn(v, _, x, _))
exists(BasicBlock x | dominanceFrontier(x, b) and ssa_defn_rec(v, x))
/* We can also eliminate those nodes where the variable is not live on any incoming edge */
and live_at_start_of_bb(v, b)
}
private predicate ssa_defn_rec(LocalScopeVariable v, BasicBlock b) {
phi_node(v, b)
or
variableUpdate(v, _, b, _)
}
/**
* Holds if `v` is defined, for the purpose of SSA, at `node`, which is at
* position `index` in block `b`. This includes definitions from phi nodes.

17
cpp/ql/src/semmle/code/cpp/controlflow/internal/PrimitiveBasicBlocks.qll
View File

@@ -31,8 +31,11 @@ private cached module Cached {
// or the node's predecessor has more than one successor,
// then the node is the start of a new primitive basic block.
or
strictcount (Node pred, Node other
| successors_extended(pred,node) and successors_extended(pred,other)) > 1
strictcount(Node pred | successors_extended(pred, node)) > 1
or
exists(ControlFlowNode pred | successors_extended(pred, node) |
strictcount(ControlFlowNode other | successors_extended(pred, other)) > 1
)
// If the node has zero predecessors then it is the start of
// a BB. However, the C++ AST contains many nodes with zero
@@ -63,8 +66,14 @@ private cached module Cached {
/** Holds if `node` is the `pos`th control-flow node in primitive basic block `bb`. */
cached
predicate primitive_basic_block_member(Node node, PrimitiveBasicBlock bb, int pos) {
pos = getMemberIndex(node) and
member_step*(bb, node)
primitive_basic_block_entry_node(bb) and
(
pos = 0 and
node = bb
or
pos = getMemberIndex(node) and
member_step+(bb, node)
)
}
/** Gets the number of control-flow nodes in the primitive basic block `bb`. */

2950
cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl.qll
View File

File diff suppressed because it is too large Load Diff

2950
cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl2.qll
View File

File diff suppressed because it is too large Load Diff

2950
cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl3.qll
View File

File diff suppressed because it is too large Load Diff

2950
cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl4.qll
View File

File diff suppressed because it is too large Load Diff

60
cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImplCommon.qll
View File

@@ -1,4 +1,3 @@
import DataFlowUtil
private import DataFlowPrivate
private import DataFlowDispatch
@@ -55,9 +54,7 @@ private module ImplCommon {
*/
cached
predicate parameterValueFlowsThrough(ParameterNode p) {
exists(ReturnNode ret |
parameterValueFlow(p, ret)
)
exists(ReturnNode ret | parameterValueFlow(p, ret))
}
/**
@@ -112,16 +109,19 @@ private module ImplCommon {
storeViaSideEffect(node1, f, node2) or
storeReturn(node1, f, node2)
}
private predicate storeViaSideEffect(Node node1, Content f, PostUpdateNode node2) {
storeStep(node1, f, node2) and readStep(_, f, _) or
storeStep(node1, f, node2) and readStep(_, f, _)
or
exists(Call call, int i1, int i2 |
setterCall(call, i1, i2, f) and
node1.(ArgumentNode).argumentOf(call, i1) and
node2.getPreUpdateNode().(ArgumentNode).argumentOf(call, i2) and
compatibleTypes(node1.getTypeBound(), f.getType()) and
compatibleTypes(node2.getTypeBound(), f.getDeclaringType())
compatibleTypes(node2.getTypeBound(), f.getContainerType())
)
}
pragma[nomagic]
private predicate setterInParam(ParameterNode p1, Content f, ParameterNode p2) {
exists(Node n1, PostUpdateNode n2 |
@@ -131,6 +131,7 @@ private module ImplCommon {
p1 != p2
)
}
pragma[nomagic]
private predicate setterCall(Call call, int i1, int i2, Content f) {
exists(Callable callable, ParameterNode p1, ParameterNode p2 |
@@ -140,6 +141,7 @@ private module ImplCommon {
p2.isParameterOf(callable, i2)
)
}
private predicate storeReturn(Node node1, Content f, Node node2) {
exists(ParameterNode p, ArgumentNode arg |
arg = node1 and
@@ -147,9 +149,10 @@ private module ImplCommon {
setterReturn(p, f) and
arg.argumentOf(node2.asExpr(), _) and
compatibleTypes(node1.getTypeBound(), f.getType()) and
compatibleTypes(node2.getTypeBound(), f.getDeclaringType())
compatibleTypes(node2.getTypeBound(), f.getContainerType())
)
}
private predicate setterReturn(ParameterNode p, Content f) {
exists(Node n1, Node n2, ReturnNode ret |
parameterValueFlow(p, n1) and
@@ -164,16 +167,18 @@ private module ImplCommon {
*/
cached
predicate read(Node node1, Content f, Node node2) {
readStep(node1, f, node2) and storeStep(_, f, _) or
readStep(node1, f, node2) and storeStep(_, f, _)
or
exists(ParameterNode p, ArgumentNode arg |
arg = node1 and
viableParamArg(p, arg) and
getter(p, f) and
arg.argumentOf(node2.asExpr(), _) and
compatibleTypes(node1.getTypeBound(), f.getDeclaringType()) and
compatibleTypes(node1.getTypeBound(), f.getContainerType()) and
compatibleTypes(node2.getTypeBound(), f.getType())
)
}
private predicate getter(ParameterNode p, Content f) {
exists(Node n1, Node n2, ReturnNode ret |
parameterValueFlow(p, n1) and
@@ -196,9 +201,7 @@ private module ImplCommon {
* expression that reaches a `this` parameter.
*/
private predicate callHasInstanceArgument(Call call) {
exists(ArgumentNode arg |
arg.argumentOf(call, -1)
)
exists(ArgumentNode arg | arg.argumentOf(call, -1))
}
cached
@@ -207,9 +210,11 @@ private module ImplCommon {
TSpecificCall(Call call, int i, boolean emptyAp) {
reducedViableImplInCallContext(_, _, call) and
(emptyAp = true or emptyAp = false) and
(exists(call.getArgument(i))
or
i = -1 and callHasInstanceArgument(call))
(
exists(call.getArgument(i))
or
i = -1 and callHasInstanceArgument(call)
)
} or
TSomeCall(ParameterNode p, boolean emptyAp) { emptyAp = true or emptyAp = false } or
TReturn(Method m, MethodAccess ma) { reducedViableImplInReturn(m, ma) }
@@ -232,19 +237,22 @@ import ImplCommon
* this dispatch target of `ma` implies a reduced set of dispatch origins
* to which data may flow if it should reach a `return` statement.
*/
abstract class CallContext extends TCallContext {
abstract string toString();
}
abstract class CallContext extends TCallContext { abstract string toString(); }
class CallContextAny extends CallContext, TAnyCallContext {
override string toString() { result = "CcAny" }
}
abstract class CallContextCall extends CallContext { }
class CallContextSpecificCall extends CallContextCall, TSpecificCall {
override string toString() { result = "CcCall" }
}
class CallContextSomeCall extends CallContextCall, TSomeCall {
override string toString() { result = "CcSomeCall" }
}
class CallContextReturn extends CallContext, TReturn {
override string toString() { result = "CcReturn" }
}
@@ -263,12 +271,14 @@ predicate resolveReturn(CallContext cc, Callable callable, Call call) {
bindingset[call, cc]
Callable resolveCall(Call call, CallContext cc) {
exists(Call ctx | cc = TSpecificCall(ctx, _, _) |
if reducedViableImplInCallContext(call, _, ctx) then
result = prunedViableImplInCallContext(call, ctx)
else
result = viableCallable(call)
) or
result = viableCallable(call) and cc instanceof CallContextSomeCall or
result = viableCallable(call) and cc instanceof CallContextAny or
if reducedViableImplInCallContext(call, _, ctx)
then result = prunedViableImplInCallContext(call, ctx)
else result = viableCallable(call)
)
or
result = viableCallable(call) and cc instanceof CallContextSomeCall
or
result = viableCallable(call) and cc instanceof CallContextAny
or
result = viableCallable(call) and cc instanceof CallContextReturn
}

22
cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowPrivate.qll
View File

@@ -69,18 +69,9 @@ class Content extends TContent {
path = "" and sl = 0 and sc = 0 and el = 0 and ec = 0
}
/** Gets the type of the object containing this content. */
abstract RefType getDeclaringType();
abstract RefType getContainerType();
/** Gets the type of this content. */
abstract Type getType();
/**
* Holds if this content may contain an object of the same type as the one
* that contains this content, and if this fact should be used to compress
* access paths.
*
* Examples include the tail pointer in a linked list or the left and right
* pointers in a binary tree.
*/
predicate isSelfRef() { none() }
}
private class FieldContent extends Content, TFieldContent {
Field f;
@@ -90,17 +81,17 @@ private class FieldContent extends Content, TFieldContent {
override predicate hasLocationInfo(string path, int sl, int sc, int el, int ec) {
f.getLocation().hasLocationInfo(path, sl, sc, el, ec)
}
override RefType getDeclaringType() { result = f.getDeclaringType() }
override RefType getContainerType() { result = f.getDeclaringType() }
override Type getType() { result = f.getType() }
}
private class CollectionContent extends Content, TCollectionContent {
override string toString() { result = "collection" }
override RefType getDeclaringType() { none() }
override RefType getContainerType() { none() }
override Type getType() { none() }
}
private class ArrayContent extends Content, TArrayContent {
override string toString() { result = "array" }
override RefType getDeclaringType() { none() }
override RefType getContainerType() { none() }
override Type getType() { none() }
}
@@ -132,6 +123,11 @@ RefType getErasedRepr(Type t) {
result instanceof VoidType // stub implementation
}
/** Gets a string representation of a type returned by `getErasedRepr`. */
string ppReprType(Type t) {
result = t.toString()
}
/**
* Holds if `t1` and `t2` are compatible, that is, whether data can flow from
* a node of type `t1` to a node of type `t2`.

7
cpp/ql/src/semmle/code/cpp/headers/MultipleInclusion.qll
View File

@@ -114,8 +114,11 @@ pragma[noopt] predicate correctIncludeGuard(HeaderFile hf, PreprocessorDirective
*/
predicate startsWithIfndef(HeaderFile hf, PreprocessorDirective ifndef, string macroName) {
ifndefDirective(ifndef, macroName) and
ifndef.getFile() = hf and
ifndef.getLocation().getStartLine() = min(int l | includeGuardRelevantLine(hf, l))
exists(Location loc |
loc = ifndef.getLocation() and
loc.getFile() = hf and
loc.getStartLine() = min(int l | includeGuardRelevantLine(hf, l))
)
}
private predicate endifLocation(PreprocessorEndif endif, File f, int line) {

36
cpp/ql/src/semmle/code/cpp/models/implementations/Printf.qll
View File

@@ -12,7 +12,8 @@ class Printf extends FormattingFunction {
hasGlobalName("wprintf") or
hasGlobalName("wprintf_s") or
hasGlobalName("g_printf")
)
) and
not exists(getDefinition().getFile().getRelativePath())
}
override int getFormatParameterIndex() { result=0 }
@@ -26,7 +27,15 @@ class Printf extends FormattingFunction {
* The standard functions `fprintf`, `fwprintf` and their glib variants.
*/
class Fprintf extends FormattingFunction {
Fprintf() { this instanceof TopLevelFunction and (hasGlobalName("fprintf") or hasGlobalName("fwprintf") or hasGlobalName("g_fprintf"))}
Fprintf() {
this instanceof TopLevelFunction and
(
hasGlobalName("fprintf") or
hasGlobalName("fwprintf") or
hasGlobalName("g_fprintf")
) and
not exists(getDefinition().getFile().getRelativePath())
}
override int getFormatParameterIndex() { result=1 }
override predicate isWideCharDefault() { hasGlobalName("fwprintf") }
@@ -47,7 +56,8 @@ class Sprintf extends FormattingFunction {
hasGlobalName("g_strdup_printf") or
hasGlobalName("g_sprintf") or
hasGlobalName("__builtin___sprintf_chk")
)
) and
not exists(getDefinition().getFile().getRelativePath())
}
override predicate isWideCharDefault() {
@@ -100,7 +110,8 @@ class Snprintf extends FormattingFunction {
or hasGlobalName("g_snprintf")
or hasGlobalName("wnsprintf")
or hasGlobalName("__builtin___snprintf_chk")
)
) and
not exists(getDefinition().getFile().getRelativePath())
}
override int getFormatParameterIndex() {
@@ -133,10 +144,13 @@ class Snprintf extends FormattingFunction {
* in the buffer.
*/
predicate returnsFullFormatLength() {
hasGlobalName("snprintf") or
hasGlobalName("g_snprintf") or
hasGlobalName("__builtin___snprintf_chk") or
hasGlobalName("snprintf_s")
(
hasGlobalName("snprintf") or
hasGlobalName("g_snprintf") or
hasGlobalName("__builtin___snprintf_chk") or
hasGlobalName("snprintf_s")
) and
not exists(getDefinition().getFile().getRelativePath())
}
override int getSizeParameterIndex() {
@@ -158,7 +172,8 @@ class StringCchPrintf extends FormattingFunction {
or hasGlobalName("StringCbPrintfEx")
or hasGlobalName("StringCbPrintf_l")
or hasGlobalName("StringCbPrintf_lEx")
)
) and
not exists(getDefinition().getFile().getRelativePath())
}
override int getFormatParameterIndex() {
@@ -187,7 +202,8 @@ class Syslog extends FormattingFunction {
Syslog() {
this instanceof TopLevelFunction and (
hasGlobalName("syslog")
)
) and
not exists(getDefinition().getFile().getRelativePath())
}
override int getFormatParameterIndex() { result=1 }

69
cpp/ql/src/semmle/code/cpp/models/interfaces/FormattingFunction.qll
View File

@@ -7,6 +7,38 @@
*/
import semmle.code.cpp.Function
private Type stripTopLevelSpecifiersOnly(Type t) {
(
result = stripTopLevelSpecifiersOnly(t.(SpecifiedType).getBaseType())
) or (
result = t and
not t instanceof SpecifiedType
)
}
/**
* A type that is used as a format string by any formatting function.
*/
Type getAFormatterWideType() {
exists(FormattingFunction ff |
result = stripTopLevelSpecifiersOnly(ff.getDefaultCharType()) and
result.getSize() != 1
)
}
/**
* A type that is used as a format string by any formatting function, or `wchar_t` if
* there is none.
*/
private Type getAFormatterWideTypeOrDefault() {
result = getAFormatterWideType() or
(
not exists(getAFormatterWideType()) and
result instanceof Wchar_t
)
}
/**
* A standard library function that uses a `printf`-like formatting string.
*/
@@ -20,6 +52,43 @@ abstract class FormattingFunction extends Function {
*/
predicate isWideCharDefault() { none() }
/**
* Gets the default character type expected for `%s` by this function. Typically
* `char` or `wchar_t`.
*/
Type getDefaultCharType() {
result = stripTopLevelSpecifiersOnly(getParameter(getFormatParameterIndex()).getType().
getUnderlyingType().(PointerType).getBaseType())
}
/**
* Gets the non-default character type expected for `%S` by this function. Typically
* `wchar_t` or `char`. On some snapshots there may be multiple results where we can't tell
* which is correct for a particular function.
*/
Type getNonDefaultCharType() {
(
getDefaultCharType().getSize() = 1 and
result = getAFormatterWideTypeOrDefault()
) or (
getDefaultCharType().getSize() > 1 and
result instanceof PlainCharType
)
}
/**
* Gets the wide character type for this function. This is usually `wchar_t`. On some
* snapshots there may be multiple results where we can't tell which is correct for a
* particular function.
*/
Type getWideCharType() {
(
result = getDefaultCharType() or
result = getNonDefaultCharType()
) and
result.getSize() > 1
}
/**
* Gets the position at which the output parameter, if any, occurs.
*/

351
cpp/ql/src/semmle/code/cpp/rangeanalysis/SimpleRangeAnalysis.qll
View File

@@ -44,6 +44,7 @@
import cpp
private import RangeAnalysisUtils
import RangeSSA
import SimpleRangeAnalysisCached
/**
* This fixed set of lower bounds is used when the lower bounds of an
@@ -292,6 +293,61 @@ predicate analyzableDef(RangeSsaDefinition def, LocalScopeVariable v) {
assignmentDef(def, v, _) or defDependsOnDef(def, v, _, _)
}
/**
* Computes a normal form of `x` where -0.0 has changed to +0.0. This can be
* needed on the lesser side of a floating-point comparison or on both sides of
* a floating point equality because QL does not follow IEEE in floating-point
* comparisons but instead defines -0.0 to be less than and distinct from 0.0.
*/
bindingset[x]
private float normalizeFloatUp(float x) {
result = x + 0.0
}
/**
* Computes `x + y`, rounded towards +Inf. This is the general case where both
* `x` and `y` may be large numbers.
*/
bindingset[x, y]
private float addRoundingUp(float x, float y) {
if normalizeFloatUp((x + y) - x) < y or normalizeFloatUp((x + y) - y) < x
then result = (x + y).nextUp()
else result = (x + y)
}
/**
* Computes `x + y`, rounded towards -Inf. This is the general case where both
* `x` and `y` may be large numbers.
*/
bindingset[x, y]
private float addRoundingDown(float x, float y) {
if (x + y) - x > normalizeFloatUp(y) or (x + y) - y > normalizeFloatUp(x)
then result = (x + y).nextDown()
else result = (x + y)
}
/**
* Computes `x + small`, rounded towards +Inf, where `small` is a small
* constant.
*/
bindingset[x, small]
private float addRoundingUpSmall(float x, float small) {
if (x + small) - x < small
then result = (x + small).nextUp()
else result = (x + small)
}
/**
* Computes `x + small`, rounded towards -Inf, where `small` is a small
* constant.
*/
bindingset[x, small]
private float addRoundingDownSmall(float x, float small) {
if (x + small) - x > small
then result = (x + small).nextDown()
else result = (x + small)
}
/**
* Gets the truncated lower bounds of the fully converted expression.
*/
@@ -406,27 +462,6 @@ deprecated predicate negative_overflow(Expr expr) {
exprMightOverflowNegatively(expr)
}
/**
* Holds if the expression might overflow negatively. This predicate
* does not consider the possibility that the expression might overflow
* due to a conversion.
*/
cached
predicate exprMightOverflowNegatively(Expr expr) {
getLowerBoundsImpl(expr) < exprMinVal(expr)
}
/**
* Holds if the expression might overflow negatively. Conversions
* are also taken into account. For example the expression
* `(int16)(x+y)` might overflow due to the `(int16)` cast, rather than
* due to the addition.
*/
cached
predicate convertedExprMightOverflowNegatively(Expr expr) {
exprMightOverflowNegatively(expr) or
convertedExprMightOverflowNegatively(expr.getConversion())
}
/**
* Holds if the expression might overflow positively. This predicate
@@ -439,39 +474,6 @@ deprecated predicate positive_overflow(Expr expr) {
exprMightOverflowPositively(expr)
}
/**
* Holds if the expression might overflow positively. This predicate
* does not consider the possibility that the expression might overflow
* due to a conversion.
*/
cached
predicate exprMightOverflowPositively(Expr expr) {
getUpperBoundsImpl(expr) > exprMaxVal(expr)
}
/**
* Holds if the expression might overflow positively. Conversions
* are also taken into account. For example the expression
* `(int16)(x+y)` might overflow due to the `(int16)` cast, rather than
* due to the addition.
*/
cached
predicate convertedExprMightOverflowPositively(Expr expr) {
exprMightOverflowPositively(expr) or
convertedExprMightOverflowPositively(expr.getConversion())
}
/**
* Holds if the expression might overflow (either positively or
* negatively). The possibility that the expression might overflow
* due to an implicit or explicit cast is also considered.
*/
cached
predicate convertedExprMightOverflow(Expr expr) {
convertedExprMightOverflowNegatively(expr) or
convertedExprMightOverflowPositively(expr)
}
/** Only to be called by `getTruncatedLowerBounds`. */
private
float getLowerBoundsImpl(Expr expr) {
@@ -523,13 +525,13 @@ float getLowerBoundsImpl(Expr expr) {
| expr = addExpr and
xLow = getFullyConvertedLowerBounds(addExpr.getLeftOperand()) and
yLow = getFullyConvertedLowerBounds(addExpr.getRightOperand()) and
result = xLow+yLow)
result = addRoundingDown(xLow, yLow))
or
exists (SubExpr subExpr, float xLow, float yHigh
| expr = subExpr and
xLow = getFullyConvertedLowerBounds(subExpr.getLeftOperand()) and
yHigh = getFullyConvertedUpperBounds(subExpr.getRightOperand()) and
result = xLow-yHigh)
result = addRoundingDown(xLow, -yHigh))
or
exists (PrefixIncrExpr incrExpr, float xLow
| expr = incrExpr and
@@ -539,7 +541,7 @@ float getLowerBoundsImpl(Expr expr) {
exists (PrefixDecrExpr decrExpr, float xLow
| expr = decrExpr and
xLow = getFullyConvertedLowerBounds(decrExpr.getOperand()) and
result = xLow-1)
result = addRoundingDownSmall(xLow, -1))
or
// `PostfixIncrExpr` and `PostfixDecrExpr` return the value of their
// operand. The incrementing/decrementing behavior is handled in
@@ -645,18 +647,18 @@ float getUpperBoundsImpl(Expr expr) {
| expr = addExpr and
xHigh = getFullyConvertedUpperBounds(addExpr.getLeftOperand()) and
yHigh = getFullyConvertedUpperBounds(addExpr.getRightOperand()) and
result = xHigh+yHigh)
result = addRoundingUp(xHigh, yHigh))
or
exists (SubExpr subExpr, float xHigh, float yLow
| expr = subExpr and
xHigh = getFullyConvertedUpperBounds(subExpr.getLeftOperand()) and
yLow = getFullyConvertedLowerBounds(subExpr.getRightOperand()) and
result = xHigh-yLow)
result = addRoundingUp(xHigh, -yLow))
or
exists (PrefixIncrExpr incrExpr, float xHigh
| expr = incrExpr and
xHigh = getFullyConvertedUpperBounds(incrExpr.getOperand()) and
result = xHigh+1)
result = addRoundingUpSmall(xHigh, 1))
or
exists (PrefixDecrExpr decrExpr, float xHigh
| expr = decrExpr and
@@ -849,7 +851,7 @@ float getDefLowerBoundsImpl(RangeSsaDefinition def, LocalScopeVariable v) {
assignAdd.getLValue() = nextDef.getAUse(v) and
lhsLB = getDefLowerBounds(nextDef, v) and
rhsLB = getFullyConvertedLowerBounds(assignAdd.getRValue()) and
result = lhsLB + rhsLB)
result = addRoundingDown(lhsLB, rhsLB))
or
exists (
AssignSubExpr assignSub, RangeSsaDefinition nextDef, float lhsLB, float rhsUB
@@ -857,7 +859,7 @@ float getDefLowerBoundsImpl(RangeSsaDefinition def, LocalScopeVariable v) {
assignSub.getLValue() = nextDef.getAUse(v) and
lhsLB = getDefLowerBounds(nextDef, v) and
rhsUB = getFullyConvertedUpperBounds(assignSub.getRValue()) and
result = lhsLB - rhsUB)
result = addRoundingDown(lhsLB, -rhsUB))
or
exists (IncrementOperation incr, float newLB
| def = incr and
@@ -869,7 +871,7 @@ float getDefLowerBoundsImpl(RangeSsaDefinition def, LocalScopeVariable v) {
| def = decr and
decr.getOperand() = v.getAnAccess() and
newLB = getFullyConvertedLowerBounds(decr.getOperand()) and
result = newLB-1)
result = addRoundingDownSmall(newLB, -1))
or
// Phi nodes.
result = getPhiLowerBounds(v, def)
@@ -892,7 +894,7 @@ float getDefUpperBoundsImpl(RangeSsaDefinition def, LocalScopeVariable v) {
assignAdd.getLValue() = nextDef.getAUse(v) and
lhsUB = getDefUpperBounds(nextDef, v) and
rhsUB = getFullyConvertedUpperBounds(assignAdd.getRValue()) and
result = lhsUB + rhsUB)
result = addRoundingUp(lhsUB, rhsUB))
or
exists (
AssignSubExpr assignSub, RangeSsaDefinition nextDef, float lhsUB, float rhsLB
@@ -900,13 +902,13 @@ float getDefUpperBoundsImpl(RangeSsaDefinition def, LocalScopeVariable v) {
assignSub.getLValue() = nextDef.getAUse(v) and
lhsUB = getDefUpperBounds(nextDef, v) and
rhsLB = getFullyConvertedLowerBounds(assignSub.getRValue()) and
result = lhsUB - rhsLB)
result = addRoundingUp(lhsUB, -rhsLB))
or
exists (IncrementOperation incr, float newUB
| def = incr and
incr.getOperand() = v.getAnAccess() and
newUB = getFullyConvertedUpperBounds(incr.getOperand()) and
result = newUB+1)
result = addRoundingUpSmall(newUB, 1))
or
exists (DecrementOperation decr, float newUB
| def = decr and
@@ -921,28 +923,6 @@ float getDefUpperBoundsImpl(RangeSsaDefinition def, LocalScopeVariable v) {
unanalyzableDefBounds(def, v, _, result)
}
/** Holds if the definition might overflow negatively. */
cached
predicate defMightOverflowNegatively(RangeSsaDefinition def, LocalScopeVariable v) {
getDefLowerBoundsImpl(def, v) < varMinVal(v)
}
/** Holds if the definition might overflow positively. */
cached
predicate defMightOverflowPositively(RangeSsaDefinition def, LocalScopeVariable v) {
getDefUpperBoundsImpl(def, v) > varMaxVal(v)
}
/**
* Holds if the definition might overflow (either positively or
* negatively).
*/
cached
predicate defMightOverflow(RangeSsaDefinition def, LocalScopeVariable v) {
defMightOverflowNegatively(def, v) or
defMightOverflowPositively(def, v)
}
/**
* Get the lower bounds for a `RangeSsaDefinition`. Most of the work is
* done by `getDefLowerBoundsImpl`, but this is where widening is applied
@@ -1133,63 +1113,142 @@ predicate exprTypeBounds(Expr expr, float boundValue, boolean isLowerBound) {
(isLowerBound = false and boundValue = exprMaxVal(expr.getFullyConverted()))
}
/**
* Gets the lower bound of the expression.
*
* Note: expressions in C/C++ are often implicitly or explicitly cast to a
* different result type. Such casts can cause the value of the expression
* to overflow or to be truncated. This predicate computes the lower bound
* of the expression without including the effect of the casts. To compute
* the lower bound of the expression after all the casts have been applied,
* call `lowerBound` like this:
*
* `lowerBound(expr.getFullyConverted())`
*/
cached
float lowerBound(Expr expr) {
// Combine the lower bounds returned by getTruncatedLowerBounds into a
// single minimum value.
result = min(float lb | lb = getTruncatedLowerBounds(expr) | lb)
}
private cached module SimpleRangeAnalysisCached {
/**
* Gets the lower bound of the expression.
*
* Note: expressions in C/C++ are often implicitly or explicitly cast to a
* different result type. Such casts can cause the value of the expression
* to overflow or to be truncated. This predicate computes the lower bound
* of the expression without including the effect of the casts. To compute
* the lower bound of the expression after all the casts have been applied,
* call `lowerBound` like this:
*
* `lowerBound(expr.getFullyConverted())`
*/
cached
float lowerBound(Expr expr) {
// Combine the lower bounds returned by getTruncatedLowerBounds into a
// single minimum value.
result = min(float lb | lb = getTruncatedLowerBounds(expr) | lb)
}
/**
* Gets the upper bound of the expression.
*
* Note: expressions in C/C++ are often implicitly or explicitly cast to a
* different result type. Such casts can cause the value of the expression
* to overflow or to be truncated. This predicate computes the upper bound
* of the expression without including the effect of the casts. To compute
* the upper bound of the expression after all the casts have been applied,
* call `upperBound` like this:
*
* `upperBound(expr.getFullyConverted())`
*/
cached
float upperBound(Expr expr) {
// Combine the upper bounds returned by getTruncatedUpperBounds into a
// single maximum value.
result = max(float ub | ub = getTruncatedUpperBounds(expr) | ub)
}
/**
* Holds if `expr` has a provably empty range. For example:
*
* 10 < expr and expr < 5
*
* The range of an expression can only be empty if it can never be
* executed. For example:
*
* if (10 < x) {
* if (x < 5) {
* // Unreachable code
* return x; // x has an empty range: 10 < x && x < 5
* }
* }
*/
cached
predicate exprWithEmptyRange(Expr expr) {
analyzableExpr(expr) and
(not exists(lowerBound(expr)) or
not exists(upperBound(expr)) or
lowerBound(expr) > upperBound(expr))
}
/**
* Gets the upper bound of the expression.
*
* Note: expressions in C/C++ are often implicitly or explicitly cast to a
* different result type. Such casts can cause the value of the expression
* to overflow or to be truncated. This predicate computes the upper bound
* of the expression without including the effect of the casts. To compute
* the upper bound of the expression after all the casts have been applied,
* call `upperBound` like this:
*
* `upperBound(expr.getFullyConverted())`
*/
cached
float upperBound(Expr expr) {
// Combine the upper bounds returned by getTruncatedUpperBounds into a
// single maximum value.
result = max(float ub | ub = getTruncatedUpperBounds(expr) | ub)
}
/**
* Holds if `expr` has a provably empty range. For example:
*
* 10 < expr and expr < 5
*
* The range of an expression can only be empty if it can never be
* executed. For example:
*
* if (10 < x) {
* if (x < 5) {
* // Unreachable code
* return x; // x has an empty range: 10 < x && x < 5
* }
* }
*/
cached
predicate exprWithEmptyRange(Expr expr) {
analyzableExpr(expr) and
(not exists(lowerBound(expr)) or
not exists(upperBound(expr)) or
lowerBound(expr) > upperBound(expr))
/** Holds if the definition might overflow negatively. */
cached
predicate defMightOverflowNegatively(RangeSsaDefinition def, LocalScopeVariable v) {
getDefLowerBoundsImpl(def, v) < varMinVal(v)
}
/** Holds if the definition might overflow positively. */
cached
predicate defMightOverflowPositively(RangeSsaDefinition def, LocalScopeVariable v) {
getDefUpperBoundsImpl(def, v) > varMaxVal(v)
}
/**
* Holds if the definition might overflow (either positively or
* negatively).
*/
cached
predicate defMightOverflow(RangeSsaDefinition def, LocalScopeVariable v) {
defMightOverflowNegatively(def, v) or
defMightOverflowPositively(def, v)
}
/**
* Holds if the expression might overflow negatively. This predicate
* does not consider the possibility that the expression might overflow
* due to a conversion.
*/
cached
predicate exprMightOverflowNegatively(Expr expr) {
getLowerBoundsImpl(expr) < exprMinVal(expr)
}
/**
* Holds if the expression might overflow negatively. Conversions
* are also taken into account. For example the expression
* `(int16)(x+y)` might overflow due to the `(int16)` cast, rather than
* due to the addition.
*/
cached
predicate convertedExprMightOverflowNegatively(Expr expr) {
exprMightOverflowNegatively(expr) or
convertedExprMightOverflowNegatively(expr.getConversion())
}
/**
* Holds if the expression might overflow positively. This predicate
* does not consider the possibility that the expression might overflow
* due to a conversion.
*/
cached
predicate exprMightOverflowPositively(Expr expr) {
getUpperBoundsImpl(expr) > exprMaxVal(expr)
}
/**
* Holds if the expression might overflow positively. Conversions
* are also taken into account. For example the expression
* `(int16)(x+y)` might overflow due to the `(int16)` cast, rather than
* due to the addition.
*/
cached
predicate convertedExprMightOverflowPositively(Expr expr) {
exprMightOverflowPositively(expr) or
convertedExprMightOverflowPositively(expr.getConversion())
}
/**
* Holds if the expression might overflow (either positively or
* negatively). The possibility that the expression might overflow
* due to an implicit or explicit cast is also considered.
*/
cached
predicate convertedExprMightOverflow(Expr expr) {
convertedExprMightOverflowNegatively(expr) or
convertedExprMightOverflowPositively(expr)
}
}

2
cpp/ql/src/semmle/code/cpp/valuenumbering/GlobalValueNumbering.qll
View File

@@ -312,7 +312,7 @@ private predicate analyzableLocalScopeVariable(VariableAccess access) {
strictcount (SsaDefinition def, Variable v | def.getAUse(v) = access | v) = 1 and
count (SsaDefinition def, Variable v
| def.getAUse(v) = access
| def.getDefiningValue(v)) <= 1 and
| def.getDefiningValue(v).getFullyConverted()) <= 1 and
not analyzableConst(access)
}

64
cpp/ql/test/library-tests/noexcept/copy_from_prototype/copy_from_prototype.expected
View File

@@ -1,32 +1,32 @@
| copy_from_prototype.cpp:3:7:3:7 | a | a<int>::a(a<int> &&) | copy_from_prototype.cpp:3:7:3:7 | a<int> | <no expr> |
| copy_from_prototype.cpp:3:7:3:7 | a | a<int>::a(const a<int> &) | copy_from_prototype.cpp:3:7:3:7 | a<int> | <no expr> |
| copy_from_prototype.cpp:3:7:3:7 | operator= | a<int>::operator=(a<int> &&) | copy_from_prototype.cpp:3:7:3:7 | a<int> | <no expr> |
| copy_from_prototype.cpp:3:7:3:7 | operator= | a<int>::operator=(const a<int> &) | copy_from_prototype.cpp:3:7:3:7 | a<int> | <no expr> |
| copy_from_prototype.cpp:4:26:4:26 | a | a<<unnamed>>::a<(unnamed)>() | copy_from_prototype.cpp:3:7:3:7 | a<<unnamed>> | 123 |
| copy_from_prototype.cpp:4:26:4:26 | a | a<int>::a() | copy_from_prototype.cpp:3:7:3:7 | a<int> | <no expr> |
| copy_from_prototype.cpp:4:26:4:26 | a | a<int>::a<(unnamed)>() | copy_from_prototype.cpp:3:7:3:7 | a<int> | <no expr> |
| copy_from_prototype.cpp:7:7:7:7 | b | b::b() | copy_from_prototype.cpp:7:7:7:7 | b | <no expr> |
| copy_from_prototype.cpp:7:7:7:7 | b | b::b(b &&) | copy_from_prototype.cpp:7:7:7:7 | b | <no expr> |
| copy_from_prototype.cpp:7:7:7:7 | b | b::b(const b &) | copy_from_prototype.cpp:7:7:7:7 | b | <no expr> |
| copy_from_prototype.cpp:7:7:7:7 | operator= | b::operator=(b &&) | copy_from_prototype.cpp:7:7:7:7 | b | <no expr> |
| copy_from_prototype.cpp:7:7:7:7 | operator= | b::operator=(const b &) | copy_from_prototype.cpp:7:7:7:7 | b | <no expr> |
| copy_from_prototype.cpp:13:7:13:7 | c | c<int>::c(c<int> &&) | copy_from_prototype.cpp:13:7:13:7 | c<int> | <no expr> |
| copy_from_prototype.cpp:13:7:13:7 | c | c<int>::c(const c<int> &) | copy_from_prototype.cpp:13:7:13:7 | c<int> | <no expr> |
| copy_from_prototype.cpp:13:7:13:7 | operator= | c<int>::operator=(c<int> &&) | copy_from_prototype.cpp:13:7:13:7 | c<int> | <no expr> |
| copy_from_prototype.cpp:13:7:13:7 | operator= | c<int>::operator=(const c<int> &) | copy_from_prototype.cpp:13:7:13:7 | c<int> | <no expr> |
| copy_from_prototype.cpp:14:26:14:26 | c | c<T>::c<(unnamed)>() | copy_from_prototype.cpp:13:7:13:7 | c<T> | Unknown literal |
| copy_from_prototype.cpp:14:26:14:26 | c | c<int>::c() | copy_from_prototype.cpp:13:7:13:7 | c<int> | <no expr> |
| copy_from_prototype.cpp:14:26:14:26 | c | c<int>::c<(unnamed)>() | copy_from_prototype.cpp:13:7:13:7 | c<int> | <no expr> |
| copy_from_prototype.cpp:17:7:17:7 | d | d::d() | copy_from_prototype.cpp:17:7:17:7 | d | <no expr> |
| copy_from_prototype.cpp:17:7:17:7 | d | d::d(const d &) | copy_from_prototype.cpp:17:7:17:7 | d | <no expr> |
| copy_from_prototype.cpp:17:7:17:7 | d | d::d(d &&) | copy_from_prototype.cpp:17:7:17:7 | d | <no expr> |
| copy_from_prototype.cpp:17:7:17:7 | operator= | d::operator=(const d &) | copy_from_prototype.cpp:17:7:17:7 | d | <no expr> |
| copy_from_prototype.cpp:17:7:17:7 | operator= | d::operator=(d &&) | copy_from_prototype.cpp:17:7:17:7 | d | <no expr> |
| copy_from_prototype.cpp:22:8:22:8 | e | e<int>::e(const e<int> &) | copy_from_prototype.cpp:22:8:22:8 | e<int> | <no expr> |
| copy_from_prototype.cpp:22:8:22:8 | e | e<int>::e(e<int> &&) | copy_from_prototype.cpp:22:8:22:8 | e<int> | <no expr> |
| copy_from_prototype.cpp:22:8:22:8 | operator= | e<int>::operator=(const e<int> &) | copy_from_prototype.cpp:22:8:22:8 | e<int> | <no expr> |
| copy_from_prototype.cpp:22:8:22:8 | operator= | e<int>::operator=(e<int> &&) | copy_from_prototype.cpp:22:8:22:8 | e<int> | <no expr> |
| copy_from_prototype.cpp:23:26:23:26 | e | e<T>::e<(unnamed)>() | copy_from_prototype.cpp:22:8:22:8 | e<T> | 456 |
| copy_from_prototype.cpp:26:35:26:43 | e | e<int>::e<(unnamed)>() | copy_from_prototype.cpp:22:8:22:8 | e<int> | 456 |
| file://:0:0:0:0 | operator= | __va_list_tag::operator=() | file://:0:0:0:0 | __va_list_tag | <none> |
| file://:0:0:0:0 | operator= | __va_list_tag::operator=() | file://:0:0:0:0 | __va_list_tag | <none> |
| copy_from_prototype.cpp:3:7:3:7 | a | a<int>::a(a<int> &&) -> void | copy_from_prototype.cpp:3:7:3:7 | a<int> | <no expr> |
| copy_from_prototype.cpp:3:7:3:7 | a | a<int>::a(const a<int> &) -> void | copy_from_prototype.cpp:3:7:3:7 | a<int> | <no expr> |
| copy_from_prototype.cpp:3:7:3:7 | operator= | a<int>::operator=(a<int> &&) -> a<int> & | copy_from_prototype.cpp:3:7:3:7 | a<int> | <no expr> |
| copy_from_prototype.cpp:3:7:3:7 | operator= | a<int>::operator=(const a<int> &) -> a<int> & | copy_from_prototype.cpp:3:7:3:7 | a<int> | <no expr> |
| copy_from_prototype.cpp:4:26:4:26 | a | a<<unnamed>>::a<(unnamed)>() -> void | copy_from_prototype.cpp:3:7:3:7 | a<<unnamed>> | 123 |
| copy_from_prototype.cpp:4:26:4:26 | a | a<int>::a() -> void | copy_from_prototype.cpp:3:7:3:7 | a<int> | <no expr> |
| copy_from_prototype.cpp:4:26:4:26 | a | a<int>::a<(unnamed)>() -> void | copy_from_prototype.cpp:3:7:3:7 | a<int> | <no expr> |
| copy_from_prototype.cpp:7:7:7:7 | b | b::b() -> void | copy_from_prototype.cpp:7:7:7:7 | b | <no expr> |
| copy_from_prototype.cpp:7:7:7:7 | b | b::b(b &&) -> void | copy_from_prototype.cpp:7:7:7:7 | b | <no expr> |
| copy_from_prototype.cpp:7:7:7:7 | b | b::b(const b &) -> void | copy_from_prototype.cpp:7:7:7:7 | b | <no expr> |
| copy_from_prototype.cpp:7:7:7:7 | operator= | b::operator=(b &&) -> b & | copy_from_prototype.cpp:7:7:7:7 | b | <no expr> |
| copy_from_prototype.cpp:7:7:7:7 | operator= | b::operator=(const b &) -> b & | copy_from_prototype.cpp:7:7:7:7 | b | <no expr> |
| copy_from_prototype.cpp:13:7:13:7 | c | c<int>::c(c<int> &&) -> void | copy_from_prototype.cpp:13:7:13:7 | c<int> | <no expr> |
| copy_from_prototype.cpp:13:7:13:7 | c | c<int>::c(const c<int> &) -> void | copy_from_prototype.cpp:13:7:13:7 | c<int> | <no expr> |
| copy_from_prototype.cpp:13:7:13:7 | operator= | c<int>::operator=(c<int> &&) -> c<int> & | copy_from_prototype.cpp:13:7:13:7 | c<int> | <no expr> |
| copy_from_prototype.cpp:13:7:13:7 | operator= | c<int>::operator=(const c<int> &) -> c<int> & | copy_from_prototype.cpp:13:7:13:7 | c<int> | <no expr> |
| copy_from_prototype.cpp:14:26:14:26 | c | c<T>::c<(unnamed)>() -> void | copy_from_prototype.cpp:13:7:13:7 | c<T> | Unknown literal |
| copy_from_prototype.cpp:14:26:14:26 | c | c<int>::c() -> void | copy_from_prototype.cpp:13:7:13:7 | c<int> | <no expr> |
| copy_from_prototype.cpp:14:26:14:26 | c | c<int>::c<(unnamed)>() -> void | copy_from_prototype.cpp:13:7:13:7 | c<int> | <no expr> |
| copy_from_prototype.cpp:17:7:17:7 | d | d::d() -> void | copy_from_prototype.cpp:17:7:17:7 | d | <no expr> |
| copy_from_prototype.cpp:17:7:17:7 | d | d::d(const d &) -> void | copy_from_prototype.cpp:17:7:17:7 | d | <no expr> |
| copy_from_prototype.cpp:17:7:17:7 | d | d::d(d &&) -> void | copy_from_prototype.cpp:17:7:17:7 | d | <no expr> |
| copy_from_prototype.cpp:17:7:17:7 | operator= | d::operator=(const d &) -> d & | copy_from_prototype.cpp:17:7:17:7 | d | <no expr> |
| copy_from_prototype.cpp:17:7:17:7 | operator= | d::operator=(d &&) -> d & | copy_from_prototype.cpp:17:7:17:7 | d | <no expr> |
| copy_from_prototype.cpp:22:8:22:8 | e | e<int>::e(const e<int> &) -> void | copy_from_prototype.cpp:22:8:22:8 | e<int> | <no expr> |
| copy_from_prototype.cpp:22:8:22:8 | e | e<int>::e(e<int> &&) -> void | copy_from_prototype.cpp:22:8:22:8 | e<int> | <no expr> |
| copy_from_prototype.cpp:22:8:22:8 | operator= | e<int>::operator=(const e<int> &) -> e<int> & | copy_from_prototype.cpp:22:8:22:8 | e<int> | <no expr> |
| copy_from_prototype.cpp:22:8:22:8 | operator= | e<int>::operator=(e<int> &&) -> e<int> & | copy_from_prototype.cpp:22:8:22:8 | e<int> | <no expr> |
| copy_from_prototype.cpp:23:26:23:26 | e | e<T>::e<(unnamed)>() -> void | copy_from_prototype.cpp:22:8:22:8 | e<T> | 456 |
| copy_from_prototype.cpp:26:35:26:43 | e | e<int>::e<(unnamed)>() -> void | copy_from_prototype.cpp:22:8:22:8 | e<int> | 456 |
| file://:0:0:0:0 | operator= | __va_list_tag::operator=() -> __va_list_tag & | file://:0:0:0:0 | __va_list_tag | <none> |
| file://:0:0:0:0 | operator= | __va_list_tag::operator=() -> __va_list_tag & | file://:0:0:0:0 | __va_list_tag | <none> |

12
cpp/ql/test/library-tests/noexcept/copy_from_prototype/copy_from_prototype.ql
View File

@@ -1,19 +1,9 @@
import cpp
string functionName(Function f) {
exists(string name, string templateArgs, string args |
result = name + templateArgs + args
and name = f.getQualifiedName()
and if exists(f.getATemplateArgument())
then templateArgs = "<" + concat(int i | exists(f.getTemplateArgument(i)) | f.getTemplateArgument(i).toString(), "," order by i) + ">"
else templateArgs = ""
and args = "(" + concat(int i | exists(f.getParameter(i)) | f.getParameter(i).getType().toString(), "," order by i) + ")")
}
from Function f, string e
where if f.hasExceptionSpecification()
then if exists(f.getADeclarationEntry().getNoExceptExpr())
then e = f.getADeclarationEntry().getNoExceptExpr().toString()
else e = "<no expr>"
else e = "<none>"
select f, functionName(f), f.getDeclaringType(), e
select f, f.getFullSignature(), f.getDeclaringType(), e

6
cpp/ql/test/query-tests/Best Practices/Likely Errors/EmptyBlock/empty_block.cpp
View File

@@ -48,4 +48,10 @@ int f(int x) {
// GOOD (no block)
for (;;) ;
// GOOD (has comment): [FALSE POSITIVE]
if (x) {} // comment
// GOOD (has comment): [FALSE POSITIVE]
if (x) {} // comment
}

3
cpp/ql/test/query-tests/Critical/DeadCodeGoto/DeadCodeGoto.expected Normal file
View File

@@ -0,0 +1,3 @@
| test.cpp:2:2:2:12 | goto ... | This statement makes $@ unreachable. | test.cpp:3:2:3:5 | ExprStmt | ExprStmt |
| test.cpp:9:3:9:8 | break; | This statement makes $@ unreachable. | test.cpp:10:3:10:6 | ExprStmt | ExprStmt |
| test.cpp:37:3:37:8 | break; | This statement makes $@ unreachable. | test.cpp:38:3:38:11 | return ... | return ... |

1
cpp/ql/test/query-tests/Critical/DeadCodeGoto/DeadCodeGoto.qlref Normal file
View File

@@ -0,0 +1 @@
Critical/DeadCodeGoto.ql

83
cpp/ql/test/query-tests/Critical/DeadCodeGoto/test.cpp Normal file
View File

@@ -0,0 +1,83 @@
int test1(int x) {
goto label; // BAD
x++;
label: return x;
}
int test2(int x) {
do {
break; // BAD
x++;
} while(false);
return x;
}
int test3(int x) {
goto label; // GOOD
label: x++;
return x;
}
int test4(int x) {
goto label; // GOOD
do {
label: x++;
} while(false);
return x;
}
int test5(int x, int y) {
switch(y) {
case 0:
break; // GOOD
case 1:
goto label; // GOOD
break;
case 2:
break; // BAD
return x;
case 3:
return x;
break; // GOOD
case 4:
goto label; // GOOD
case 5:
goto label;; // GOOD
default:
x++;
}
label:
return x;
}
void test6(int x, int cond) {
if (cond) {
x++;
} else goto end; // GOOD
x++;
end:
}
void test7(int x, int cond) {
if (cond)
{
goto target;
}
goto somewhere_else; // GOOD
while (x < 10) // not dead code
{
target:
x++;
}
somewhere_else:
switch (1)
{
goto end;
while (x < 10) // not dead code
{
case 1:
x++;
} break;
}
end:
}

1
cpp/ql/test/query-tests/Likely Bugs/Arithmetic/PointlessComparison/PointlessComparison.expected
View File

@@ -32,3 +32,4 @@
| PointlessComparison.c:129:12:129:16 | ... > ... | Comparison is always false because a <= 3. |
| PointlessComparison.c:197:7:197:11 | ... < ... | Comparison is always false because x >= 0. |
| RegressionTests.cpp:57:7:57:22 | ... <= ... | Comparison is always true because * ... <= 4294967295. |
| Templates.cpp:9:10:9:24 | ... <= ... | Comparison is always true because local <= 32767. |

9
cpp/ql/test/query-tests/Likely Bugs/Arithmetic/PointlessComparison/RegressionTests.cpp
View File

@@ -57,3 +57,12 @@ static int foo(size_t *size)
if (*size <= MAX_VAL) // BAD (pointless comparison) [NO LONGER REPORTED]
*size = MAX_VAL;
}
// ODASA-7205
int regression_test_01(unsigned long bb) {
if (bb + 1 == 0) { // GOOD [NO LONGER REPORTED]
return 0;
} else {
return 1;
}
}

17
cpp/ql/test/query-tests/Likely Bugs/Arithmetic/PointlessComparison/Templates.cpp Normal file
View File

@@ -0,0 +1,17 @@
template<typename T>
bool sometimesPointless(T param) {
return param <= 0xFFFF; // GOOD (hypothetical instantiations are okay)
}
template<typename T>
bool alwaysPointless(T param) {
short local = param;
return local <= 0xFFFF; // BAD (in all instantiations)
}
static int caller(int i) {
return
sometimesPointless<short>(i) ||
alwaysPointless<short>(i) ||
alwaysPointless<int>(i);
}

17
cpp/ql/test/query-tests/Likely Bugs/Arithmetic/UnsignedGEZero/Templates.cpp Normal file
View File

@@ -0,0 +1,17 @@
template<typename T>
bool sometimesPointless(T param) {
return param >= 0; // GOOD (FALSE POSITIVE: hypothetical instantiations are okay)
}
template<typename T>
bool alwaysPointless(T param) {
unsigned int local = param;
return local >= 0; // BAD (in all instantiations)
}
static int caller(int i) {
return
sometimesPointless<unsigned int>(i) ||
alwaysPointless<unsigned int>(i) ||
alwaysPointless<int>(i);
}

1
cpp/ql/test/query-tests/Likely Bugs/Arithmetic/UnsignedGEZero/UnsignedGEZero.expected
View File

@@ -1,3 +1,4 @@
| Templates.cpp:9:10:9:19 | ... >= ... | Pointless comparison of unsigned value to zero. |
| UnsignedGEZero.c:40:6:40:12 | ... >= ... | Pointless comparison of unsigned value to zero. |
| UnsignedGEZero.c:48:6:48:15 | ... >= ... | Pointless comparison of unsigned value to zero. |
| UnsignedGEZero.c:54:6:54:12 | ... >= ... | Pointless comparison of unsigned value to zero. |

1
cpp/ql/test/query-tests/Likely Bugs/Format/WrongNumberOfFormatArguments/TooManyFormatArguments.expected
View File

@@ -1,5 +1,4 @@
| custom_printf.cpp:31:5:31:12 | call to myPrintf | Format expects 2 arguments but given 3 |
| custom_printf.cpp:44:2:44:7 | call to printf | Format expects 0 arguments but given 2 |
| macros.cpp:12:2:12:31 | call to printf | Format expects 2 arguments but given 3 |
| macros.cpp:16:2:16:30 | call to printf | Format expects 2 arguments but given 3 |
| test.c:7:2:7:7 | call to printf | Format expects 0 arguments but given 1 |

1
cpp/ql/test/query-tests/Likely Bugs/Format/WrongNumberOfFormatArguments/WrongNumberOfFormatArguments.expected
View File

@@ -1,5 +1,4 @@
| custom_printf.cpp:29:5:29:12 | call to myPrintf | Format expects 2 arguments but given 1 |
| custom_printf.cpp:45:2:45:7 | call to printf | Format expects 2 arguments but given 0 |
| macros.cpp:14:2:14:37 | call to printf | Format expects 4 arguments but given 3 |
| macros.cpp:21:2:21:36 | call to printf | Format expects 4 arguments but given 3 |
| test.c:9:2:9:7 | call to printf | Format expects 1 arguments but given 0 |

4
cpp/ql/test/query-tests/Likely Bugs/Format/WrongNumberOfFormatArguments/custom_printf.cpp
View File

@@ -41,6 +41,6 @@ void test_custom_printf2()
{
// notTheFormat format ...
printf(0, "%i %i", 100, 200); // GOOD
printf("", "%i %i", 100, 200); // GOOD [FALSE POSITIVE]
printf("%i %i", "" ); // GOOD [FALSE POSITIVE]
printf("", "%i %i", 100, 200); // GOOD
printf("%i %i", "" ); // GOOD
}

10
cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/WrongTypeFormatArguments.expected Normal file
View File

@@ -0,0 +1,10 @@
| tests.cpp:18:15:18:22 | Hello | This argument should be of type 'char *' but is of type 'char16_t *' |
| tests.cpp:19:15:19:22 | Hello | This argument should be of type 'char *' but is of type 'wchar_t *' |
| tests.cpp:25:17:25:23 | Hello | This argument should be of type 'wchar_t *' but is of type 'char *' |
| tests.cpp:26:17:26:24 | Hello | This argument should be of type 'wchar_t *' but is of type 'char16_t *' |
| tests.cpp:30:17:30:24 | Hello | This argument should be of type 'char *' but is of type 'char16_t *' |
| tests.cpp:31:17:31:24 | Hello | This argument should be of type 'char *' but is of type 'wchar_t *' |
| tests.cpp:33:36:33:42 | Hello | This argument should be of type 'char16_t *' but is of type 'char *' |
| tests.cpp:35:36:35:43 | Hello | This argument should be of type 'char16_t *' but is of type 'wchar_t *' |
| tests.cpp:38:36:38:43 | Hello | This argument should be of type 'char *' but is of type 'char16_t *' |
| tests.cpp:39:36:39:43 | Hello | This argument should be of type 'char *' but is of type 'wchar_t *' |

1
cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/WrongTypeFormatArguments.qlref Normal file
View File

@@ -0,0 +1 @@
Likely Bugs/Format/WrongTypeFormatArguments.ql

3
cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/formattingFunction.expected Normal file
View File

@@ -0,0 +1,3 @@
| tests.cpp:8:5:8:10 | printf | char | char16_t, wchar_t | char16_t, wchar_t |
| tests.cpp:9:5:9:11 | wprintf | wchar_t | char | wchar_t |
| tests.cpp:10:5:10:12 | swprintf | char16_t | char | char16_t |

8
cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/formattingFunction.ql Normal file
View File

@@ -0,0 +1,8 @@
import cpp
from FormattingFunction f
select
f,
concat(f.getDefaultCharType().toString(), ", "),
concat(f.getNonDefaultCharType().toString(), ", "),
concat(f.getWideCharType().toString(), ", ")

40
cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_byte_wprintf/tests.cpp Normal file
View File

@@ -0,0 +1,40 @@
/*
* Test for custom definitions of *wprintf using different types than the
* platform wide character type.
*/
typedef unsigned int size_t;
int printf(const char * format, ...);
int wprintf(const wchar_t * format, ...); // on wchar_t
int swprintf(char16_t * s, size_t n, const char16_t * format, ...); // on char16_t
#define BUF_SIZE (4096)
void tests() {
char16_t buffer[BUF_SIZE];
printf("%s", "Hello"); // GOOD
printf("%s", u"Hello"); // BAD: expecting char
printf("%s", L"Hello"); // BAD: expecting char
printf("%S", "Hello"); // BAD: expecting wchar_t or char16_t [NOT DETECTED]
printf("%S", u"Hello"); // GOOD
printf("%S", L"Hello"); // GOOD
wprintf(L"%s", "Hello"); // BAD: expecting wchar_t
wprintf(L"%s", u"Hello"); // BAD: expecting wchar_t
wprintf(L"%s", L"Hello"); // GOOD
wprintf(L"%S", "Hello"); // GOOD
wprintf(L"%S", u"Hello"); // BAD: expecting char
wprintf(L"%S", L"Hello"); // BAD: expecting char
swprintf(buffer, BUF_SIZE, u"%s", "Hello"); // BAD: expecting char16_t
swprintf(buffer, BUF_SIZE, u"%s", u"Hello"); // GOOD
swprintf(buffer, BUF_SIZE, u"%s", L"Hello"); // BAD: expecting char16_t
swprintf(buffer, BUF_SIZE, u"%S", "Hello"); // GOOD
swprintf(buffer, BUF_SIZE, u"%S", u"Hello"); // BAD: expecting char
swprintf(buffer, BUF_SIZE, u"%S", L"Hello"); // BAD: expecting char
}

2
cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_word_size/WrongTypeFormatArguments.expected Normal file
View File

@@ -0,0 +1,2 @@
| tests_32.cpp:14:16:14:23 | void_ptr | This argument should be of type 'long' but is of type 'void *' |
| tests_64.cpp:14:16:14:23 | void_ptr | This argument should be of type 'long' but is of type 'void *' |

1
cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_word_size/WrongTypeFormatArguments.qlref Normal file
View File

@@ -0,0 +1 @@
Likely Bugs/Format/WrongTypeFormatArguments.ql

17
cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_word_size/tests_32.cpp Normal file
View File

@@ -0,0 +1,17 @@
// semmle-extractor-options: --edg --target --edg linux_i686
/*
* Test for printf in a snapshot that contains multiple word/pointer sizes.
*/
int printf(const char * format, ...);
void test_32()
{
long l;
void *void_ptr;
printf("%li", l); // GOOD
printf("%li", void_ptr); // BAD
printf("%p", l); // BAD [NOT DETECTED]
printf("%p", void_ptr); // GOOD
}

17
cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_mixed_word_size/tests_64.cpp Normal file
View File

@@ -0,0 +1,17 @@
// semmle-extractor-options: --edg --target --edg linux_x86_64
/*
* Test for printf in a snapshot that contains multiple word/pointer sizes.
*/
int printf(const char * format, ...);
void test_64()
{
long l;
void *void_ptr;
printf("%li", l); // GOOD
printf("%li", void_ptr); // BAD
printf("%p", l); // BAD [NOT DETECTED]
printf("%p", void_ptr); // GOOD
}

8
cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_signed_chars/WrongTypeFormatArguments.expected
View File

@@ -11,14 +11,6 @@
| printf1.h:45:18:45:20 | ull | This argument should be of type 'unsigned int' but is of type 'unsigned long long' |
| printf1.h:46:18:46:20 | ull | This argument should be of type 'unsigned int' but is of type 'unsigned long long' |
| printf1.h:47:19:47:21 | ull | This argument should be of type 'unsigned int' but is of type 'unsigned long long' |
| printf1.h:68:19:68:21 | sst | This argument should be of type 'size_t' but is of type 'long' |
| printf1.h:70:19:70:20 | ul | This argument should be of type 'ssize_t' but is of type 'unsigned long' |
| printf1.h:71:19:71:20 | st | This argument should be of type 'ssize_t' but is of type 'unsigned long' |
| printf1.h:72:19:72:20 | ST | This argument should be of type 'ssize_t' but is of type 'unsigned long' |
| printf1.h:73:19:73:22 | c_st | This argument should be of type 'ssize_t' but is of type 'unsigned long' |
| printf1.h:74:19:74:22 | C_ST | This argument should be of type 'ssize_t' but is of type 'unsigned long' |
| printf1.h:75:19:75:28 | sizeof(<expr>) | This argument should be of type 'ssize_t' but is of type 'unsigned long' |
| printf1.h:83:23:83:35 | ... - ... | This argument should be of type 'size_t' but is of type 'long' |
| real_world.h:61:21:61:22 | & ... | This argument should be of type 'int *' but is of type 'short *' |
| real_world.h:62:22:62:23 | & ... | This argument should be of type 'short *' but is of type 'int *' |
| real_world.h:63:22:63:24 | & ... | This argument should be of type 'short *' but is of type 'unsigned int *' |

27
cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_signed_chars/printf1.h
View File

@@ -65,14 +65,14 @@ void g()
printf("%zu", c_st); // ok
printf("%zu", C_ST); // ok
printf("%zu", sizeof(ul)); // ok
printf("%zu", sst); // not ok [NOT DETECTED ON MICROSOFT]
printf("%zu", sst); // not ok [NOT DETECTED]
printf("%zd", ul); // not ok
printf("%zd", st); // not ok
printf("%zd", ST); // not ok
printf("%zd", c_st); // not ok
printf("%zd", C_ST); // not ok
printf("%zd", sizeof(ul)); // not ok
printf("%zd", ul); // not ok [NOT DETECTED]
printf("%zd", st); // not ok [NOT DETECTED]
printf("%zd", ST); // not ok [NOT DETECTED]
printf("%zd", c_st); // not ok [NOT DETECTED]
printf("%zd", C_ST); // not ok [NOT DETECTED]
printf("%zd", sizeof(ul)); // not ok [NOT DETECTED]
printf("%zd", sst); // ok
{
char *ptr_a, *ptr_b;
@@ -80,8 +80,8 @@ void g()
printf("%tu", ptr_a - ptr_b); // ok
printf("%td", ptr_a - ptr_b); // ok
printf("%zu", ptr_a - ptr_b); // ok (dubious) [DETECTED ON LINUX ONLY]
printf("%zd", ptr_a - ptr_b); // ok (dubious) [DETECTED ON MICROSOFT ONLY]
printf("%zu", ptr_a - ptr_b); // ok (dubious)
printf("%zd", ptr_a - ptr_b); // ok (dubious)
}
}
@@ -92,3 +92,12 @@ void h(int i, struct some_type *j, int k)
// going on.
printf("%i %R %i", i, j, k); // GOOD (as far as we can tell)
}
typedef long ptrdiff_t;
void fun1(unsigned char* a, unsigned char* b) {
ptrdiff_t pdt;
printf("%td\n", pdt); // GOOD
printf("%td\n", a-b); // GOOD
}

2
cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_two_byte_wprintf/WrongTypeFormatArguments.expected Normal file
View File

@@ -0,0 +1,2 @@
| printf.cpp:45:29:45:35 | test | This argument should be of type 'char *' but is of type 'char16_t *' |
| printf.cpp:52:29:52:35 | test | This argument should be of type 'char16_t *' but is of type 'wchar_t *' |

1
cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_two_byte_wprintf/WrongTypeFormatArguments.qlref Normal file
View File

@@ -0,0 +1 @@
Likely Bugs/Format/WrongTypeFormatArguments.ql

2
cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_two_byte_wprintf/formattingFunction.expected Normal file
View File

@@ -0,0 +1,2 @@
| printf.cpp:15:5:15:12 | swprintf | char16_t | char | char16_t |
| printf.cpp:26:5:26:11 | sprintf | char | char16_t | char16_t |

8
cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_two_byte_wprintf/formattingFunction.ql Normal file
View File

@@ -0,0 +1,8 @@
import cpp
from FormattingFunction f
select
f,
concat(f.getDefaultCharType().toString(), ", "),
concat(f.getNonDefaultCharType().toString(), ", "),
concat(f.getWideCharType().toString(), ", ")

53
cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_two_byte_wprintf/printf.cpp Normal file
View File

@@ -0,0 +1,53 @@
/*
* Test for custom definitions of *wprintf using different types than the
* platform wide character type.
*/
#define WCHAR char16_t
typedef void *va_list;
#define va_start(va, other)
#define va_end(args)
int vswprintf(WCHAR *dest, WCHAR *format, va_list args) {
return 0;
}
int swprintf(WCHAR *dest, WCHAR *format, ...) {
va_list args;
va_start(args, format);
int ret = vswprintf(dest, format, args);
va_end(args);
return ret;
}
int sprintf(char *dest, char *format, ...);
// ---
void test1() {
WCHAR string[20];
swprintf(string, u"test %s", u"test"); // GOOD
}
void test2() {
char string[20];
sprintf(string, "test %S", u"test"); // GOOD
}
void test3() {
char string[20];
sprintf(string, "test %s", u"test"); // BAD: `char16_t` string parameter read as `char` string
}
void test4() {
char string[20];
sprintf(string, "test %S", L"test"); // BAD: `wchar_t` string parameter read as `char16_t` string
}

8
cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_unsigned_chars/WrongTypeFormatArguments.expected
View File

@@ -11,14 +11,6 @@
| printf1.h:45:18:45:20 | ull | This argument should be of type 'unsigned int' but is of type 'unsigned long long' |
| printf1.h:46:18:46:20 | ull | This argument should be of type 'unsigned int' but is of type 'unsigned long long' |
| printf1.h:47:19:47:21 | ull | This argument should be of type 'unsigned int' but is of type 'unsigned long long' |
| printf1.h:68:19:68:21 | sst | This argument should be of type 'size_t' but is of type 'long' |
| printf1.h:70:19:70:20 | ul | This argument should be of type 'ssize_t' but is of type 'unsigned long' |
| printf1.h:71:19:71:20 | st | This argument should be of type 'ssize_t' but is of type 'unsigned long' |
| printf1.h:72:19:72:20 | ST | This argument should be of type 'ssize_t' but is of type 'unsigned long' |
| printf1.h:73:19:73:22 | c_st | This argument should be of type 'ssize_t' but is of type 'unsigned long' |
| printf1.h:74:19:74:22 | C_ST | This argument should be of type 'ssize_t' but is of type 'unsigned long' |
| printf1.h:75:19:75:28 | sizeof(<expr>) | This argument should be of type 'ssize_t' but is of type 'unsigned long' |
| printf1.h:83:23:83:35 | ... - ... | This argument should be of type 'size_t' but is of type 'long' |
| real_world.h:61:21:61:22 | & ... | This argument should be of type 'int *' but is of type 'short *' |
| real_world.h:62:22:62:23 | & ... | This argument should be of type 'short *' but is of type 'int *' |
| real_world.h:63:22:63:24 | & ... | This argument should be of type 'short *' but is of type 'unsigned int *' |

4
cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_unsigned_chars/formattingFunction.expected Normal file
View File

@@ -0,0 +1,4 @@
| common.h:12:12:12:17 | printf | char | wchar_t | wchar_t |
| format.h:4:13:4:17 | error | char | wchar_t | wchar_t |
| real_world.h:8:12:8:18 | fprintf | char | wchar_t | wchar_t |
| real_world.h:33:6:33:12 | msg_out | char | wchar_t | wchar_t |

8
cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_unsigned_chars/formattingFunction.ql Normal file
View File

@@ -0,0 +1,8 @@
import cpp
from FormattingFunction f
select
f,
concat(f.getDefaultCharType().toString(), ", "),
concat(f.getNonDefaultCharType().toString(), ", "),
concat(f.getWideCharType().toString(), ", ")

27
cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Linux_unsigned_chars/printf1.h
View File

@@ -65,14 +65,14 @@ void g()
printf("%zu", c_st); // ok
printf("%zu", C_ST); // ok
printf("%zu", sizeof(ul)); // ok
printf("%zu", sst); // not ok [NOT DETECTED ON MICROSOFT]
printf("%zu", sst); // not ok [NOT DETECTED]
printf("%zd", ul); // not ok
printf("%zd", st); // not ok
printf("%zd", ST); // not ok
printf("%zd", c_st); // not ok
printf("%zd", C_ST); // not ok
printf("%zd", sizeof(ul)); // not ok
printf("%zd", ul); // not ok [NOT DETECTED]
printf("%zd", st); // not ok [NOT DETECTED]
printf("%zd", ST); // not ok [NOT DETECTED]
printf("%zd", c_st); // not ok [NOT DETECTED]
printf("%zd", C_ST); // not ok [NOT DETECTED]
printf("%zd", sizeof(ul)); // not ok [NOT DETECTED]
printf("%zd", sst); // ok
{
char *ptr_a, *ptr_b;
@@ -80,8 +80,8 @@ void g()
printf("%tu", ptr_a - ptr_b); // ok
printf("%td", ptr_a - ptr_b); // ok
printf("%zu", ptr_a - ptr_b); // ok (dubious) [DETECTED ON LINUX ONLY]
printf("%zd", ptr_a - ptr_b); // ok (dubious) [DETECTED ON MICROSOFT ONLY]
printf("%zu", ptr_a - ptr_b); // ok (dubious)
printf("%zd", ptr_a - ptr_b); // ok (dubious)
}
}
@@ -92,3 +92,12 @@ void h(int i, struct some_type *j, int k)
// going on.
printf("%i %R %i", i, j, k); // GOOD (as far as we can tell)
}
typedef long ptrdiff_t;
void fun1(unsigned char* a, unsigned char* b) {
ptrdiff_t pdt;
printf("%td\n", pdt); // GOOD
printf("%td\n", a-b); // GOOD
}

1
cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Microsoft/WrongTypeFormatArguments.expected
View File

@@ -11,7 +11,6 @@
| printf1.h:45:18:45:20 | ull | This argument should be of type 'unsigned int' but is of type 'unsigned long long' |
| printf1.h:46:18:46:20 | ull | This argument should be of type 'unsigned int' but is of type 'unsigned long long' |
| printf1.h:47:19:47:21 | ull | This argument should be of type 'unsigned int' but is of type 'unsigned long long' |
| printf1.h:70:19:70:20 | ul | This argument should be of type 'ssize_t' but is of type 'unsigned long' |
| printf1.h:71:19:71:20 | st | This argument should be of type 'ssize_t' but is of type 'unsigned long long' |
| printf1.h:72:19:72:20 | ST | This argument should be of type 'ssize_t' but is of type 'unsigned long long' |
| printf1.h:73:19:73:22 | c_st | This argument should be of type 'ssize_t' but is of type 'unsigned long long' |

17
cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Microsoft/printf1.h
View File

@@ -65,9 +65,9 @@ void g()
printf("%zu", c_st); // ok
printf("%zu", C_ST); // ok
printf("%zu", sizeof(ul)); // ok
printf("%zu", sst); // not ok [NOT DETECTED ON MICROSOFT]
printf("%zu", sst); // not ok [NOT DETECTED]
printf("%zd", ul); // not ok
printf("%zd", ul); // not ok [NOT DETECTED]
printf("%zd", st); // not ok
printf("%zd", ST); // not ok
printf("%zd", c_st); // not ok
@@ -80,8 +80,8 @@ void g()
printf("%tu", ptr_a - ptr_b); // ok
printf("%td", ptr_a - ptr_b); // ok
printf("%zu", ptr_a - ptr_b); // ok (dubious) [DETECTED ON LINUX ONLY]
printf("%zd", ptr_a - ptr_b); // ok (dubious) [DETECTED ON MICROSOFT ONLY]
printf("%zu", ptr_a - ptr_b); // ok (dubious)
printf("%zd", ptr_a - ptr_b); // ok (dubious) [FALSE POSITIVE]
}
}
@@ -92,3 +92,12 @@ void h(int i, struct some_type *j, int k)
// going on.
printf("%i %R %i", i, j, k); // GOOD (as far as we can tell)
}
typedef long long ptrdiff_t;
void fun1(unsigned char* a, unsigned char* b) {
ptrdiff_t pdt;
printf("%td\n", pdt); // GOOD
printf("%td\n", a-b); // GOOD
}

2
cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Microsoft/real_world.h
View File

@@ -43,7 +43,7 @@ void someFunction()
WCHAR filename[MAX_LONGPATH];
int linenum;
msg_out("Source file: %S @ %d\n", filename, linenum); // GOOD
msg_out("Source file: %S @ %d\n", filename, linenum); // GOOD [FALSE POSITIVE]
}
// --------------------------------------------------------------

2
cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Microsoft_no_wchar/WrongTypeFormatArguments.expected
View File

@@ -11,13 +11,13 @@
| printf1.h:45:18:45:20 | ull | This argument should be of type 'unsigned int' but is of type 'unsigned long long' |
| printf1.h:46:18:46:20 | ull | This argument should be of type 'unsigned int' but is of type 'unsigned long long' |
| printf1.h:47:19:47:21 | ull | This argument should be of type 'unsigned int' but is of type 'unsigned long long' |
| printf1.h:70:19:70:20 | ul | This argument should be of type 'ssize_t' but is of type 'unsigned long' |
| printf1.h:71:19:71:20 | st | This argument should be of type 'ssize_t' but is of type 'unsigned long long' |
| printf1.h:72:19:72:20 | ST | This argument should be of type 'ssize_t' but is of type 'unsigned long long' |
| printf1.h:73:19:73:22 | c_st | This argument should be of type 'ssize_t' but is of type 'unsigned long long' |
| printf1.h:74:19:74:22 | C_ST | This argument should be of type 'ssize_t' but is of type 'unsigned long long' |
| printf1.h:75:19:75:28 | sizeof(<expr>) | This argument should be of type 'ssize_t' but is of type 'unsigned long long' |
| printf1.h:84:23:84:35 | ... - ... | This argument should be of type 'ssize_t' but is of type 'long long' |
| printf1.h:130:18:130:18 | 0 | This argument should be of type 'void *' but is of type 'int' |
| real_world.h:61:21:61:22 | & ... | This argument should be of type 'int *' but is of type 'short *' |
| real_world.h:62:22:62:23 | & ... | This argument should be of type 'short *' but is of type 'int *' |
| real_world.h:63:22:63:24 | & ... | This argument should be of type 'short *' but is of type 'unsigned int *' |

45
cpp/ql/test/query-tests/Likely Bugs/Format/WrongTypeFormatArguments/Microsoft_no_wchar/printf1.h
View File

@@ -65,9 +65,9 @@ void g()
printf("%zu", c_st); // ok
printf("%zu", C_ST); // ok
printf("%zu", sizeof(ul)); // ok
printf("%zu", sst); // not ok [NOT DETECTED ON MICROSOFT]
printf("%zu", sst); // not ok [NOT DETECTED]
printf("%zd", ul); // not ok
printf("%zd", ul); // not ok [NOT DETECTED]
printf("%zd", st); // not ok
printf("%zd", ST); // not ok
printf("%zd", c_st); // not ok
@@ -80,8 +80,8 @@ void g()
printf("%tu", ptr_a - ptr_b); // ok
printf("%td", ptr_a - ptr_b); // ok
printf("%zu", ptr_a - ptr_b); // ok (dubious) [DETECTED ON LINUX ONLY]
printf("%zd", ptr_a - ptr_b); // ok (dubious) [DETECTED ON MICROSOFT ONLY]
printf("%zu", ptr_a - ptr_b); // ok (dubious)
printf("%zd", ptr_a - ptr_b); // ok (dubious) [FALSE POSITIVE]
}
}
@@ -92,3 +92,40 @@ void h(int i, struct some_type *j, int k)
// going on.
printf("%i %R %i", i, j, k); // GOOD (as far as we can tell)
}
typedef long long ptrdiff_t;
void fun1(unsigned char* a, unsigned char* b) {
ptrdiff_t pdt;
printf("%td\n", pdt); // GOOD
printf("%td\n", a-b); // GOOD
}
typedef wchar_t WCHAR_T; // WCHAR_T -> wchar_t -> int
typedef int MYCHAR; // MYCHAR -> int (notably not via the wchar_t typedef)
void fun2() {
wchar_t *myString1;
WCHAR_T *myString2;
int *myString3;
MYCHAR *myString4;
printf("%S", myString1); // GOOD
printf("%S", myString2); // GOOD
printf("%S", myString3); // GOOD
printf("%S", myString4); // GOOD
}
typedef void *VOIDPTR;
typedef int (*FUNPTR)(int);
void fun3(void *p1, VOIDPTR p2, FUNPTR p3, char *p4)
{
printf("%p\n", p1); // GOOD
printf("%p\n", p2); // GOOD
printf("%p\n", p3); // GOOD
printf("%p\n", p4); // GOOD
printf("%p\n", p4 + 1); // GOOD
printf("%p\n", 0); // GOOD [FALSE POSITIVE]
}

89
cpp/ql/test/query-tests/Likely Bugs/Likely Typos/inconsistentLoopDirection/inconsistentLoopDirection.c Normal file
View File

@@ -0,0 +1,89 @@
void Signed()
{
signed char i;
for (i = 0; i < 100; i--) //BUG
{
}
for (i = 0; i < 100; i++)
{
}
for (i = 100; i >= 0; i++) //BUG
{
}
for (i = 100; i >= 0; i--)
{
}
}
void Unsigned()
{
unsigned long i;
for (i = 0; i < 100; i--) //BUG
{
}
for (i = 0; i < 100; i++)
{
}
for (i = 100; i >= 0; i++) //BUG
{
}
for (i = 100; i >= 0; i--)
{
}
}
void InitializationOutsideLoop()
{
signed char i = 0;
for (; i < 100; i--) //BUG
{
}
i = 0;
for (; i < 100; i++)
{
}
i = 100;
for (; i >= 0; i++) //BUG
{
}
i = 100;
for (; i >= 0; i--)
{
}
}
void NegativeTestCase()
{
int i;
for (i = 0; (200 - i) < 100; i--)
{
// code ...
}
}
void NegativeTestCaseNested()
{
int k;
int i;
for (k = 200; k < 300; k++)
{
for (i = 0; (k - i) < 100; i--)
{
// code ...
}
}
}

180
cpp/ql/test/query-tests/Likely Bugs/Likely Typos/inconsistentLoopDirection/inconsistentLoopDirection.cpp Normal file
View File

@@ -0,0 +1,180 @@
void Signed()
{
signed char i;
for (i = 0; i < 100; i--) //BUG
{
}
for (i = 0; i < 100; i++)
{
}
for (i = 100; i >= 0; i++) //BUG
{
}
for (i = 100; i >= 0; i--)
{
}
}
void Unsigned()
{
unsigned long i;
for (i = 0; i < 100; i--) //BUG
{
}
for (i = 0; i < 100; i++)
{
}
for (i = 100; i >= 0; i++) //BUG
{
}
for (i = 100; i >= 0; i--)
{
}
}
void DeclarationInLoop()
{
for (signed char i = 0; i < 100; --i) //BUG
{
}
for (signed char i = 0; i < 100; ++i)
{
}
for (unsigned char i = 100; i >= 0; ++i) //BUG
{
}
for (unsigned char i = 100; i >= 0; --i)
{
}
}
void SignedWithVariables()
{
signed char i;
signed char min = 0;
signed char max = 100;
for (i = min; i < max; i--) //BUG
{
}
for (i = min; i < max; i++)
{
}
for (i = max; i >= min; i++) //BUG
{
}
for (i = max; i >= min; i--)
{
}
}
void InitializationOutsideLoop()
{
signed char i = 0;
for (; i < 100; --i) //BUG
{
}
i = 0;
for (; i < 100; ++i)
{
}
i = 100;
for (; i >= 0; ++i) //BUG
{
}
i = 100;
for (; i >= 0; --i)
{
}
}
void InvalidCondition()
{
signed char i;
signed char min = 0;
signed char max = 100;
for (i = max; i < min; i--) //BUG
{
}
for (i = min; i > max; i++) //BUG
{
}
}
void InvalidConditionUnsignedCornerCase()
{
unsigned char i;
unsigned char min = 0;
unsigned char max = 100;
for (i = 100; i < 0; i--) //BUG
{
}
// Limitation.
// Currently odasa will not detect this for-loop condition as always true
// The rule will still detect the mismatch iterator, but the error message may change in the future.
for (i = 200; i >= 0; i++) //BUG
{
}
}
void NegativeTestCase()
{
for (int i = 0; (100 - i) < 200; i--)
{
// code ...
}
}
void NegativeTestCaseNested()
{
for (int k = 200; k < 300; k++)
{
for (int i = 0; (k - i) < 100; i--)
{
// code ...
}
}
}
//////////////////////////////////////
// Query limitation:
//
// The following test cases are bugs,
// but will not be found due to the itearion expression
// not being a prefix or postfix increment/decrement
//
void FalseNegativeTestCases()
{
for (int i = 0; i < 10; i = i - 1) {}
// For comparison
for (int i = 0; i < 10; i-- ) {} // BUG
for (int i = 100; i > 0; i += 2) {}
// For comparison
for (int i = 100; i > 0; i ++ ) {} // BUG
}

22
cpp/ql/test/query-tests/Likely Bugs/Likely Typos/inconsistentLoopDirection/inconsistentLoopDirection.expected Normal file
View File

@@ -0,0 +1,22 @@
| inconsistentLoopDirection.c:5:5:7:5 | for(...;...;...) ... | Ill-defined for-loop: a loop using variable "i" counts downward from a value (0), but the terminal condition is higher (100). |
| inconsistentLoopDirection.c:13:5:15:5 | for(...;...;...) ... | Ill-defined for-loop: a loop using variable "i" counts upward from a value (100), but the terminal condition is lower (0). |
| inconsistentLoopDirection.c:27:5:29:5 | for(...;...;...) ... | Ill-defined for-loop: a loop using variable "i" counts downward from a value (0), but the terminal condition is higher (100). |
| inconsistentLoopDirection.c:35:5:37:5 | for(...;...;...) ... | Ill-defined for-loop: a loop using variable "i" counts upward from a value (100), but the terminal condition is lower (0). |
| inconsistentLoopDirection.c:48:5:50:5 | for(...;...;...) ... | Ill-defined for-loop: a loop using variable "i" counts downward from a value (0), but the terminal condition is higher (100). |
| inconsistentLoopDirection.c:58:5:60:5 | for(...;...;...) ... | Ill-defined for-loop: a loop using variable "i" counts upward from a value (100), but the terminal condition is lower (0). |
| inconsistentLoopDirection.cpp:5:5:7:5 | for(...;...;...) ... | Ill-defined for-loop: a loop using variable "i" counts downward from a value (0), but the terminal condition is higher (100). |
| inconsistentLoopDirection.cpp:13:5:15:5 | for(...;...;...) ... | Ill-defined for-loop: a loop using variable "i" counts upward from a value (100), but the terminal condition is lower (0). |
| inconsistentLoopDirection.cpp:27:5:29:5 | for(...;...;...) ... | Ill-defined for-loop: a loop using variable "i" counts downward from a value (0), but the terminal condition is higher (100). |
| inconsistentLoopDirection.cpp:35:5:37:5 | for(...;...;...) ... | Ill-defined for-loop: a loop using variable "i" counts upward from a value (100), but the terminal condition is lower (0). |
| inconsistentLoopDirection.cpp:46:5:48:5 | for(...;...;...) ... | Ill-defined for-loop: a loop using variable "i" counts downward from a value (0), but the terminal condition is higher (100). |
| inconsistentLoopDirection.cpp:54:5:56:5 | for(...;...;...) ... | Ill-defined for-loop: a loop using variable "i" counts upward from a value (100), but the terminal condition is lower (0). |
| inconsistentLoopDirection.cpp:69:5:71:5 | for(...;...;...) ... | Ill-defined for-loop: a loop using variable "i" counts downward from a value (min), but the terminal condition is higher (max). |
| inconsistentLoopDirection.cpp:77:5:79:5 | for(...;...;...) ... | Ill-defined for-loop: a loop using variable "i" counts upward from a value (max), but the terminal condition is lower (min). |
| inconsistentLoopDirection.cpp:91:5:93:5 | for(...;...;...) ... | Ill-defined for-loop: a loop using variable "i" counts downward from a value (0), but the terminal condition is higher (100). |
| inconsistentLoopDirection.cpp:101:5:103:5 | for(...;...;...) ... | Ill-defined for-loop: a loop using variable "i" counts upward from a value (100), but the terminal condition is lower (0). |
| inconsistentLoopDirection.cpp:118:5:120:5 | for(...;...;...) ... | Ill-defined for-loop: a loop using variable "i" counts downward from a value (max), but the terminal condition is always false. |
| inconsistentLoopDirection.cpp:122:5:124:5 | for(...;...;...) ... | Ill-defined for-loop: a loop using variable "i" counts upward from a value (min), but the terminal condition is always false. |
| inconsistentLoopDirection.cpp:133:5:135:5 | for(...;...;...) ... | Ill-defined for-loop: a loop using variable "i" counts downward from a value (100), but the terminal condition is always false. |
| inconsistentLoopDirection.cpp:140:5:142:5 | for(...;...;...) ... | Ill-defined for-loop: a loop using variable "i" counts upward from a value (200), but the terminal condition is lower (0). |
| inconsistentLoopDirection.cpp:175:5:175:36 | for(...;...;...) ... | Ill-defined for-loop: a loop using variable "i" counts downward from a value (0), but the terminal condition is higher (10). |
| inconsistentLoopDirection.cpp:179:5:179:38 | for(...;...;...) ... | Ill-defined for-loop: a loop using variable "i" counts upward from a value (100), but the terminal condition is lower (0). |

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