mirror of
https://github.com/github/codeql.git
synced 2026-04-04 14:48:20 +02:00
Merge branch 'main' into more-public-dataflow-apis
This commit is contained in:
@@ -52,5 +52,6 @@ ql/cpp/ql/src/Summary/LinesOfUserCode.ql
|
||||
ql/cpp/ql/src/Telemetry/CompilerErrors.ql
|
||||
ql/cpp/ql/src/Telemetry/DatabaseQuality.ql
|
||||
ql/cpp/ql/src/Telemetry/ExtractionMetrics.ql
|
||||
ql/cpp/ql/src/Telemetry/ExtractorInformation.ql
|
||||
ql/cpp/ql/src/Telemetry/MissingIncludes.ql
|
||||
ql/cpp/ql/src/Telemetry/SucceededIncludes.ql
|
||||
|
||||
@@ -160,6 +160,7 @@ ql/cpp/ql/src/Summary/LinesOfUserCode.ql
|
||||
ql/cpp/ql/src/Telemetry/CompilerErrors.ql
|
||||
ql/cpp/ql/src/Telemetry/DatabaseQuality.ql
|
||||
ql/cpp/ql/src/Telemetry/ExtractionMetrics.ql
|
||||
ql/cpp/ql/src/Telemetry/ExtractorInformation.ql
|
||||
ql/cpp/ql/src/Telemetry/MissingIncludes.ql
|
||||
ql/cpp/ql/src/Telemetry/SucceededIncludes.ql
|
||||
ql/cpp/ql/src/jsf/4.06 Pre-Processing Directives/AV Rule 32.ql
|
||||
|
||||
@@ -93,5 +93,6 @@ ql/cpp/ql/src/Summary/LinesOfUserCode.ql
|
||||
ql/cpp/ql/src/Telemetry/CompilerErrors.ql
|
||||
ql/cpp/ql/src/Telemetry/DatabaseQuality.ql
|
||||
ql/cpp/ql/src/Telemetry/ExtractionMetrics.ql
|
||||
ql/cpp/ql/src/Telemetry/ExtractorInformation.ql
|
||||
ql/cpp/ql/src/Telemetry/MissingIncludes.ql
|
||||
ql/cpp/ql/src/Telemetry/SucceededIncludes.ql
|
||||
|
||||
@@ -1,3 +1,9 @@
|
||||
## 8.0.1
|
||||
|
||||
### Minor Analysis Improvements
|
||||
|
||||
* Inline expectations test comments, which are of the form `// $ tag` or `// $ tag=value`, are now parsed more strictly and will not be recognized if there isn't a space after the `$` symbol.
|
||||
|
||||
## 8.0.0
|
||||
|
||||
### Breaking Changes
|
||||
|
||||
@@ -0,0 +1,4 @@
|
||||
---
|
||||
category: feature
|
||||
---
|
||||
* Added a new data flow node, `IndirectUninitializedNode`, that represents uninitialized local variables behind a number of indirections.
|
||||
@@ -1,4 +1,5 @@
|
||||
---
|
||||
category: minorAnalysis
|
||||
---
|
||||
## 8.0.1
|
||||
|
||||
### Minor Analysis Improvements
|
||||
|
||||
* Inline expectations test comments, which are of the form `// $ tag` or `// $ tag=value`, are now parsed more strictly and will not be recognized if there isn't a space after the `$` symbol.
|
||||
@@ -1,2 +1,2 @@
|
||||
---
|
||||
lastReleaseVersion: 8.0.0
|
||||
lastReleaseVersion: 8.0.1
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
name: codeql/cpp-all
|
||||
version: 8.0.1-dev
|
||||
version: 8.0.2-dev
|
||||
groups: cpp
|
||||
dbscheme: semmlecode.cpp.dbscheme
|
||||
extractor: cpp
|
||||
|
||||
@@ -524,6 +524,12 @@ class Function extends Declaration, ControlFlowNode, AccessHolder, @function {
|
||||
not exists(NewOrNewArrayExpr new | e = new.getAllocatorCall().getArgument(0))
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Holds if this function has an ambiguous return type, meaning that zero or multiple return
|
||||
* types for this function are present in the database (this can occur in `build-mode: none`).
|
||||
*/
|
||||
predicate hasAmbiguousReturnType() { count(this.getType()) != 1 }
|
||||
}
|
||||
|
||||
pragma[noinline]
|
||||
|
||||
@@ -623,6 +623,25 @@ module Public {
|
||||
*/
|
||||
LocalVariable asUninitialized() { result = this.(UninitializedNode).getLocalVariable() }
|
||||
|
||||
/**
|
||||
* Gets the uninitialized local variable corresponding to this node behind
|
||||
* `index` number of indirections, if any.
|
||||
*/
|
||||
LocalVariable asIndirectUninitialized(int index) {
|
||||
exists(IndirectUninitializedNode indirectUninitializedNode |
|
||||
this = indirectUninitializedNode and
|
||||
indirectUninitializedNode.getIndirectionIndex() = index
|
||||
|
|
||||
result = indirectUninitializedNode.getLocalVariable()
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Gets the uninitialized local variable corresponding to this node behind
|
||||
* a number indirections, if any.
|
||||
*/
|
||||
LocalVariable asIndirectUninitialized() { result = this.asIndirectUninitialized(_) }
|
||||
|
||||
/**
|
||||
* Gets the positional parameter corresponding to the node that represents
|
||||
* the value of the parameter after `index` number of loads, if any. For
|
||||
@@ -767,16 +786,13 @@ module Public {
|
||||
final override Type getType() { result = this.getPreUpdateNode().getType() }
|
||||
}
|
||||
|
||||
/**
|
||||
* The value of an uninitialized local variable, viewed as a node in a data
|
||||
* flow graph.
|
||||
*/
|
||||
class UninitializedNode extends Node {
|
||||
abstract private class AbstractUninitializedNode extends Node {
|
||||
LocalVariable v;
|
||||
int indirectionIndex;
|
||||
|
||||
UninitializedNode() {
|
||||
AbstractUninitializedNode() {
|
||||
exists(SsaImpl::Definition def, SsaImpl::SourceVariable sv |
|
||||
def.getIndirectionIndex() = 0 and
|
||||
def.getIndirectionIndex() = indirectionIndex and
|
||||
def.getValue().asInstruction() instanceof UninitializedInstruction and
|
||||
SsaImpl::defToNode(this, def, sv) and
|
||||
v = sv.getBaseVariable().(SsaImpl::BaseIRVariable).getIRVariable().getAst()
|
||||
@@ -787,6 +803,25 @@ module Public {
|
||||
LocalVariable getLocalVariable() { result = v }
|
||||
}
|
||||
|
||||
/**
|
||||
* The value of an uninitialized local variable, viewed as a node in a data
|
||||
* flow graph.
|
||||
*/
|
||||
class UninitializedNode extends AbstractUninitializedNode {
|
||||
UninitializedNode() { indirectionIndex = 0 }
|
||||
}
|
||||
|
||||
/**
|
||||
* The value of an uninitialized local variable behind one or more levels of
|
||||
* indirection, viewed as a node in a data flow graph.
|
||||
*/
|
||||
class IndirectUninitializedNode extends AbstractUninitializedNode {
|
||||
IndirectUninitializedNode() { indirectionIndex > 0 }
|
||||
|
||||
/** Gets the indirection index of this node. */
|
||||
int getIndirectionIndex() { result = indirectionIndex }
|
||||
}
|
||||
|
||||
/**
|
||||
* The value of a parameter at function entry, viewed as a node in a data
|
||||
* flow graph. This includes both explicit parameters such as `x` in `f(x)`
|
||||
|
||||
@@ -1,3 +1,7 @@
|
||||
## 1.5.13
|
||||
|
||||
No user-facing changes.
|
||||
|
||||
## 1.5.12
|
||||
|
||||
No user-facing changes.
|
||||
|
||||
@@ -218,7 +218,9 @@ where
|
||||
// only report if we cannot prove that the result of the
|
||||
// multiplication will be less (resp. greater) than the
|
||||
// maximum (resp. minimum) number we can compute.
|
||||
overflows(me, t1)
|
||||
overflows(me, t1) and
|
||||
// exclude cases where the expression type may not have been extracted accurately
|
||||
not me.getParent().(Call).getTarget().hasAmbiguousReturnType()
|
||||
select me,
|
||||
"Multiplication result may overflow '" + me.getType().toString() + "' before it is converted to '"
|
||||
+ me.getFullyConverted().getType().toString() + "'."
|
||||
|
||||
@@ -168,9 +168,11 @@ where
|
||||
formatOtherArgType(ffc, n, expected, arg, actual) and
|
||||
not actual.getUnspecifiedType().(IntegralType).getSize() = sizeof_IntType()
|
||||
) and
|
||||
// Exclude some cases where we're less confident the result is correct / clear / valuable
|
||||
not arg.isAffectedByMacro() and
|
||||
not arg.isFromUninstantiatedTemplate(_) and
|
||||
not actual.stripType() instanceof ErroneousType and
|
||||
not arg.getType().stripType().(RoutineType).getReturnType() instanceof ErroneousType and
|
||||
not arg.(Call).mayBeFromImplicitlyDeclaredFunction() and
|
||||
// Make sure that the format function definition is consistent
|
||||
count(ffc.getTarget().getFormatParameterIndex()) = 1
|
||||
|
||||
@@ -4,7 +4,7 @@
|
||||
* allows for a cross-site scripting vulnerability.
|
||||
* @kind path-problem
|
||||
* @problem.severity error
|
||||
* @security-severity 6.1
|
||||
* @security-severity 7.8
|
||||
* @precision high
|
||||
* @id cpp/cgi-xss
|
||||
* @tags security
|
||||
|
||||
@@ -23,13 +23,31 @@ import Flow::PathGraph
|
||||
|
||||
predicate isSource(FlowSource source, string sourceType) { sourceType = source.getSourceType() }
|
||||
|
||||
/**
|
||||
* Holds if `f` is a printf-like function or a (possibly nested) wrapper
|
||||
* that forwards a format-string parameter to one.
|
||||
*
|
||||
* Functions that *implement* printf-like behavior (e.g. a custom
|
||||
* `vsnprintf` variant) internally parse the caller-supplied format string
|
||||
* and build small, bounded, local format strings such as `"%d"` or `"%ld"`
|
||||
* for inner `sprintf` calls. Taint that reaches those inner calls via the
|
||||
* parsed format specifier is not exploitable, so sinks inside such
|
||||
* functions should be excluded.
|
||||
*/
|
||||
private predicate isPrintfImplementation(Function f) {
|
||||
f instanceof PrintfLikeFunction
|
||||
or
|
||||
exists(PrintfLikeFunction printf | printf.wrapperFunction(f, _, _))
|
||||
}
|
||||
|
||||
module Config implements DataFlow::ConfigSig {
|
||||
predicate isSource(DataFlow::Node node) { isSource(node, _) }
|
||||
|
||||
predicate isSink(DataFlow::Node node) {
|
||||
exists(PrintfLikeFunction printf |
|
||||
printf.outermostWrapperFunctionCall([node.asExpr(), node.asIndirectExpr()], _)
|
||||
)
|
||||
) and
|
||||
not isPrintfImplementation([node.asExpr(), node.asIndirectExpr()].getEnclosingFunction())
|
||||
}
|
||||
|
||||
private predicate isArithmeticNonCharType(ArithmeticType type) {
|
||||
|
||||
@@ -18,7 +18,8 @@ import IncorrectPointerScalingCommon
|
||||
private predicate isCharSzPtrExpr(Expr e) {
|
||||
exists(PointerType pt | pt = e.getFullyConverted().getUnspecifiedType() |
|
||||
pt.getBaseType() instanceof CharType or
|
||||
pt.getBaseType() instanceof VoidType
|
||||
pt.getBaseType() instanceof VoidType or
|
||||
pt.getBaseType() instanceof ErroneousType // this could be char / void type in a successful compilation
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
25
cpp/ql/src/Telemetry/DatabaseQuality.qll
Normal file
25
cpp/ql/src/Telemetry/DatabaseQuality.qll
Normal file
@@ -0,0 +1,25 @@
|
||||
import cpp
|
||||
import codeql.util.ReportStats
|
||||
|
||||
module CallTargetStats implements StatsSig {
|
||||
private class RelevantCall extends Call {
|
||||
RelevantCall() { this.getFile() = any(File f | f.fromSource() and exists(f.getRelativePath())) }
|
||||
}
|
||||
|
||||
// We assume that calls with an implicit target are calls that could not be
|
||||
// resolved. This is accurate in the vast majority of cases, but is inaccurate
|
||||
// for calls that deliberately rely on implicitly declared functions.
|
||||
private predicate hasImplicitTarget(RelevantCall call) {
|
||||
call.getTarget().getADeclarationEntry().isImplicit()
|
||||
}
|
||||
|
||||
int getNumberOfOk() { result = count(RelevantCall call | not hasImplicitTarget(call)) }
|
||||
|
||||
int getNumberOfNotOk() { result = count(RelevantCall call | hasImplicitTarget(call)) }
|
||||
|
||||
string getOkText() { result = "calls with call target" }
|
||||
|
||||
string getNotOkText() { result = "calls with missing call target" }
|
||||
}
|
||||
|
||||
module CallTargetStatsReport = ReportStats<CallTargetStats>;
|
||||
25
cpp/ql/src/Telemetry/ExtractorInformation.ql
Normal file
25
cpp/ql/src/Telemetry/ExtractorInformation.ql
Normal file
@@ -0,0 +1,25 @@
|
||||
/**
|
||||
* @name C/C++ extraction information
|
||||
* @description Information about the extraction for a C/C++ database
|
||||
* @kind metric
|
||||
* @tags summary telemetry
|
||||
* @id cpp/telemetry/extraction-information
|
||||
*/
|
||||
|
||||
import cpp
|
||||
import DatabaseQuality
|
||||
|
||||
from string key, float value
|
||||
where
|
||||
(
|
||||
CallTargetStatsReport::numberOfOk(key, value) or
|
||||
CallTargetStatsReport::numberOfNotOk(key, value) or
|
||||
CallTargetStatsReport::percentageOfOk(key, value)
|
||||
) and
|
||||
/* Infinity */
|
||||
value != 1.0 / 0.0 and
|
||||
/* -Infinity */
|
||||
value != -1.0 / 0.0 and
|
||||
/* NaN */
|
||||
value != 0.0 / 0.0
|
||||
select key, value
|
||||
@@ -0,0 +1,4 @@
|
||||
---
|
||||
category: minorAnalysis
|
||||
---
|
||||
* Fixed an issue with the "Multiplication result converted to larger type" (`cpp/integer-multiplication-cast-to-long`) query causing false positive results in `build-mode: none` databases.
|
||||
@@ -0,0 +1,4 @@
|
||||
---
|
||||
category: queryMetadata
|
||||
---
|
||||
* The `@security-severity` metadata of `cpp/cgi-xss` has been increased from 6.1 (medium) to 7.8 (high).
|
||||
@@ -0,0 +1,4 @@
|
||||
---
|
||||
category: minorAnalysis
|
||||
---
|
||||
* Fixed an issue with the "Wrong type of arguments to formatting function" (`cpp/wrong-type-format-argument`) query causing false positive results in `build-mode: none` databases.
|
||||
@@ -0,0 +1,4 @@
|
||||
---
|
||||
category: minorAnalysis
|
||||
---
|
||||
* Fixed an issue with the "Suspicious add with sizeof" (`cpp/suspicious-add-sizeof`) query causing false positive results in `build-mode: none` databases.
|
||||
@@ -0,0 +1,4 @@
|
||||
---
|
||||
category: minorAnalysis
|
||||
---
|
||||
* Fixed an issue with the "Uncontrolled format string" (`cpp/tainted-format-string`) query involving certain kinds of formatting function implementations.
|
||||
3
cpp/ql/src/change-notes/released/1.5.13.md
Normal file
3
cpp/ql/src/change-notes/released/1.5.13.md
Normal file
@@ -0,0 +1,3 @@
|
||||
## 1.5.13
|
||||
|
||||
No user-facing changes.
|
||||
@@ -1,2 +1,2 @@
|
||||
---
|
||||
lastReleaseVersion: 1.5.12
|
||||
lastReleaseVersion: 1.5.13
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
name: codeql/cpp-queries
|
||||
version: 1.5.13-dev
|
||||
version: 1.5.14-dev
|
||||
groups:
|
||||
- cpp
|
||||
- queries
|
||||
|
||||
@@ -0,0 +1,28 @@
|
||||
// semmle-extractor-options: --expect_errors
|
||||
|
||||
void test_float_double1(float f, double d) {
|
||||
float r1 = f * f; // GOOD
|
||||
float r2 = f * d; // GOOD
|
||||
double r3 = f * f; // BAD
|
||||
double r4 = f * d; // GOOD
|
||||
|
||||
float f1 = fabsf(f * f); // GOOD
|
||||
float f2 = fabsf(f * d); // GOOD
|
||||
double f3 = fabs(f * f); // BAD [NOT DETECTED]
|
||||
double f4 = fabs(f * d); // GOOD
|
||||
}
|
||||
|
||||
double fabs(double f);
|
||||
float fabsf(float f);
|
||||
|
||||
void test_float_double2(float f, double d) {
|
||||
float r1 = f * f; // GOOD
|
||||
float r2 = f * d; // GOOD
|
||||
double r3 = f * f; // BAD
|
||||
double r4 = f * d; // GOOD
|
||||
|
||||
float f1 = fabsf(f * f); // GOOD
|
||||
float f2 = fabsf(f * d); // GOOD
|
||||
double f3 = fabs(f * f); // BAD [NOT DETECTED]
|
||||
double f4 = fabs(f * d); // GOOD
|
||||
}
|
||||
@@ -1,3 +1,5 @@
|
||||
| Buildless.c:6:17:6:21 | ... * ... | Multiplication result may overflow 'float' before it is converted to 'double'. |
|
||||
| Buildless.c:21:17:21:21 | ... * ... | Multiplication result may overflow 'float' before it is converted to 'double'. |
|
||||
| IntMultToLong.c:4:10:4:14 | ... * ... | Multiplication result may overflow 'int' before it is converted to 'long long'. |
|
||||
| IntMultToLong.c:7:16:7:20 | ... * ... | Multiplication result may overflow 'int' before it is converted to 'long long'. |
|
||||
| IntMultToLong.c:18:19:18:23 | ... * ... | Multiplication result may overflow 'float' before it is converted to 'double'. |
|
||||
|
||||
@@ -1 +1,3 @@
|
||||
| second.cpp:26:18:26:39 | ... - ... | This format specifier for type 'int' does not match the argument type 'long'. |
|
||||
| second.cpp:29:18:29:39 | ... - ... | This format specifier for type 'unsigned int' does not match the argument type 'long'. |
|
||||
| tests.c:7:18:7:18 | 1 | This format specifier for type 'char *' does not match the argument type 'int'. |
|
||||
|
||||
@@ -0,0 +1,3 @@
|
||||
|
||||
// defines type size_t plausibly
|
||||
typedef unsigned long size_t;
|
||||
@@ -0,0 +1,32 @@
|
||||
// semmle-extractor-options: --expect_errors
|
||||
|
||||
int printf(const char * format, ...);
|
||||
|
||||
// defines type `myFunctionPointerType`, referencing `size_t`
|
||||
typedef size_t (*myFunctionPointerType) ();
|
||||
|
||||
void test_size_t() {
|
||||
size_t s = 0;
|
||||
|
||||
printf("%zd", s); // GOOD
|
||||
printf("%zi", s); // GOOD
|
||||
printf("%zu", s); // GOOD (we generally permit signedness changes)
|
||||
printf("%zx", s); // GOOD (we generally permit signedness changes)
|
||||
printf("%d", s); // BAD [NOT DETECTED]
|
||||
printf("%ld", s); // DUBIOUS [NOT DETECTED]
|
||||
printf("%lld", s); // DUBIOUS [NOT DETECTED]
|
||||
printf("%u", s); // BAD [NOT DETECTED]
|
||||
|
||||
char buffer[1024];
|
||||
|
||||
printf("%zd", &buffer[1023] - buffer); // GOOD
|
||||
printf("%zi", &buffer[1023] - buffer); // GOOD
|
||||
printf("%zu", &buffer[1023] - buffer); // GOOD
|
||||
printf("%zx", &buffer[1023] - buffer); // GOOD
|
||||
printf("%d", &buffer[1023] - buffer); // BAD
|
||||
printf("%ld", &buffer[1023] - buffer); // DUBIOUS [NOT DETECTED]
|
||||
printf("%lld", &buffer[1023] - buffer); // DUBIOUS [NOT DETECTED]
|
||||
printf("%u", &buffer[1023] - buffer); // BAD
|
||||
// (for the `%ld` and `%lld` cases, the signedness and type sizes match, `%zd` would be most correct
|
||||
// and robust but the developer may know enough to make this safe)
|
||||
}
|
||||
@@ -1,3 +1,5 @@
|
||||
| buildless.cpp:5:15:5:25 | sizeof(int) | Suspicious sizeof offset in a pointer arithmetic expression. The type of the pointer is $@. | file://:0:0:0:0 | const short * | const short * |
|
||||
| buildless.cpp:6:13:6:23 | sizeof(int) | Suspicious sizeof offset in a pointer arithmetic expression. The type of the pointer is $@. | file://:0:0:0:0 | const int * | const int * |
|
||||
| test.cpp:6:30:6:40 | sizeof(int) | Suspicious sizeof offset in a pointer arithmetic expression. The type of the pointer is $@. | file://:0:0:0:0 | int * | int * |
|
||||
| test.cpp:14:30:14:40 | sizeof(int) | Suspicious sizeof offset in a pointer arithmetic expression. The type of the pointer is $@. | file://:0:0:0:0 | int * | int * |
|
||||
| test.cpp:22:25:22:35 | sizeof(int) | Suspicious sizeof offset in a pointer arithmetic expression. The type of the pointer is $@. | file://:0:0:0:0 | int * | int * |
|
||||
|
||||
@@ -0,0 +1,10 @@
|
||||
// semmle-extractor-options: --expect_errors
|
||||
|
||||
void test_buildless(const char *p_c, const short *p_short, const int *p_int, const uint8_t *p_8, const uint16_t *p_16, const uint32_t *p_32) {
|
||||
*(p_c + sizeof(int)); // GOOD (`sizeof(char)` is 1)
|
||||
*(p_short + sizeof(int)); // BAD
|
||||
*(p_int + sizeof(int)); // BAD
|
||||
*(p_8 + sizeof(int)); // GOOD (`sizeof(uint8_t)` is 1, but there's an error in the type)
|
||||
*(p_16 + sizeof(int)); // BAD [NOT DETECTED]
|
||||
*(p_32 + sizeof(int)); // BAD [NOT DETECTED]
|
||||
}
|
||||
@@ -93,3 +93,9 @@ private:
|
||||
myChar * const myCharsPointer;
|
||||
myInt * const myIntsPointer;
|
||||
};
|
||||
|
||||
typedef unsigned char uint8_t;
|
||||
typedef unsigned short uint16_t;
|
||||
typedef unsigned int uint32_t;
|
||||
|
||||
void test_buildless(const char *p_c, const short *p_short, const int *p_int, const uint8_t *p_8, const uint16_t *p_16, const uint32_t *p_32);
|
||||
|
||||
Reference in New Issue
Block a user