Inquire about the implementation details of python/cwe-502 in Codeql

[fe1w0]

Hi, CodeQL Team!

I hope this message finds you well.
There are some questions about codeql-main/python/ql/src/Security/CWE-502/UnsafeDeserialization.ql, regarding implementation details.

As shown in the following ql file, no other source, sink, or flow to are set in the overall query.

• CWE-502/UnsafeDeserialization.ql

import python
import semmle.python.security.dataflow.UnsafeDeserializationQuery
import UnsafeDeserializationFlow::PathGraph

from UnsafeDeserializationFlow::PathNode source, UnsafeDeserializationFlow::PathNode sink
where UnsafeDeserializationFlow::flowPath(source, sink)
select sink.getNode(), source, sink, "Unsafe deserialization depends on a $@.", source.getNode(),
  "user-provided value"

• UnsafeDeserializationQuery.ql

private module UnsafeDeserializationConfig implements DataFlow::ConfigSig {
  predicate isSource(DataFlow::Node source) { source instanceof Source }

  predicate isSink(DataFlow::Node sink) { sink instanceof Sink }

  predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
}

/** Global taint-tracking for detecting "code execution from deserialization" vulnerabilities. */
module UnsafeDeserializationFlow = TaintTracking::Global<UnsafeDeserializationConfig>;

Therefore, my confusion is that codeql is the first in the internal process, has the pickle.loads parameter as the sink parameter. If yes, can you tell me about the rule file conveniently ? In addition, what are the default source and sink variables in personal curiosity codeql

[RasmusWL]

Hi @fe1w0, if you open the file in VS Code with the CodeQL extension installed, you can simply ctrl+click Source to see the definition 😊

This takes you to the following piece of code:

codeql/python/ql/lib/semmle/python/security/dataflow/UnsafeDeserializationCustomizations.qll

Lines 19 to 54 in 4bc4e08

	/**
	* A data flow source for "code execution from deserialization" vulnerabilities.
	*/
	abstract class Source extends DataFlow::Node { }
	/**
	* A data flow sink for "code execution from deserialization" vulnerabilities.
	*/
	abstract class Sink extends DataFlow::Node { }
	/**
	* A sanitizer for "code execution from deserialization" vulnerabilities.
	*/
	abstract class Sanitizer extends DataFlow::Node { }
	/**
	* A source of remote user input, considered as a flow source.
	*/
	class RemoteFlowSourceAsSource extends Source, RemoteFlowSource { }
	/**
	* An insecure decoding, considered as a flow sink.
	*/
	class InsecureDecodingAsSink extends Sink {
	InsecureDecodingAsSink() {
	exists(Decoding d |
	d.mayExecuteInput() and
	this = d.getAnInput()
	)
	}
	}
	/**
	* A comparison with a constant string, considered as a sanitizer-guard.
	*/
	class StringConstCompareAsSanitizerGuard extends Sanitizer, StringConstCompareBarrier { }

What it means is that:

• Any RemoteFlowSouce should be a Source
• input to any decoding that may execute input should be a Sink
• Comparisons with string constants are sanitizers

If you search our code for pickle.loads you'll find the concrete modeling, where you can see that it will be recognized as a sink:

codeql/python/ql/lib/semmle/python/frameworks/Stdlib.qll

Lines 1343 to 1357 in f3b27d6

	/**
	* A call to `pickle.loads`
	* See https://docs.python.org/3/library/pickle.html#pickle.loads
	*/
	private class PickleLoadsCall extends Decoding::Range, API::CallNode {
	PickleLoadsCall() { this = pickle_loads().getACall() }
	override predicate mayExecuteInput() { any() }
	override DataFlow::Node getAnInput() { result in [this.getArg(0), this.getArgByName("data")] }
	override DataFlow::Node getOutput() { result = this }
	override string getFormat() { result = "pickle" }
	}

[fe1w0]

Thank you, RasmusWL, for your patient response. My question has been resolved.🌟😀👍