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
12 changes: 4 additions & 8 deletions java/ql/lib/experimental/quantum/JCA.qll
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ module JCAModel {
}

/**
* Data-flow configuration modelling flow from a cipher string literal to a cipher algorithm consumer.
* Data-flow configuration modeling flow from a cipher string literal to a cipher algorithm consumer.
*/
private module CipherAlgorithmStringToCipherConsumerConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node src) { src.asExpr() instanceof CipherStringLiteral }
Expand Down Expand Up @@ -404,9 +404,7 @@ module JCAModel {
* For example, in `Cipher.getInstance(algorithm)`, this class represents `algorithm`.
*/
class CipherGetInstanceAlgorithmArg extends CipherAlgorithmValueConsumer instanceof Expr {
CipherGetInstanceCall call;

CipherGetInstanceAlgorithmArg() { this = call.getAlgorithmArg() }
CipherGetInstanceAlgorithmArg() { this = any(CipherGetInstanceCall call).getAlgorithmArg() }

override Crypto::ConsumerInputDataFlowNode getInputNode() { result.asExpr() = this }

Expand Down Expand Up @@ -1333,7 +1331,7 @@ module JCAModel {
}

/**
* Data-flow configuration modelling flow from a key agreement string literal to a key agreement algorithm consumer.
* Data-flow configuration modeling flow from a key agreement string literal to a key agreement algorithm consumer.
*/
private module KeyAgreementAlgorithmStringToConsumerConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node src) { src.asExpr() instanceof KeyAgreementStringLiteral }
Expand Down Expand Up @@ -1539,11 +1537,9 @@ module JCAModel {
}

class MacOperationCall extends Crypto::MacOperationInstance instanceof MethodCall {
Expr output;

MacOperationCall() {
super.getMethod().getDeclaringType().hasQualifiedName("javax.crypto", "Mac") and
(
exists(Expr output |
super.getMethod().hasStringSignature(["doFinal()", "doFinal(byte[])"]) and this = output
or
super.getMethod().hasStringSignature("doFinal(byte[], int)") and
Expand Down
2 changes: 1 addition & 1 deletion java/ql/lib/experimental/quantum/Language.qll
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ private class ConstantDataSource extends Crypto::GenericConstantSourceInstance i
}

/**
* An instance of random number generation, modelled as the expression
* An instance of random number generation, modeled as the expression
* tied to an output node (i.e., the result of the source of randomness)
*/
abstract class RandomnessInstance extends Crypto::RandomNumberGenerationInstance {
Expand Down
4 changes: 2 additions & 2 deletions java/ql/lib/printAst.ql
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ external string selectedSourceFile();

class PrintAstConfigurationOverride extends PrintAstConfiguration {
/**
* Holds if the location matches the selected file in the VS Code extension and
* the element is `fromSource`.
* Holds if the location `l` matches the selected file in the VS Code extension and
* the element `e` is `fromSource`.
*/
override predicate shouldPrint(Element e, Location l) {
super.shouldPrint(e, l) and
Expand Down
3 changes: 2 additions & 1 deletion java/ql/lib/semmle/code/java/Concurrency.qll
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ predicate locallySynchronizedOnThis(Expr e, RefType thisType) {
}

/**
* Holds if `e` is synchronized by a `synchronized` modifier on the enclosing (static) method.
* Holds if `e` is synchronized by a `synchronized` modifier on the enclosing (static) method
* declared in the type `classType`.
*/
predicate locallySynchronizedOnClass(Expr e, RefType classType) {
exists(SynchronizedCallable c | c = e.getEnclosingCallable() |
Expand Down
10 changes: 5 additions & 5 deletions java/ql/lib/semmle/code/java/Conversions.qll
Original file line number Diff line number Diff line change
Expand Up @@ -100,12 +100,12 @@ class InvocationConversionContext extends ConversionSite {
* See the Java Language Specification, Section 5.4.
*/
class StringConversionContext extends ConversionSite {
AddExpr a;

StringConversionContext() {
a.getAnOperand() = this and
not this.getType() instanceof TypeString and
a.getAnOperand().getType() instanceof TypeString
exists(AddExpr a |
a.getAnOperand() = this and
not this.getType() instanceof TypeString and
a.getAnOperand().getType() instanceof TypeString
)
}

override Type getConversionTarget() { result instanceof TypeString }
Expand Down
8 changes: 4 additions & 4 deletions java/ql/lib/semmle/code/java/Statement.qll
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ class TryStmt extends Stmt, @trystmt {
CatchClause getACatchClause() { result.getParent() = this }

/**
* Gets the `catch` clause at the specified (zero-based) position
* Gets the `catch` clause at the specified (zero-based) position `index`
* in this `try` statement.
*/
CatchClause getCatchClause(int index) {
Expand All @@ -305,7 +305,7 @@ class TryStmt extends Stmt, @trystmt {
/** Gets a resource variable declaration, if any. */
LocalVariableDeclStmt getAResourceDecl() { result.getParent() = this and result.getIndex() <= -3 }

/** Gets the resource variable declaration at the specified position in this `try` statement. */
/** Gets the resource variable declaration at the specified position `index` in this `try` statement. */
LocalVariableDeclStmt getResourceDecl(int index) {
result = this.getAResourceDecl() and
index = -3 - result.getIndex()
Expand All @@ -314,7 +314,7 @@ class TryStmt extends Stmt, @trystmt {
/** Gets a resource expression, if any. */
VarAccess getAResourceExpr() { result.getParent() = this and result.getIndex() <= -3 }

/** Gets the resource expression at the specified position in this `try` statement. */
/** Gets the resource expression at the specified position `index` in this `try` statement. */
VarAccess getResourceExpr(int index) {
result = this.getAResourceExpr() and
index = -3 - result.getIndex()
Expand All @@ -323,7 +323,7 @@ class TryStmt extends Stmt, @trystmt {
/** Gets a resource in this `try` statement, if any. */
ExprParent getAResource() { result = this.getAResourceDecl() or result = this.getAResourceExpr() }

/** Gets the resource at the specified position in this `try` statement. */
/** Gets the resource at the specified position `index` in this `try` statement. */
ExprParent getResource(int index) {
result = this.getResourceDecl(index) or result = this.getResourceExpr(index)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,8 @@ private predicate enhancedForStmtStep(Node node1, Node node2, Type containerType
}

/**
* Holds if the step from `node1` to `node2` reads a value from an array.
* Holds if the step from `node1` to `node2` reads a value from an array, where
* the elements are of type `elemType`.
* This covers ordinary array reads as well as array iteration through enhanced
* `for` statements.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -263,8 +263,8 @@ class Content extends TContent {

/**
* 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`.
* For more information, see
* [Locations](https://codeql.github.com/docs/writing-codeql-queries/providing-locations-in-codeql-queries/).
*/
Expand Down Expand Up @@ -362,8 +362,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`.
* 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 @@ -204,7 +204,7 @@ private module Impl {
/** Gets the character value of expression `e`. */
string getCharValue(Expr e) { result = e.(CharacterLiteral).getValue() }

/** Gets the constant `float` value of non-`ConstantIntegerExpr` expressions. */
/** Gets the constant `float` value of non-`ConstantIntegerExpr` expression `e`. */
float getNonIntegerValue(Expr e) {
result = e.(LongLiteral).getValue().toFloat() or
result = e.(FloatLiteral).getValue().toFloat() or
Expand Down Expand Up @@ -256,12 +256,12 @@ private module Impl {
exists(EnhancedForStmt for | def = for.getVariable())
}

/** Returns the operand of the operation if `def` is a decrement. */
/** Returns the operand of the operation if `e` is a decrement. */
Expr getDecrementOperand(Element e) {
result = e.(PostDecExpr).getExpr() or result = e.(PreDecExpr).getExpr()
}

/** Returns the operand of the operation if `def` is an increment. */
/** Returns the operand of the operation if `e` is an increment. */
Expr getIncrementOperand(Element e) {
result = e.(PostIncExpr).getExpr() or result = e.(PreIncExpr).getExpr()
}
Expand Down
35 changes: 16 additions & 19 deletions java/ql/lib/semmle/code/java/frameworks/Mockito.qll
Original file line number Diff line number Diff line change
Expand Up @@ -223,10 +223,10 @@ class MockitoInjectedField extends MockitoAnnotatedField {
// If there is no initializer for this field, and there is a most mockable constructor,
// then we are doing a parameterized injection of mocks into a most mockable constructor.
result = mockInjectedClass.getAMostMockableConstructor()
else
if this.usingPropertyInjection()
then
// We will call the no-arg constructor if the field wasn't initialized.
else (
this.usingPropertyInjection() and
// We will call the no-arg constructor if the field wasn't initialized.
(
not exists(this.getInitializer()) and
result = mockInjectedClass.getNoArgsConstructor()
or
Expand All @@ -241,9 +241,8 @@ class MockitoInjectedField extends MockitoAnnotatedField {
// once, but we instead assume that there are sufficient mocks to go around.
mockedField.getType().(RefType).getAnAncestor() = result.getParameterType(0)
)
else
// There's no instance, and no no-arg constructor we can call, so injection fails.
none()
)
)
)
}

Expand All @@ -253,18 +252,16 @@ class MockitoInjectedField extends MockitoAnnotatedField {
* Field injection only occurs if property injection and not constructor injection is used.
*/
Field getASetField() {
if this.usingPropertyInjection()
then
result = this.getMockInjectedClass().getASetField() and
exists(MockitoMockedField mockedField |
mockedField.getDeclaringType() = this.getDeclaringType() and
mockedField.isValid()
|
// We make a simplifying assumption here - in theory, each mock can only be injected
// once, but we instead assume that there are sufficient mocks to go around.
mockedField.getType().(RefType).getAnAncestor() = result.getType()
)
else none()
this.usingPropertyInjection() and
result = this.getMockInjectedClass().getASetField() and
exists(MockitoMockedField mockedField |
mockedField.getDeclaringType() = this.getDeclaringType() and
mockedField.isValid()
|
// We make a simplifying assumption here - in theory, each mock can only be injected
// once, but we instead assume that there are sufficient mocks to go around.
mockedField.getType().(RefType).getAnAncestor() = result.getType()
)
}
}

Expand Down
4 changes: 2 additions & 2 deletions java/ql/lib/semmle/code/java/frameworks/javaee/ejb/EJB.qll
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,14 @@ class SessionEjb extends EJB {
// Either the EJB does not declare any business interfaces explicitly
// and implements a single interface candidate,
// which is then considered to be the business interface...
count(this.getAnExplicitBusinessInterface()) = 0 and
not exists(this.getAnExplicitBusinessInterface()) and
count(this.getAnImplementedBusinessInterfaceCandidate()) = 1 and
result = this.getAnImplementedBusinessInterfaceCandidate()
or
// ...or each business interface needs to be declared explicitly.
(
count(this.getAnImplementedBusinessInterfaceCandidate()) != 1 or
count(this.getAnExplicitBusinessInterface()) != 0
exists(this.getAnExplicitBusinessInterface())
) and
result = this.getAnExplicitBusinessInterface()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ private predicate isVarargs(Argument arg, DataFlow::ImplicitVarargsArray varargs
arg.isVararg() and arg.getCall() = varargs.getCall()
}

/** Holds if `store` closes `file`. */
/** Holds if `closeCall` closes `file`. */
private predicate closesFile(DataFlow::Node file, Call closeCall) {
closeCall.getCallee() instanceof CloseFileMethod and
if closeCall.getCallee().isStatic()
Expand Down
2 changes: 1 addition & 1 deletion java/ql/lib/semmle/code/java/security/FileWritable.qll
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ private VarAccess getFileForPathConversion(Expr pathExpr) {
}

/**
* Holds if `fileAccess` is used in the `setWorldWritableExpr` to set the file to be world writable.
* Holds if `fileAccess` is used in the `setWorldWritable` to set the file to be world writable.
*/
private predicate fileSetWorldWritable(VarAccess fileAccess, Expr setWorldWritable) {
// Calls to `File.setWritable(.., false)`.
Expand Down
3 changes: 2 additions & 1 deletion java/ql/lib/semmle/code/java/security/TempDirUtils.qll
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ class MethodFileCreateTempFile extends Method {
}

/**
* Holds if `expDest` is some constructor call `new java.io.File(expSource)`, where the specific `File` constructor being used has `paramCount` parameters.
* Holds if `expSource` is an argument to a constructor call `exprDest` (constructor from `java.io.File`), where
* the specific `File` constructor being used has `paramCount` parameters.
*/
predicate isFileConstructorArgument(Expr expSource, Expr exprDest, int paramCount) {
exists(ConstructorCall construtorCall |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -552,7 +552,7 @@ private DataFlow::Node getASafelyConfiguredParser() {
}

/**
* Holds if `parseMethodQualifierExpr` is a `jodd.json.JsonParser` instance that is configured unsafely
* Holds if `parserExpr` is a `jodd.json.JsonParser` instance that is configured unsafely
* and which never appears to be configured safely.
*/
private predicate joddJsonParserConfiguredUnsafely(Expr parserExpr) {
Expand Down
2 changes: 1 addition & 1 deletion java/ql/src/Likely Bugs/Comparison/HashedButNoHash.ql
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
import java
import Equality

/** A class that defines an `equals` method but no `hashCode` method. */
/** Holds if `c` defines an `equals` method but no `hashCode` method. */
predicate eqNoHash(Class c) {
exists(Method m | m = c.getAMethod() |
m instanceof EqualsMethod and
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ private predicate synchronizedThisAccess(MethodCall ma, Type thisType) {

/**
* Auxiliary predicate for `unsynchronizedVarAccess`. Holds if
* there is an enclosing `synchronized` statement on the variable.
* there is an enclosing `synchronized` statement on the variable access `x`.
*/
predicate synchronizedVarAccess(VarAccess x) {
exists(SynchronizedStmt s, VarAccess y |
Expand Down
4 changes: 2 additions & 2 deletions java/ql/src/Likely Bugs/Termination/SpinOnField.ql
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ class Empty extends Stmt {
class EmptyLoop extends Stmt {
EmptyLoop() {
exists(ForStmt stmt | stmt = this |
count(stmt.getAnInit()) = 0 and
count(stmt.getAnUpdate()) = 0 and
not exists(stmt.getAnInit()) and
not exists(stmt.getAnUpdate()) and
stmt.getStmt() instanceof Empty
)
or
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ private predicate valueOccurrenceCount(string value, int n, string context) {
n > 20
}

private predicate occurenceCount(Literal lit, string value, int n, string context) {
private predicate occurrenceCount(Literal lit, string value, int n, string context) {
valueOccurrenceCount(value, n, context) and
value = lit.getValue() and
nonTrivialValue(_, lit, context)
Expand All @@ -119,7 +119,7 @@ private predicate check(Literal lit, string value, int n, string context, Compil
// Check that the literal is nontrivial
not trivial(lit) and
// Check that it is repeated a number of times
occurenceCount(lit, value, n, context) and
occurrenceCount(lit, value, n, context) and
n > 20 and
f = lit.getCompilationUnit()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ private predicate overloadedMethodsMostSpecific(Method n, Method m) {
private predicate whitelist(string name) { name = "visit" }

/**
* Method `m` has name `name`, number of parameters `numParams`
* Method `m` has name `name`, number of parameters `numParam`
* and is declared in `t` or inherited from a supertype of `t`.
*/
pragma[nomagic]
Expand Down
16 changes: 7 additions & 9 deletions java/ql/src/experimental/quantum/Analysis/ArtifactReuse.qll
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,13 @@ import experimental.quantum.Language
*/
private module WrapperConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) {
exists(Call c |
c = source.asExpr()
// not handling references yet, I think we want to flat say references are only ok
// if I know the source, otherwise, it has to be through an additional flow step, which
// we filter as a source, i.e., references are only allowed as sources only,
// no inferrece? Not sure if that would work
//or
// source.(DataFlow::PostUpdateNode).getPreUpdateNode().asExpr() = c.getAnArgument()
) and
source.asExpr() instanceof Call and
// not handling references yet, I think we want to flat say references are only ok
// if I know the source, otherwise, it has to be through an additional flow step, which
// we filter as a source, i.e., references are only allowed as sources only,
// no inferrece? Not sure if that would work
//or
// source.(DataFlow::PostUpdateNode).getPreUpdateNode().asExpr() = c.getAnArgument()
// Filter out sources that are known additional flow steps, as these are likely not the
// kind of wrapper source we are looking for.
not exists(AdditionalFlowInputStep s | s.getOutput() = source)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
*/

//THIS QUERY IS A REPLICA OF: https://github.com/github/codeql/blob/main/java/ql/src/Security/CWE/CWE-327/BrokenCryptoAlgorithm.ql
//but uses the **NEW MODELLING**
//but uses the **NEW MODELING**
import experimental.quantum.Language

/**
Expand Down
4 changes: 2 additions & 2 deletions java/ql/src/utils/flowtestcasegenerator/FlowTestCase.qll
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ class TestCase extends TTestCase {

/**
* Returns a call to `source()` wrapped in `newWith` methods as needed according to `input`.
* For example, if the input specification is `ArrayElement of MapValue of Argument[0]`, this
* For example, if the `stack` is `Argument[0].MapValue.ArrayElement`, this
* will return `newWithMapValue(newWithArrayElement(source()))`.
*/
string getInput(SummaryComponentStack stack) {
Expand All @@ -269,7 +269,7 @@ class TestCase extends TTestCase {

/**
* Returns `out` wrapped in `get` methods as needed according to `output`.
* For example, if the output specification is `ArrayElement of MapValue of Argument[0]`, this
* For example, if the `componentStack` is `Argument[0].MapValue.ArrayElement`, this
* will return `getArrayElement(getMapValue(out))`.
*/
string getOutput(SummaryComponentStack componentStack) {
Expand Down