Which is the best practice for customizing (or reusing source, sink) of codeql core libraries?

[vovikhangcdv]

Hi there,

As I know, we need to create a Configuration for a brand new path query and then define Source, Sink. But I want to customize existed queries such as Command Injection (js) by adding more subclass of Source and Sink.

For example:

I want to mix the source of Command Injection and Shell Command Injection From Environment with extra sinks.
So I want something look like:

/**
 * @kind path-problem
 */
 import javascript
 import DataFlow
 import DataFlow::PathGraph
 import semmle.javascript.security.dataflow.CommandInjectionCustomizations::CommandInjection
 import semmle.javascript.security.dataflow.ShellCommandInjectionFromEnvironmentCustomizations::ShellCommandInjectionFromEnvironment
 
 class MyConfig extends TaintTracking::Configuration {
   MyConfig() { this = "MyConfig" }
   override predicate isSource(Node node) { node instanceof Source }
   override predicate isSink(Node node) { node instanceof Sink }
 }
 
 from MyConfig cfg, PathNode source, PathNode sink
 where cfg.hasFlowPath(source, sink)
 select sink.getNode(), source, sink, "taint from $@.", source.getNode(), "here"

Unfortunately, this will yell a conflict error. I usually go to the library to edit the core library or bring/fork/clone all sources of the core library into a single new query. But I think it is not an appreciated approach.

I wonder, which is the best practice for customizing (or reusing source, sink) of codeql core libraries?

[MathiasVP]

Hi @vovikhangcdv,

If you replace your two imports:

 import semmle.javascript.security.dataflow.CommandInjectionCustomizations::CommandInjection
 import semmle.javascript.security.dataflow.ShellCommandInjectionFromEnvironmentCustomizations::ShellCommandInjectionFromEnvironment

with

import semmle.javascript.security.dataflow.CommandInjectionCustomizations
import semmle.javascript.security.dataflow.ShellCommandInjectionFromEnvironmentCustomizations

(i.e., without the :: part)

then you can rewrite your configuration as:

class MyConfig extends TaintTracking::Configuration {
  MyConfig() { this = "MyConfig" }

  override predicate isSource(Node node) {
    node instanceof CommandInjection::Source or
    node instanceof ShellCommandInjectionFromEnvironment::Source
  }

  override predicate isSink(Node node) {
    node instanceof CommandInjection::Sink or
    node instanceof ShellCommandInjectionFromEnvironment::Sink
  }
}

which marks both sources and sinks from the two queries as sources and sinks for your new query. If you want to make it slightly more pretty you could do:

import semmle.javascript.security.dataflow.CommandInjectionCustomizations
import semmle.javascript.security.dataflow.ShellCommandInjectionFromEnvironmentCustomizations

class Source extends DataFlow::Node {
  Source() {
    this instanceof ShellCommandInjectionFromEnvironment::Source
    or
    this instanceof CommandInjection::Source
  }
}

class Sink extends DataFlow::Node {
  Sink() {
    this instanceof ShellCommandInjectionFromEnvironment::Sink
    or
    this instanceof CommandInjection::Sink
  }
}

In your QL file. Now the configuration's isSource and isSink remains the same one-liners, but you've included the sources and sinks from both queries in the configuration.

Of course, you can then create your own MyQuryCustomizations file with these Source and Sink classes if this is something you find yourself needing regularly.

Does that answer help?

[vovikhangcdv]

This is the answer I am looking for!
Thank you, @MathiasVP. It helps the code be cleaner and more straightforward.