Skip to content
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.

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

Invoking a process with sensitive command line argument - CWE 214 #15

Closed
vigneshwaran-mohanasundaram opened this issue Jul 7, 2023 · 6 comments

Comments

@vigneshwaran-mohanasundaram

Hi Marcono,

I stumbled on your java codeql query packs and i found it very useful. I myself a newbie and trying to learn codeql to develop custom queries for my use-cases. I dont see we have standard query pack which covers the CWE#214 (Invoking a process with sensitive command line argument) . I have attached a simple java program on which am trying to match the value of argument "command" which is passed to runtime.exec() which contains sensitive command line parameter .
Could you please assist me here if you have some time to spare ?

Thanks
Vigneshwaran M

sample.txt

@vigneshwaran-mohanasundaram
Copy link
Author

Forgot to attach my draft queries . But am struggling to match "-p" in my expr :s

/**

  • @name Potential Sensitive command-line argument constructed using string
  • @kind problem
  • @problem.severity warning
  • @id java/command-line-sensitive-argument
  • @tags security
  • @precision high
  • @security-severity 9.1
    */

import java

from MethodAccess mc, Expr arg , Variable var
where
(mc.getMethod().getName().toLowerCase().matches("exec") or
mc.getMethod().getName().toLowerCase().matches("command") or
mc.getMethod().getName().toLowerCase().matches("start"))
and
var.getAnAccess() = mc.getArgument(0) and var.getAnAssignedValue() = arg and arg.toString().matches("%-p\%")
select mc, "Potential Sensitive command-line argument constructed using string: ", arg.toString()

@Marcono1234
Copy link
Owner

Maybe the queries java/concatenated-command-line and java/command-line-injection and the library file CommandLineQuery.qll from the github/codeql repository can be helpful with this.

You would probably want to use CommandInjectionSink from CommandLineQuery.qll to handle detection of calls to exec and similar, then you don't have to determine this yourself.

To track flow from your password variable to the command execution you can use taint tracking, this is more powerful than having to manually check variable assignments and reads. You have to use taint tracking instead of data flow tracking because the string concatenation (... + " -p" + password + ...) creates a new value and is therefore not considered data flow, see the documentation for more details.

For example your query could then look like this:

/**
 * @kind problem
 */

import java
import semmle.code.java.dataflow.TaintTracking
import semmle.code.java.security.CommandLineQuery

from CompileTimeConstantExpr passwordOption, CommandInjectionSink sink
where
  passwordOption.getStringValue().matches("%-p%") and
  TaintTracking::localTaint(DataFlow::exprNode(passwordOption), sink)
select passwordOption, "This password option is used as command line argument $@", sink, "here"

Unless you explicitly want to check for the -p parameter as command line argument, you could instead use SensitiveExpr from SensitiveActions.qll, that covers multiple names of variables or methods which might have a sensitive value, such as password, passwd, ...

/**
 * @kind problem
 */

import java
import semmle.code.java.dataflow.TaintTracking
import semmle.code.java.security.CommandLineQuery
import semmle.code.java.security.SensitiveActions

from SensitiveExpr sensitiveExpr, CommandInjectionSink sink
where TaintTracking::localTaint(DataFlow::exprNode(sensitiveExpr), sink)
select sensitiveExpr, "This sensitive value is used as command line argument $@", sink, "here"

Also a tip regarding the code of your query, avoid checking the result of the toString() predicate, to my knowledge that is just a human readable string representation without any guaranteed format.

I hope that helps. Though I think in the future if you ask on https://github.com/github/codeql (possibly as Discussion) or on Stack Overflow you might get better or a greater variety of answers.

@vigneshwaran-mohanasundaram
Copy link
Author

Hi Marcono,

Thanks much for your quick help and assistance and the explanation . Much grateful for your help and the tips

-Vignesh

@vigneshwaran-mohanasundaram
Copy link
Author

Hi Marcono,

Thanks much for your quick help and assistance and the explanation . Much grateful for your help and the tips

-Vignesh

@Marcono1234
Copy link
Owner

No problem, but if you are using this code for your own query SensitiveCommandlineArgument.ql (especially if you copy the code without any changes), it would have been nice if you added a comment like the following to your code:

// Based on https://github.com/Marcono1234/codeql-java-queries/issues/15#issuecomment-1627404064

This also has the advantage that in case you or someone else wants to get more information about this again in the future, they can revisit my comment above.

No worries though, you can do with your code whatever you want to do regarding licensing; I won't raise any claims there or similar. It is just that writing that answer above also took me some time, so it would have been nice if that was recognized.

@vigneshwaran-mohanasundaram
Copy link
Author

Rearranged my folder structure little bit

https://github.com/vigneshwaran-mohanasundaram/codeql-shared/blob/main/java/ql/SensitiveCommandlineArgument.ql

Done :) . Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants