Merge pull request #33 from trailofbits/mschwager-codeql-query-format

Autoformat existing CodeQL queries and add CI check

Author: mschwager

.git-blame-ignore-revs

@@ -0,0 +1,2 @@
+# Format CodeQL .ql and .qll files using `codeql query format`
+b3b8f8f95bf1a149fdca9a8e16593caa34f4678b

.github/workflows/test.yml

@@ -16,7 +16,5 @@ jobs:
           version: '2.23.8'
           platform: 'linux64'
           checksum: 'e61bc8aa8d86d45acd9d1c36629a12bbfb3365cd07a31666a2ebc91c6a1940b2'
-      - run: |
-          codeql test run --threads=0 ./cpp/test/
-          codeql test run --threads=0 ./go/test/
-          codeql test run --threads=0 ./java/test/
+      - run: make format-check
+      - run: make test

Makefile

@@ -0,0 +1,21 @@
+test:
+	find . -iname "test" -type d -maxdepth 2 -mindepth 2 -print0 | \
+	  xargs -0 codeql test run --threads=0
+
+format:
+	find . \( -iname '*.ql' -o -iname '*.qll' \) -print0 | \
+	  xargs -0 codeql query format --in-place
+
+format-check:
+	find . \( -iname '*.ql' -o -iname '*.qll' \) -print0 | \
+	  xargs -0 codeql query format --check-only
+
+pack-install:
+	find . -iname "qlpack.yml" -exec \
+	  sh -c 'codeql pack install $$(dirname "$$1")' sh {} \;
+
+pack-upgrade:
+	find . -iname "qlpack.yml" -exec \
+	  sh -c 'codeql pack upgrade $$(dirname "$$1")' sh {} \;
+
+.PHONY: test format format-check pack-install pack-upgrade

README.md

@@ -103,16 +103,21 @@ codeql resolve packs | grep trailofbits
 #### Before committing
 
 Run tests:
+
+```sh
+make test
+```
+
+Format queries:
+
 ```sh
-cd codeql-queries
-codeql test run ./cpp/test
-codeql test run ./go/test
-codeql test run ./java/test
+make format
 ```
 
-Update dependencies:
+Install dependencies:
+
 ```sh
-bash ./scripts/install_all.sh
+make install
 ```
 
 Generate query tables and copy-paste it to README.md file

cpp/lib/trailofbits/crypto/common.qll

@@ -1,6 +1,5 @@
 import common.InitUpdateFinalPattern
 import common.SymmetricCipherContext
-
 import common.Csprng
 import common.ErrorCode
 import common.HashFunction

cpp/lib/trailofbits/crypto/common/Csprng.qll

@@ -1,20 +1,20 @@
 import cpp
 
-
 abstract class Csprng extends Function {
   abstract int getAStrongRandomnessSource();
 
   abstract int getRequestedBytes();
 }
 
-
 class CsprngCall extends FunctionCall {
   Csprng csprng;
 
   // Ensures that the call target is an instance of Csprng.
   CsprngCall() { csprng = this.getTarget() }
 
-  Expr getAStrongRandomnessSource() { result = this.getArgument(csprng.getAStrongRandomnessSource()) }
+  Expr getAStrongRandomnessSource() {
+    result = this.getArgument(csprng.getAStrongRandomnessSource())
+  }
 
   Expr getRequestedBytes() { result = this.getArgument(csprng.getRequestedBytes()) }
 }

cpp/lib/trailofbits/crypto/common/CsprngInitializer.qll

@@ -1,32 +1,25 @@
 import cpp
 import StrongRandomnessSink
 
