Skip to content

CPP: Clean up new queries and libraries #2274

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jbj merged 6 commits into from
Nov 11, 2019
Merged

CPP: Clean up new queries and libraries #2274

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
399ac1f
CPP: Rename 'getAssertedFalseCondition' to something less misleading.
geoffw0 Nov 6, 2019
0c3f4e5
CPP: Make some library predicates private.
geoffw0 Nov 6, 2019
81c58d5
CPP: Improve QLDoc comments.
geoffw0 Nov 6, 2019
e886cf7
CPP: 'i.e.' -> 'that is'.
geoffw0 Nov 6, 2019
6c38f55
CPP: QLDoc protocols.qll.
geoffw0 Nov 6, 2019
58b6fc6
CPP: Autoformat.
geoffw0 Nov 8, 2019
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/src/Microsoft/SAL.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ class SALParameter extends Parameter {
}

/**
* A SAL element, i.e. a SAL annotation or a declaration entry
* A SAL element, that is, a SAL annotation or a declaration entry
* that may have SAL annotations.
*/
library class SALElement extends Element {
Expand Down
18 changes: 9 additions & 9 deletions cpp/ql/src/Security/CWE/CWE-457/InitializationFunctions.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,9 @@ class ParameterNullCheck extends ParameterCheck {
(
va = this.(NotExpr).getOperand() or
va = any(EQExpr eq | eq = this and eq.getAnOperand().getValue() = "0").getAnOperand() or
va = getAssertedFalseCondition(this) or
va = getCheckedFalseCondition(this) or
va = any(NEExpr eq |
eq = getAssertedFalseCondition(this) and eq.getAnOperand().getValue() = "0"
eq = getCheckedFalseCondition(this) and eq.getAnOperand().getValue() = "0"
).getAnOperand()
)
or
Expand All @@ -101,7 +101,7 @@ class ParameterNullCheck extends ParameterCheck {
va = this or
va = any(NEExpr eq | eq = this and eq.getAnOperand().getValue() = "0").getAnOperand() or
va = any(EQExpr eq |
eq = getAssertedFalseCondition(this) and eq.getAnOperand().getValue() = "0"
eq = getCheckedFalseCondition(this) and eq.getAnOperand().getValue() = "0"
).getAnOperand()
)
)
Expand Down Expand Up @@ -567,7 +567,7 @@ Expr getAnInitializedArgument(Call call) { result = call.getArgument(initialized
* the call, under the given context and evidence.
*/
pragma[nomagic]
int conditionallyInitializedArgument(
private int conditionallyInitializedArgument(
Call call, ConditionalInitializationFunction target, Context c, Evidence e
) {
target = getTarget(call) and
Expand All @@ -588,7 +588,7 @@ Expr getAConditionallyInitializedArgument(
/**
* Gets the type signature for the functions parameters.
*/
string typeSig(Function f) {
private string typeSig(Function f) {
result = concat(int i, Type pt |
pt = f.getParameter(i).getType()
|
Expand All @@ -599,7 +599,7 @@ string typeSig(Function f) {
/**
* Holds where qualifiedName and typeSig make up the signature for the function.
*/
predicate functionSignature(Function f, string qualifiedName, string typeSig) {
private predicate functionSignature(Function f, string qualifiedName, string typeSig) {
qualifiedName = f.getQualifiedName() and
typeSig = typeSig(f)
}
Expand All @@ -611,7 +611,7 @@ predicate functionSignature(Function f, string qualifiedName, string typeSig) {
* This is useful for identifying call to target dependencies across libraries, where the libraries
* are never statically linked together.
*/
Function getAPossibleDefinition(Function undefinedFunction) {
private Function getAPossibleDefinition(Function undefinedFunction) {
not undefinedFunction.isDefined() and
exists(string qn, string typeSig |
functionSignature(undefinedFunction, qn, typeSig) and functionSignature(result, qn, typeSig)
Expand Down Expand Up @@ -669,15 +669,15 @@ FieldAccess getAFieldAccess(Variable v) {
}

/**
* Gets a condition which is asserted to be false by the given `ne` expression, according to this pattern:
* Gets a condition which is checked to be false by the given `ne` expression, according to this pattern:
* ```
* int a = !!result;
* if (!a) { // <- ne
* ....
* }
* ```
*/
Expr getAssertedFalseCondition(NotExpr ne) {
private Expr getCheckedFalseCondition(NotExpr ne) {
exists(LocalVariable v |
result = v.getInitializer().getExpr().(NotExpr).getOperand().(NotExpr).getOperand() and
ne.getOperand() = v.getAnAccess() and
Expand Down
7 changes: 3 additions & 4 deletions cpp/ql/src/semmle/code/cpp/NestedFields.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import cpp

/**
* Gets a `Field` that is nested within the given `Struct`.
*
* This identifies `Field`s which are located in the same memory
* Gets a `Field` that is within the given `Struct`, either directly or nested
* inside one or more levels of member structs.
*/
private Field getANestedField(Struct s) {
result = s.getAField()
Expand All @@ -15,7 +14,7 @@ private Field getANestedField(Struct s) {
}

/**
* Unwraps a series of field accesses to determine the inner-most qualifier.
* Unwraps a series of field accesses to determine the outer-most qualifier.
*/
private Expr getUltimateQualifier(FieldAccess fa) {
exists(Expr qualifier | qualifier = fa.getQualifier() |
Expand Down
2 changes: 1 addition & 1 deletion cpp/ql/src/semmle/code/cpp/dispatch/VirtualDispatch.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ module VirtualDispatch {

/**
* Holds if `c` cannot inherit the member function `f`,
* i.e. `c` or one of its supertypes overrides `f`.
* that is, `c` or one of its supertypes overrides `f`.
*/
private predicate cannotInherit(Class c, MemberFunction f) {
exists(Class overridingType, MemberFunction override |
Expand Down
60 changes: 32 additions & 28 deletions cpp/ql/src/semmle/code/cpp/security/boostorg/asio/protocols.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import semmle.code.cpp.dataflow.DataFlow

module BoostorgAsio {
/**
* Represents boost::asio::ssl::context enum
* Represents the `boost::asio::ssl::context` enum.
*/
class SslContextMethod extends Enum {
SslContextMethod() {
Expand All @@ -12,7 +12,7 @@ module BoostorgAsio {
}

/**
* returns the value for a banned protocol
* Gets an enumeration constant for a banned protocol.
*/
EnumConstant getABannedProtocolConstant() {
result = this.getAnEnumConstant() and
Expand Down Expand Up @@ -56,14 +56,15 @@ module BoostorgAsio {
}

/**
* returns the value for a approved protocols, but that are hard-coded (i.e. no protocol negotiation)
* Gets an enumeration constant for an approved protocol, that is hard-coded
* (no protocol negotiation).
*/
EnumConstant getAnApprovedButHardcodedProtocolConstant() {
result = this.getATls12ProtocolConstant()
}

/**
* returns the value for a TLS v1.2 protocol
* Gets an enumeration constant for a TLS v1.2 protocol.
*/
EnumConstant getATls12ProtocolConstant() {
result = this.getAnEnumConstant() and
Expand All @@ -80,7 +81,7 @@ module BoostorgAsio {
}

/**
* returns the value for a TLS v1.3 protocol
* Gets an enumeration constant for a TLS v1.3 protocol.
*/
EnumConstant getATls13ProtocolConstant() {
result = this.getAnEnumConstant() and
Expand All @@ -97,7 +98,7 @@ module BoostorgAsio {
}

/**
* returns the value of a generic TLS or SSL/TLS protocol
* Gets an enumeration constant for a generic TLS or SSL/TLS protocol.
*/
EnumConstant getAGenericTlsProtocolConstant() {
result = this.getAnEnumConstant() and
Expand All @@ -116,7 +117,7 @@ module BoostorgAsio {
}

/**
* returns the value of a generic SSL/TLS protocol
* Gets an enumeration constant for a generic SSL/TLS protocol.
*/
EnumConstant getASslv23ProtocolConstant() {
result = this.getAnEnumConstant() and
Expand All @@ -135,55 +136,57 @@ module BoostorgAsio {
}

/**
* NOTE: ignore - Modern versions of OpenSSL do not support SSL v2 anymore, so this option is for backwards compatibility only
* Gets the value for the no_sslv2 constant, right shifted by 16 bits.
*
* Note that modern versions of OpelSSL do not support SSL v2, so this option is for backwards compatibility only.
*/
int getShiftedSslOptionsNoSsl2() {
// SSL_OP_NO_SSLv2 was removed from modern OpenSSL versions
result = 0
}

/**
* RightShift(16) value for no_sslv3 constant
* Gets the value for the no_sslv3 constant, right shifted by 16 bits.
*/
int getShiftedSslOptionsNoSsl3() {
// SSL_OP_NO_SSLv3 == 0x02000000U
result = 512
}

/**
* RightShift(16) value for no_tlsv1 constant
* Gets the value for the no_tlsv1 constant, right shifted by 16 bits.
*/
int getShiftedSslOptionsNoTls1() {
// SSL_OP_NO_TLSv1 == 0x04000000U
result = 1024
}

/**
* RightShift(16) value for no_tlsv1_1 constant
* Gets the value for the no_tlsv1_1 constant, right shifted by 16 bits.
*/
int getShiftedSslOptionsNoTls1_1() {
// SSL_OP_NO_TLSv1_1 == 0x10000000U
result = 4096
}

/**
* RightShift(16) value for no_tlsv1_2 constant
* Gets the value for the no_tlsv1_2 constant, right shifted by 16 bits.
*/
int getShiftedSslOptionsNoTls1_2() {
// SSL_OP_NO_TLSv1_2 == 0x08000000U
result = 2048
}

/**
* RightShift(16) value for no_tlsv1_3 constant
* Gets the value for the no_tlsv1_3 constant, right shifted by 16 bits.
*/
int getShiftedSslOptionsNoTls1_3() {
// SSL_OP_NO_TLSv1_2 == 0x20000000U
result = 8192
}

/**
* Represents boost::asio::ssl::context class
* Represents the `boost::asio::ssl::context` class.
*/
class SslContextClass extends Class {
SslContextClass() { this.getQualifiedName() = "boost::asio::ssl::context" }
Expand All @@ -196,7 +199,7 @@ module BoostorgAsio {
}

/**
* Represents boost::asio::ssl::context::set_options member function
* Represents `boost::asio::ssl::context::set_options` member function.
*/
class SslSetOptionsFunction extends Function {
SslSetOptionsFunction() {
Expand All @@ -205,7 +208,7 @@ module BoostorgAsio {
}

/**
* holds if the expression represents a banned protocol
* Holds if the expression represents a banned protocol.
*/
predicate isExprBannedBoostProtocol(Expr e) {
exists(Literal va | va = e |
Expand Down Expand Up @@ -244,7 +247,7 @@ module BoostorgAsio {
}

/**
* holds if the expression represents a TLS v1.2 protocol
* Holds if the expression represents a TLS v1.2 protocol.
*/
predicate isExprTls12BoostProtocol(Expr e) {
exists(Literal va | va = e |
Expand All @@ -269,7 +272,7 @@ module BoostorgAsio {
}

/**
* holds if the expression represents a protocol that requires Crypto Board approval
* Holds if the expression represents a protocol that requires Crypto Board approval.
*/
predicate isExprTls13BoostProtocol(Expr e) {
exists(Literal va | va = e |
Expand All @@ -294,7 +297,7 @@ module BoostorgAsio {
}

/**
* holds if the expression represents a generic TLS or SSL/TLS protocol
* Holds if the expression represents a generic TLS or SSL/TLS protocol.
*/
predicate isExprTlsBoostProtocol(Expr e) {
exists(Literal va | va = e |
Expand Down Expand Up @@ -325,7 +328,7 @@ module BoostorgAsio {
}

/**
* holds if the expression represents a generic SSl/TLS protocol
* Holds if the expression represents a generic SSl/TLS protocol.
*/
predicate isExprSslV23BoostProtocol(Expr e) {
exists(Literal va | va = e |
Expand All @@ -351,7 +354,8 @@ module BoostorgAsio {

//////////////////////// Dataflow /////////////////////
/**
* Abstract - Protocol value Flows to the first argument of the context constructor
* Abstract class for flows of protocol values to the first argument of a context
* constructor.
*/
abstract class SslContextCallAbstractConfig extends DataFlow::Configuration {
bindingset[this]
Expand All @@ -366,7 +370,7 @@ module BoostorgAsio {
}

/**
* any Protocol value Flows to the first argument of the context constructor
* Any protocol value that flows to the first argument of a context constructor.
*/
class SslContextCallConfig extends SslContextCallAbstractConfig {
SslContextCallConfig() { this = "SslContextCallConfig" }
Expand All @@ -380,7 +384,7 @@ module BoostorgAsio {
}

/**
* a banned protocol value Flows to the first argument of the context constructor
* A banned protocol value that flows to the first argument of a context constructor.
*/
class SslContextCallBannedProtocolConfig extends SslContextCallAbstractConfig {
SslContextCallBannedProtocolConfig() { this = "SslContextCallBannedProtocolConfig" }
Expand All @@ -395,7 +399,7 @@ module BoostorgAsio {
}

/**
* a TLS 1.2 protocol value Flows to the first argument of the context constructor
* A TLS 1.2 protocol value that flows to the first argument of a context constructor.
*/
class SslContextCallTls12ProtocolConfig extends SslContextCallAbstractConfig {
SslContextCallTls12ProtocolConfig() { this = "SslContextCallTls12ProtocolConfig" }
Expand All @@ -410,7 +414,7 @@ module BoostorgAsio {
}

/**
* a TLS 1.3 protocol value Flows to the first argument of the context constructor
* A TLS 1.3 protocol value that flows to the first argument of a context constructor.
*/
class SslContextCallTls13ProtocolConfig extends SslContextCallAbstractConfig {
SslContextCallTls13ProtocolConfig() { this = "SslContextCallTls12ProtocolConfig" }
Expand All @@ -425,7 +429,7 @@ module BoostorgAsio {
}

/**
* a generic TLS protocol value Flows to the first argument of the context constructor
* A generic TLS protocol value that flows to the first argument of a context constructor.
*/
class SslContextCallTlsProtocolConfig extends SslContextCallAbstractConfig {
SslContextCallTlsProtocolConfig() { this = "SslContextCallTlsProtocolConfig" }
Expand All @@ -440,7 +444,7 @@ module BoostorgAsio {
}

/**
* a context constructor call flows to a call calling SetOptions()
* A context constructor call that flows to a call to `SetOptions()`.
*/
class SslContextFlowsToSetOptionConfig extends DataFlow::Configuration {
SslContextFlowsToSetOptionConfig() { this = "SslContextFlowsToSetOptionConfig" }
Expand All @@ -464,7 +468,7 @@ module BoostorgAsio {
}

/**
* an option value flows to the 1st parameter of SetOptions()
* An option value that flows to the first parameter of a call to `SetOptions()`.
*/
class SslOptionConfig extends DataFlow::Configuration {
SslOptionConfig() { this = "SslOptionConfig" }
Expand Down