Skip to content

Query ideas

Marcono1234 edited this page Jun 16, 2021 · 4 revisions

Ideas for new queries.

Field type could be primitive

  • Field has boxed type
  • Field is private or package-private; or has higher visibility but is not publicly visible
  • Field is only ever assigned non-null values (use CodeQL's NullGuards.clearlyNotNullExpr())

Could ignore cases where field value is passed to method accepting only boxed type (respectively supertype), or where boxed type (or supertype) is returned by method; maybe field type was chosen for performance reasons to avoid boxing primitive type. But with true value-based types this performance penalty for wrapping primitive values might be negligible.

Similar to CodeQL's 'java/non-null-boxed-variable' (but that only covers local variables)

Array component type could be primitive

  • Array component type is boxed
  • Array is used only internally (as local variable or field)
  • Array elements are only ever assigned non-null values (use CodeQL's NullGuards.clearlyNotNullExpr())

Not handling Process streams correctly

TODO: Verify that this is actually a problem

  • Starts a new Process but does not redirect input and output stream
    Could cause deadlock when parent Java process waits for child process to finish and child process waits for parent process to provide input / consume output
    • Output stream is neither redirected using ProcessBuilder.redirectOutput nor obtained from created Process
    • Error stream is neither redirected using redirectError or redirectErrorStream​, nor obtained from created Process
    • Input stream is neither redirected using redirectInput nor obtained from created Process
    • Also consider new Process Reader and Writer methods, see JDK-8191441
  • Consider ProcessBuilder and Runtime methods for starting process

See also https://github.com/github/codeql/pull/5597/files for how this could be modelled with dataflow and the predicate isAdditionalFlowStep.

Worker Thread not started as daemon

Thread would then prevent JVM exit.

  • Thread which has lambda, method reference or Runnable implementation which looks like infinite loop (at first check for any loop other than EnhancedForLoop)
  • setDaemon(true) is not called on thread. This would not be a problem if the parent thread is a daemon, but relying on this is error-prone.

ExecutorService not being shut down

Unless a custom ThreadFactory is used (which might create daemon threads), not shutting down an executor service will either prevent JVM exit or unnecessary delay it (for Executors.newCachedThreadPool() would have to wait until they are terminated after idle time).

  • Only detect executor services created with Executors methods
  • Ignore if custom ThreadFactory is used