-
 abstract class CsprngInitializer extends Function {
   // Returns the index of the seed argument (if it exists).
   abstract int getAStrongRandomnessSink();
 }
 
 class CsprngInitializerCall extends FunctionCall {
   CsprngInitializer init;
-  
-  CsprngInitializerCall() {
-    this.getTarget() = init
-  }
+
+  CsprngInitializerCall() { this.getTarget() = init }
 
   // Returns the seed argument.
-  Expr getAStrongRandomnessSink() {
-    result = this.getArgument(init.getAStrongRandomnessSink())
-  }
+  Expr getAStrongRandomnessSink() { result = this.getArgument(init.getAStrongRandomnessSink()) }
 }
 
 // A type representing the seed argument of a CSPRNG initializer function.
 class CsprngInitializerSink extends StrongRandomnessSink {
   CsprngInitializerCall init;
 
-  CsprngInitializerSink() {
-    this = init.getAStrongRandomnessSink()
-  }
+  CsprngInitializerSink() { this = init.getAStrongRandomnessSink() }
 
   override string getDescription() {
     result = init.getTarget().getQualifiedName() + " parameter " + this.toString()

cpp/lib/trailofbits/crypto/common/CustomAllocator.qll

@@ -1,45 +1,31 @@
 import cpp
 private import semmle.code.cpp.controlflow.StackVariableReachability
 
-
 /**
  * A custom allocator which returns a pointer to the allocated object.
  */
 abstract class CustomAllocator extends Function {
   CustomDeallocator dealloc;
 
-  CustomDeallocator getDeallocator() {
-    result = dealloc
-  }
+  CustomDeallocator getDeallocator() { result = dealloc }
 
   // Override this and return the index of the pointer argument if the
   // allocated pointer is passed as an argument.
-  int getPointer() {
-    result = -1
-  }
+  int getPointer() { result = -1 }
 }
 
 class CustomAllocatorCall extends FunctionCall {
   CustomAllocator alloc;
 
-  CustomAllocatorCall() {
-    this = alloc.getACallToThisFunction()
-  }
+  CustomAllocatorCall() { this = alloc.getACallToThisFunction() }
 
   Expr getPointer() {
-    if alloc.getPointer() < 0 then
-      result = this
-    else
-      result = this.getArgument(alloc.getPointer())
+    if alloc.getPointer() < 0 then result = this else result = this.getArgument(alloc.getPointer())
   }
 
-  CustomAllocator getAllocator() {
-    result = alloc
-  }
+  CustomAllocator getAllocator() { result = alloc }
 
-  CustomDeallocatorCall getADeallocatorCall() {
-    result.getTarget() = alloc.getDeallocator()
-  }
+  CustomDeallocatorCall getADeallocatorCall() { result.getTarget() = alloc.getDeallocator() }
 }
 
 /**
@@ -52,27 +38,23 @@ abstract class CustomDeallocator extends Function {
 class CustomDeallocatorCall extends FunctionCall {
   CustomDeallocator dealloc;
 
-  CustomDeallocatorCall() {
-    this = dealloc.getACallToThisFunction()
-  }
+  CustomDeallocatorCall() { this = dealloc.getACallToThisFunction() }
 
-  Expr getPointer() {
-    result = this.getArgument(dealloc.getPointer())
-  }
+  Expr getPointer() { result = this.getArgument(dealloc.getPointer()) }
 }
 
 class CustomAllocatorLeak extends StackVariableReachabilityWithReassignment {
   CustomAllocatorCall alloc;
 
-  CustomAllocatorLeak() {
-    this = "CustomAllocatorLeak"
-  }
+  CustomAllocatorLeak() { this = "CustomAllocatorLeak" }
 
   override predicate isSourceActual(ControlFlowNode node, StackVariable var) {
     // A source is an allocation of the variable.
-    node = alloc.getPointer() and (
+    node = alloc.getPointer() and
+    (
       // When alloc returns allocated object.
-      alloc.getPointer() = var.getAnAssignedValue() or
+      alloc.getPointer() = var.getAnAssignedValue()
+      or
       // When alloc takes a pointer as an argument the variable can be either a
       // pointer or an object v, in which case &v is passed to the function and
       // we need to invoke getAChild() to get a variable access.
@@ -87,24 +69,22 @@ class CustomAllocatorLeak extends StackVariableReachabilityWithReassignment {
 
   override predicate isSinkActual(ControlFlowNode node, StackVariable var) {
     // A sink is a return statement not returning the allocated variable.
-    var.getFunction() = node.(ReturnStmt).getEnclosingFunction()
-    and not node.(ReturnStmt).getExpr() = var.getAnAccess()
+    var.getFunction() = node.(ReturnStmt).getEnclosingFunction() and
+    not node.(ReturnStmt).getExpr() = var.getAnAccess()
   }
 }
 
 class CustomAllocatorUseAfterFree extends StackVariableReachabilityWithReassignment {
-  CustomAllocatorUseAfterFree() {
-    this = "CustomAllocatorUseAfterFree"
-  }
+  CustomAllocatorUseAfterFree() { this = "CustomAllocatorUseAfterFree" }
 
   override predicate isSourceActual(ControlFlowNode node, StackVariable var) {
     node.(CustomDeallocatorCall).getPointer() = var.getAnAccess()
   }
 
   override predicate isSinkActual(ControlFlowNode node, StackVariable var) {
     // A use is an access which is not a reassignment.
-    node = var.getAnAccess() and not
-    this.isAnAssignment(node, var)
+    node = var.getAnAccess() and
+    not this.isAnAssignment(node, var)
   }
 
   override predicate isBarrier(ControlFlowNode node, StackVariable var) {

cpp/lib/trailofbits/crypto/common/ErrorCode.qll

@@ -5,7 +5,6 @@ import cpp
  */
 abstract class ReturnsErrorCode extends Function { }
 
-
 /**
  * A type modelling the return value from a function that returns an error or status code.
  */

cpp/lib/trailofbits/crypto/common/HashFunction.qll

@@ -1,6 +1,5 @@
 import cpp
 
-
 // A type modelling a (taint propagating) hash function.
 abstract class HashFunction extends TaintFunction {
   // Returns the index of the input data argument.
@@ -16,15 +15,15 @@ abstract class HashFunction extends TaintFunction {
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
     input.isParameterDeref(this.getInput()) and
     (
-      output.isParameterDeref(this.getAnOutput()) or
-      (this.returnsOutput() = true and output.isReturnValueDeref())
+      output.isParameterDeref(this.getAnOutput())
+      or
+      this.returnsOutput() = true and output.isReturnValueDeref()
     )
   }
 }
 
 // A type modelling a (taint propagating) hash context.
-class HashContext extends Expr {
-}
+class HashContext extends Expr { }
 
 // A type modelling a hash context initializer.
 abstract class HashContextInitializer extends Function {
@@ -40,14 +39,13 @@ class HashContextInitializerCall extends FunctionCall {
 
   // Returns the hash context argument.
   HashContext getContext() {
-    (init.returnsContext() = true and result = this) or
-    (result = this.getArgument(init.getAContext()))
+    init.returnsContext() = true and result = this
+    or
+    result = this.getArgument(init.getAContext())
   }
 
   // Returns the corresponding initializer function.
-  HashContextInitializer getInitializer() {
-    result = init
-  }
+  HashContextInitializer getInitializer() { result = init }
 }
 
 // A type modelling a hash context update function.
@@ -70,40 +68,35 @@ class HashContextUpdaterCall extends FunctionCall {
   HashContextUpdater update;
 
   // Returns the hash context argument.
-  HashContext getContext() {
-    result = this.getArgument(update.getContext())
-  }
+  HashContext getContext() { result = this.getArgument(update.getContext()) }
 
   // Returns the corresponding update function.
-  HashContextInitializer getUpdater() {
-    result = update
-  }
-  
+  HashContextInitializer getUpdater() { result = update }
+
   // Returns an input data argument.
-  Expr getAnInput() {
-    result = this.getArgument(update.getAnInput())
-  }
+  Expr getAnInput() { result = this.getArgument(update.getAnInput()) }
 }
 
 // A type modelling a hash context finalizer function.
 abstract class HashContextFinalizer extends TaintFunction {
   // Returns the index of the context argument.
   abstract int getContext();
-  
+
   // Returns the output digest argument (if it exists).
   abstract int getAnOutput();
 
   // True iff the finalizer function returns a pointer or reference to the
   // output digest.
   abstract boolean returnsOutput();
-  
+
   // Since C++ does not have AdditionalTaintFlow we use TaintFunction to model
   // taint from input to output through the hash context.
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
     input.isParameterDeref(this.getContext()) and
     (
-      output.isParameterDeref(this.getAnOutput()) or
-      (this.returnsOutput() = true and output.isReturnValueDeref())
+      output.isParameterDeref(this.getAnOutput())
+      or
+      this.returnsOutput() = true and output.isReturnValueDeref()
     )
   }
 }
@@ -112,19 +105,15 @@ class HashContextFinalizerCall extends FunctionCall {
   HashContextFinalizer final;
 
   // Returns the hash context argument.
-  HashContext getContext() {
-    result = this.getArgument(final.getContext())
-  }
+  HashContext getContext() { result = this.getArgument(final.getContext()) }
 
   // Returns the corresponding finalizer function.
-  HashContextInitializer getFinalizer() {
-    result = final
-  }
+  HashContextInitializer getFinalizer() { result = final }
 
   // Returns an expression representing the output digest.
   Expr getAnOutput() {
-    (final.returnsOutput() = true and result = this) or
-    (result = this.getArgument(final.getAnOutput()))
+    final.returnsOutput() = true and result = this
+    or
+    result = this.getArgument(final.getAnOutput())
   }
 }
-