Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cpp/ql/lib/Options.qll
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class CustomOptions extends Options {
override predicate returnsNull(Call call) { Options.super.returnsNull(call) }

/**
* Holds if a call to this function will never return.
* Holds if a call to the function `f` will never return.
*
* By default, this holds for `exit`, `_exit`, `abort`, `__assert_fail`,
* `longjmp`, `error`, `__builtin_unreachable` and any function with a
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -298,10 +298,11 @@ private predicate boundFlowStep(Instruction i, NonPhiOperand op, int delta, bool
else
if strictlyNegative(x)
then upper = true and delta = -1
else
if negative(x)
then upper = true and delta = 0
else none()
else (
negative(x) and
upper = true and
delta = 0
)
)
or
exists(Operand x |
Expand All @@ -321,10 +322,11 @@ private predicate boundFlowStep(Instruction i, NonPhiOperand op, int delta, bool
else
if strictlyNegative(x)
then upper = false and delta = 1
else
if negative(x)
then upper = false and delta = 0
else none()
else (
negative(x) and
upper = false and
delta = 0
)
)
or
i.(RemInstruction).getRightOperand() = op and positive(op) and delta = -1 and upper = true
Expand Down
4 changes: 2 additions & 2 deletions cpp/ql/lib/semmle/code/cpp/Concept.qll
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ class ConceptIdExpr extends Expr, @concept_id {
final Locatable getATemplateArgumentKind() { result = this.getTemplateArgumentKind(_) }

/**
* Gets the `i`th template argument passed to the concept.
* Gets template argument at index `index` passed to the concept, if any.
*
* For example, if:
* ```cpp
Expand All @@ -219,7 +219,7 @@ class ConceptIdExpr extends Expr, @concept_id {
}

/**
* Gets the kind of the `i`th template argument value passed to the concept.
* Gets the kind of the template argument value at index `index` passed to the concept, if any.
*
* For example, if:
* ```cpp
Expand Down
10 changes: 5 additions & 5 deletions cpp/ql/lib/semmle/code/cpp/Declaration.qll
Original file line number Diff line number Diff line change
Expand Up @@ -223,8 +223,8 @@ class Declaration extends Locatable, @declaration {
final Locatable getATemplateArgumentKind() { result = this.getTemplateArgumentKind(_) }

/**
* Gets the `i`th template argument used to instantiate this declaration from a
* template.
* Gets the template argument at index `index` used to instantiate this declaration from a
* template, if any.
*
* For example:
*
Expand All @@ -245,9 +245,9 @@ class Declaration extends Locatable, @declaration {
}

/**
* Gets the `i`th template argument value used to instantiate this declaration
* from a template. When called on a template, this will return the `i`th template
* parameter value if it exists.
* Gets the template argument value at index `index` used to instantiate this declaration
* from a template. When called on a template, this will return the template
* parameter value at index `index` if it exists.
*
* For example:
*
Expand Down
4 changes: 2 additions & 2 deletions cpp/ql/lib/semmle/code/cpp/commons/Printf.qll
Original file line number Diff line number Diff line change
Expand Up @@ -877,7 +877,7 @@ class FormatLiteral extends Literal instanceof StringLiteral {
}

/**
* Gets the char type required by the nth conversion specifier.
* Gets the char type required by the `n`th conversion specifier.
* - in the base case this is the default for the formatting function
* (e.g. `char` for `printf`, `char` or `wchar_t` for `wprintf`).
* - the `%C` format character reverses wideness.
Expand Down Expand Up @@ -922,7 +922,7 @@ class FormatLiteral extends Literal instanceof StringLiteral {
}

/**
* Gets the string type required by the nth conversion specifier.
* Gets the string type required by the `n`th conversion specifier.
* - in the base case this is the default for the formatting function
* (e.g. `char *` for `printf`, `char *` or `wchar_t *` for `wprintf`).
* - the `%S` format character reverses wideness on some platforms.
Expand Down
4 changes: 2 additions & 2 deletions cpp/ql/lib/semmle/code/cpp/controlflow/Dominance.qll
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ predicate postDominates(ControlFlowNode postDominator, ControlFlowNode node) {
*/

/**
* Holds if `dominator` is an immediate dominator of `node` in the control-flow
* Holds if `dom` is an immediate dominator of `node` in the control-flow
* graph of basic blocks.
*/
predicate bbIDominates(BasicBlock dom, BasicBlock node) =
Expand All @@ -117,7 +117,7 @@ private predicate bb_predecessor(BasicBlock succ, BasicBlock pred) { bb_successo
private predicate bb_exit(ExitBasicBlock exit) { any() }

/**
* Holds if `postDominator` is an immediate post-dominator of `node` in the control-flow
* Holds if `pDom` is an immediate post-dominator of `node` in the control-flow
* graph of basic blocks.
*/
predicate bbIPostDominates(BasicBlock pDom, BasicBlock node) =
Expand Down
4 changes: 2 additions & 2 deletions cpp/ql/lib/semmle/code/cpp/controlflow/internal/CFG.qll
Original file line number Diff line number Diff line change
Expand Up @@ -1042,8 +1042,8 @@ private predicate subEdgeIncludingDestructors(Pos p1, Node n1, Node n2, Pos p2)
* - `MicrosoftTryFinallyStmt`: On the edge following the `__finally` block for
* the case where an exception was thrown and needs to be propagated.
*/
DestructorCall getSynthesisedDestructorCallAfterNode(Node n, int i) {
synthetic_destructor_call(n, i, result)
DestructorCall getSynthesisedDestructorCallAfterNode(Node node, int index) {
synthetic_destructor_call(node, index, result)
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -829,8 +829,8 @@ class ContentSet instanceof Content {

/**
* Holds if this element is at the specified location.
* The location spans column `startcolumn` of line `startline` to
* column `endcolumn` of line `endline` in file `filepath`.
* The location spans column `sc` of line `sl` to
* column `ec` of line `el` in file `path`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should just rename the predicate arguments here. This makes things less clear.

* For more information, see
* [Locations](https://codeql.github.com/docs/writing-codeql-queries/providing-locations-in-codeql-queries/).
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2268,8 +2268,8 @@ class ContentSet instanceof Content {

/**
* Holds if this element is at the specified location.
* The location spans column `startcolumn` of line `startline` to
* column `endcolumn` of line `endline` in file `filepath`.
* The location spans column `sc` of line `sl` to
* column `ec` of line `el` in file `path`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above, rename the arguments.

* For more information, see
* [Locations](https://codeql.github.com/docs/writing-codeql-queries/providing-locations-in-codeql-queries/).
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ CppType getEllipsisVariablePRValueType() {
CppType getEllipsisVariableGLValueType() { result = getTypeForGLValue(any(UnknownType t)) }

/**
* Holds if the function returns a value, as opposed to returning `void`.
* Holds if the function `func` returns a value, as opposed to returning `void`.
*/
predicate hasReturnValue(Function func) { not func.getUnspecifiedType() instanceof VoidType }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -601,7 +601,7 @@ class TranslatedReturnVoidStmt extends TranslatedReturnStmt {
* The IR translation of an implicit `return` statement generated by the extractor to handle control
* flow that reaches the end of a non-`void`-returning function body. Such control flow
* produces undefined behavior in C++ but not in C. However even in C using the return value is
* undefined behaviour. We make it return uninitialized memory to get as much flow as possible.
* undefined behavior. We make it return uninitialized memory to get as much flow as possible.
*/
class TranslatedNoValueReturnStmt extends TranslatedReturnStmt, TranslatedVariableInitialization {
TranslatedNoValueReturnStmt() {
Expand Down
3 changes: 2 additions & 1 deletion cpp/ql/lib/semmle/code/cpp/ir/internal/IRUtilities.qll
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ Type getVariableType(Variable v) {
}

/**
* Holds if the database contains a `case` label with the specified minimum and maximum value.
* Holds if the database contains a `switchCase` label with the specified minimum `minValue`
* and maximum `maxValue` value.
*/
predicate hasCaseEdge(SwitchCase switchCase, string minValue, string maxValue) {
minValue = switchCase.getExpr().getFullyConverted().getValue() and
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@ class FunctionOutput extends TFunctionOutput {
/**
* Holds if this is the output value pointed to by a pointer parameter to a function, or the
* output value referred to by a reference parameter to a function, where the parameter has
* index `index`.
* index `i`.
*
* Example:
* ```
Expand All @@ -389,7 +389,7 @@ class FunctionOutput extends TFunctionOutput {
/**
* Holds if this is the output value pointed to by a pointer parameter (through `ind` number
* of indirections) to a function, or the output value referred to by a reference parameter to
* a function, where the parameter has index `index`.
* a function, where the parameter has index `i`.
*
* Example:
* ```
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -307,13 +307,12 @@ class SemStoreExpr extends SemUnaryExpr {
}

class SemConditionalExpr extends SemKnownExpr {
SemExpr condition;
SemExpr trueResult;
SemExpr falseResult;

SemConditionalExpr() {
opcode instanceof Opcode::Conditional and
Specific::conditionalExpr(this, type, condition, trueResult, falseResult)
Specific::conditionalExpr(this, type, any(SemExpr condition), trueResult, falseResult)
}

final SemExpr getBranchExpr(boolean branch) {
Expand Down
4 changes: 3 additions & 1 deletion cpp/ql/lib/semmle/code/cpp/security/FileWrite.qll
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ class FileWrite extends Expr {
Expr getDest() { fileWrite(this, _, result) }

/**
* Gets the conversion character for this write, if it exists and is known. For example in the following code the write of `value1` has conversion character `"s"`, whereas the write of `value2` has no conversion specifier.
* Gets the conversion character from `source` for this write, if it exists and is known.
* For example in the following code the write of `value1` has conversion character `"s"`, whereas
* the write of `value2` has no conversion specifier.
* ```
* fprintf(file, "%s", value1);
* stream << value2;
Expand Down
14 changes: 11 additions & 3 deletions cpp/ql/lib/semmle/code/cpp/security/boostorg/asio/protocols.qll
Original file line number Diff line number Diff line change
Expand Up @@ -191,11 +191,19 @@ module BoostorgAsio {
class SslContextClass extends Class {
SslContextClass() { this.getQualifiedName() = "boost::asio::ssl::context" }

ConstructorCall getAContructorCall() {
/**
* Gets a constructor call, if any.
*/
ConstructorCall getAConstructorCall() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs a change note.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok

this.getAConstructor().getACallToThisFunction() = result and
not result.getLocation().getFile().toString().matches("%/boost/asio/%") and
result.fromSource()
}

/**
* DEPRECATED: Use `getAConstructorCall` instead.
*/
deprecated ConstructorCall getAContructorCall() { result = this.getAConstructorCall() }
}

/**
Expand Down Expand Up @@ -368,7 +376,7 @@ module BoostorgAsio {
*/
default predicate isSink(DataFlow::Node sink) {
exists(ConstructorCall cc, SslContextClass c, Expr e | e = sink.asExpr() |
c.getAContructorCall() = cc and
c.getAConstructorCall() = cc and
cc.getArgument(0) = e
)
}
Expand Down Expand Up @@ -468,7 +476,7 @@ module BoostorgAsio {
predicate isSource(DataFlow::Node source) {
exists(SslContextClass c, ConstructorCall cc |
cc = source.asExpr() and
c.getAContructorCall() = cc
c.getAConstructorCall() = cc
)
}

Expand Down
2 changes: 1 addition & 1 deletion cpp/ql/lib/semmle/code/cpp/valuenumbering/HashCons.qll
Original file line number Diff line number Diff line change
Expand Up @@ -648,7 +648,7 @@ private predicate mk_UuidofOperator(Type t, UuidofOperator e) {
}

private predicate analyzableTypeidType(TypeidOperator e) {
count(e.getAChild()) = 0 and
not exists(e.getAChild()) and
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not a library that we test against in DCA, but it is used by the field team e.g., so if we want to change this we need to make sure in some way that it's not changing performance.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverting.

strictcount(e.getResultType()) = 1
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,12 +164,17 @@ predicate valueOccurrenceCount(string value, int n) {
n > 20
}

predicate occurenceCount(Literal lit, string value, int n) {
predicate occurrenceCount(Literal lit, string value, int n) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs a change note.

valueOccurrenceCount(value, n) and
value = lit.getValue() and
nonTrivialValue(_, lit)
}

/**
* DEPRECATED: Use `occurrenceCount` instead.
*/
deprecated predicate occurenceCount = occurrenceCount/3;

/*
* Literals repeated frequently
*/
Expand All @@ -178,7 +183,7 @@ predicate check(Literal lit, string value, int n, File f) {
// Check that the literal is nontrivial
not trivial(lit) and
// Check that it is repeated a number of times
occurenceCount(lit, value, n) and
occurrenceCount(lit, value, n) and
n > 20 and
f = lit.getFile() and
// Exclude generated files
Expand Down
15 changes: 11 additions & 4 deletions cpp/ql/src/Likely Bugs/Leap Year/LeapYear.qll
Original file line number Diff line number Diff line change
Expand Up @@ -128,11 +128,18 @@ abstract class LeapYearFieldAccess extends YearFieldAccess {
/**
* Holds if the top-level binary operation includes an addition or subtraction operator with an operand specified by `valueToCheck`.
*/
predicate additionalAdditionOrSubstractionCheckForLeapYear(int valueToCheck) {
predicate additionalAdditionOrSubtractionCheckForLeapYear(int valueToCheck) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs a change note.

additionalLogicalCheck(this, "+", valueToCheck) or
additionalLogicalCheck(this, "-", valueToCheck)
}

/**
* DEPRECATED: Use `additionalAdditionOrSubtractionCheckForLeapYear` instead.
*/
deprecated predicate additionalAdditionOrSubstractionCheckForLeapYear(int valueToCheck) {
this.additionalAdditionOrSubtractionCheckForLeapYear(valueToCheck)
}

/**
* Holds if this object is used on a modulus 4 operation, which would likely indicate the start of a leap year check.
*/
Expand Down Expand Up @@ -180,13 +187,13 @@ class StructTmLeapYearFieldAccess extends LeapYearFieldAccess {
this.additionalModulusCheckForLeapYear(100) and
// tm_year represents years since 1900
(
this.additionalAdditionOrSubstractionCheckForLeapYear(1900)
this.additionalAdditionOrSubtractionCheckForLeapYear(1900)
or
// some systems may use 2000 for 2-digit year conversions
this.additionalAdditionOrSubstractionCheckForLeapYear(2000)
this.additionalAdditionOrSubtractionCheckForLeapYear(2000)
or
// converting from/to Unix epoch
this.additionalAdditionOrSubstractionCheckForLeapYear(1970)
this.additionalAdditionOrSubtractionCheckForLeapYear(1970)
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import cpp
import semmle.code.cpp.security.boostorg.asio.protocols

predicate isSourceImpl(DataFlow::Node source, ConstructorCall cc) {
exists(BoostorgAsio::SslContextClass c | c.getAContructorCall() = cc and cc = source.asExpr())
exists(BoostorgAsio::SslContextClass c | c.getAConstructorCall() = cc and cc = source.asExpr())
}

predicate isSinkImpl(DataFlow::Node sink, FunctionCall fcSetOptions) {
Expand Down
4 changes: 2 additions & 2 deletions cpp/ql/src/Metrics/Internal/CallableExtents.ql
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ import cpp
class RangeFunction extends Function {
/**
* Holds if this function is at the specified location.
* The location spans column `startcolumn` of line `startline` to
* column `endcolumn` of line `endline` in file `filepath`.
* The location spans column `sc` of line `sl` to
* column `ec` of line `el` in file `path`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As earlier, rename the arguments.

* For more information, see
* [Locations](https://codeql.github.com/docs/writing-codeql-queries/providing-locations-in-codeql-queries/).
*/
Expand Down
8 changes: 4 additions & 4 deletions cpp/ql/src/Security/CWE/CWE-190/IntegerOverflowTainted.ql
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@ import semmle.code.cpp.controlflow.IRGuards as IRGuards
predicate outOfBoundsExpr(Expr expr, string kind) {
if convertedExprMightOverflowPositively(expr)
then kind = "overflow"
else
if convertedExprMightOverflowNegatively(expr)
then kind = "overflow negatively"
else none()
else (
convertedExprMightOverflowNegatively(expr) and
kind = "overflow negatively"
)
}

predicate isSource(FS::FlowSource source, string sourceType) { sourceType = source.getSourceType() }
Expand Down
Loading