codeql icon indicating copy to clipboard operation
codeql copied to clipboard

Path-problem result pattern

Open KylerKatz opened this issue 1 year ago • 5 comments
trafficstars

Description of the issue

Hello, I made a query that seems to be working just fine using the VS Code extension. However, when I try to query my database using the CLI with this query I am getting this error.

A fatal error occurred: Could not process query metadata for C:\Users\Kyler\Documents\Work\cwe200\codeql\codeql-custom-queries-java\CWE-535\ExposureofInformationThroughShellErrorMessage copy.ql. Error was: Expected result pattern(s) are not present for path-problem query: Expected at least two result patterns. These should include at least an 'edges' result set (see https://codeql.github.com/docs/writing-codeql-queries/creating-path-queries/). [INVALID_RESULT_PATTERNS]

I have read through that link and I have also checked out this issue here and did some looking into other path problem examples. However, I am still getting this error after trying all of this. I believe that it is something simple that I am missing right in front of me. Any help would be appreciated. Thank you in advance.

My code

import java
import semmle.code.java.dataflow.TaintTracking
import DataFlow::PathGraph
/**
 * @name Exposure of information through shell error message
 * @description Exposing error messages from shell commands can lead to information disclosure.
 * @kind path-problem
 * @problem.severity warning
 * @id java/shell-error-exposure
 * @tags security
 *       external/cwe/cwe-535
 */
class ShellErrorExposureConfig extends DataFlow::Configuration {
  ShellErrorExposureConfig() { this = "ShellErrorExposureConfig" }

  override predicate isSource(DataFlow::Node source) {
    exists(MethodAccess ma |
      // Captures getting the error stream from a process
      ma.getMethod().hasName("getErrorStream") and
      // Ensure the Process is the result of exec or start, indicating command execution
      (ma.getQualifier().(VarAccess).getVariable().getAnAssignedValue() instanceof MethodAccess and
       ma.getQualifier().(VarAccess).getVariable().getAnAssignedValue().(MethodAccess).getMethod().hasName("exec") or
       ma.getQualifier().(VarAccess).getVariable().getAnAssignedValue().(MethodAccess).getMethod().hasName("start")) and
      source.asExpr() = ma
    )
    or
    exists(MethodAccess exec |
      // Direct use of user input in command execution
      exec.getMethod().hasName("exec") and
      exec.getMethod().getDeclaringType().hasQualifiedName("java.lang", "Runtime") and
      source.asExpr() = exec
    )
  }

  override predicate isSink(DataFlow::Node sink) {
    // System.out.println or similar direct output methods as sinks
    (exists(MethodAccess println |
      println.getMethod().hasName("println") and
      println.getQualifier().(VarAccess).getVariable().getType() instanceof RefType and
      ((RefType)println.getQualifier().(VarAccess).getVariable().getType()).hasQualifiedName("java.io", "PrintStream") and
      sink.asExpr() = println.getAnArgument())
      and 
    not exists(MethodAccess sanitizeErrorOutput |
      sanitizeErrorOutput.getMethod().hasName("sanitizeErrorOutput") and
      sink.asExpr() = sanitizeErrorOutput.getAnArgument()
    ))
    or
    exists(MethodAccess getMessage |
      getMessage.getMethod().hasName(["getMessage", "getStackTrace", "getStackTraceAsString", "printStackTrace"]) and
      getMessage.getMethod().getDeclaringType().getASupertype*().hasQualifiedName("java.lang", "Throwable") and
      sink.asExpr() = getMessage
    )
  }
}

from ShellErrorExposureConfig config, DataFlow::PathNode source, DataFlow::PathNode sink
where config.hasFlowPath(source, sink)
select sink, source, sink, "Potential CWE-535: Exposure of information through shell error message"

I tried updating my select statement to match the example in this code codeql\ql\java\ql\src\Security\CWE\CWE-532\SensitiveInfoLog.ql

To be

select sink.getNode(), source, sink, "This $@ is written to a log file.", source.getNode(),
  "potentially sensitive information"

KylerKatz avatar Feb 27 '24 23:02 KylerKatz

The phrase These should include at least an 'edges' result set suggests that the edges result set is missing somehow. However, you import DataFlow::PathGraph already so it looks fine. The rest of your query looks fine at a glance too.

Have you tried re-running the query (--rerun flag)? The CLI may cache the results of a previous run, so if you had a bug in the query before you need to make sure to rerun before a fix takes effect.

aibaars avatar Feb 28 '24 16:02 aibaars

Hello,

Thank you for your response. I have a few more questions since I also seem to be getting strange behavior with some of my other queries as well. For some more context, I am using the vscode-codeql-starter-workspace

The first is related to the --rerun flag that you meantioned. I tried adding this to my command however I still see [1/1] Found in cache So I don't believe that I am using it correctly. I tried looking for this command in the documentation however, I am not able to find it. This is how I am currently using it in my command

codeql database analyze codeqldb --format=sarifv2.1.0 --output=out.sarif --rerun=true codeql-custom-queries-java/CWE-535 I wasn't sure if it is just --rerun or if I needed to add the bool as well.

In addtion to this query I also to cover CWE-208. Thankfully there is already CWE-208 queries included for java. Found in codeql\ql\java\ql\src\experimental\Security\CWE\CWE-208\TimingAttackAgainstSignature.ql. Since I would like all the queries I am using to be together I copied it to the codeql-custom-queries-java directory.

This is what that query looks like.

/**
 * @name Timing attack against signature validation
 * @description When checking a signature over a message, a constant-time algorithm should be used.
 *              Otherwise, an attacker may be able to forge a valid signature for an arbitrary message
 *              by running a timing attack if they can send to the validation procedure
 *              both the message and the signature.
 *              A successful attack can result in authentication bypass.
 * @kind path-problem
 * @problem.severity error
 * @precision high
 * @id java/timing-attack-against-signature
 * @tags security
 *       experimental
 *       external/cwe/cwe-208
 */

import java
import NonConstantTimeCheckOnSignatureQuery
import NonConstantTimeCryptoComparisonFlow::PathGraph

from
  NonConstantTimeCryptoComparisonFlow::PathNode source,
  NonConstantTimeCryptoComparisonFlow::PathNode sink
where
  NonConstantTimeCryptoComparisonFlow::flowPath(source, sink) and
  (
    source.getNode().(CryptoOperationSource).includesUserInput() and
    sink.getNode().(NonConstantTimeComparisonSink).includesUserInput()
  )
select sink.getNode(), source, sink, "Timing attack against $@ validation.", source,
  source.getNode().(CryptoOperationSource).getCall().getResultType()

When running it from this directory. I get a ERROR: Could not resolve module NonConstantTimeCryptoComparisonFlow and ERROR: Could not resolve type CryptoOperationSource and a few others. I thought that's strange since it's included in the library itself. Just to see I changed my command to point to codeql\ql\java\ql\src\experimental\Security\CWE\CWE-208\TimingAttackAgainstSignature.ql instead and it worked just fine.

I mention this, because I am unsure how many of my problems are from the CLI and how many are from the environment? I have reset the environment multiple times via downloading the zip and using git clone --recurse-submodules https://github.com/github/vscode-codeql-starter.git codeql

Sorry for the long response, I just wanted to give as much context as possible since I have been stuck on this problem for a bit and it seems like I am following everything correctly.

Thank you for any help.

KylerKatz avatar Feb 28 '24 19:02 KylerKatz

I think import NonConstantTimeCheckOnSignatureQuery is a local import which only works if the query file is in the same folder as the imported .qll file.

aibaars avatar Feb 29 '24 09:02 aibaars

Thank you, that fixed that import error. I am still getting these types of errors for a few of my queries. I have tried re-running them, and even tried a different computer. Is there anything else that might be causing this? It's for almost all of my path-problems. I appreciate your help.

Error was: Expected result pattern(s) are not present for path-problem query: Expected at least two result patterns. These should include at least an 'edges' result set (see https://codeql.github.com/docs/writing-codeql-queries/creating-path-queries/). [INVALID_RESULT_PATTERNS]

KylerKatz avatar Feb 29 '24 23:02 KylerKatz

Another reason for that error to occur is if you neither import a modules that defines query predicate edges nor define the query predicate yourself. In the example code above you have import NonConstantTimeCryptoComparisonFlow::PathGraph which should be fine. Perhaps you forgot that import statement in some of your other queries?

aibaars avatar Mar 01 '24 08:03 aibaars

Hello,

Thank you for your help @aibaars

From reading the docs suggested in the error message. It seems like I should just be able to do import MyFlow::PathGraph to use the predefined edges predicate.

This predicate defines the edge relations of the graph you are computing, and it is used to compute the paths related to each result that your query generates. You can import a predefined edges predicate from a path graph module in one of the standard data flow libraries. In addition to the path graph module, the data flow libraries contain the other classes, predicates, and modules that are commonly used in data flow analysis.

import MyFlow::PathGraph This statement imports the PathGraph module from the data flow library (DataFlow.qll), in which edges is defined.

However, the error presists. So, I have been trying to define my own edges predicate, but I am having trouble understanding exactly how to do so.

This template is given, but I am not really sure what logic to include in the body.

query predicate edges(PathNode a, PathNode b) {
  /* Logical conditions which hold if `(a,b)` is an edge in the data flow graph */
}

I would like to make it as simple as possible since I am just using a simple source and sink dataflow right now before extending my query further.

Also, I have reworked the structure of my query to match the suggestions given in that link as well just to make sure that wasn't an issue also.

KylerKatz avatar Mar 09 '24 01:03 KylerKatz