Skip to content

.qll Contribution for Sink Detection #19403

New issue

Have a question about this project? No Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “No 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? No Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

shyun020
Copy link

Hello,

After reviewing the Python CodeQL queries, I found that only frameworks defined with .qll files under the python/ql/lib/semmle/python/frameworks/ directory are recognized as sinks.

For example, MarkupSafe is correctly detected because it is defined in that location, whereas MarkUp is currently not recognized as a sink due to the absence of a .qll definition.

Therefore, I would like to create a new .qll file for MarkUp, add the corresponding unit tests, and submit a Pull Request.

The Taint Tracking query used for this test was as follows:

/**
 * @kind path-problem
 * @id python/taint-tracking
 */

import python
import semmle.python.dataflow.new.DataFlow
import semmle.python.dataflow.new.TaintTracking
import semmle.python.dataflow.new.RemoteFlowSources

module Config implements DataFlow::ConfigSig {
  predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }

  predicate isSink(DataFlow::Node sink) { any() }
}

module Flow = TaintTracking::Global<Config>;

import Flow::PathGraph

from Flow::PathNode source, Flow::PathNode sink
where Flow::flowPath(source, sink)
select sink.getNode(), source, sink,
"sink function: " + sink.getNode().asExpr().getScope().(Function).getName()

The target code tested was as follows:

from flask import Flask, request
import markupsafe
import markup
import yaml

app = Flask(__name__)

@app.route('/markupsafe', methods=['POST'])
def markupsafe_sink():
    user_input = request.form.get('input', '')
    result = markupsafe.Markup(user_input)
    return f"Rendered using markupsafe: {result}"

@app.route('/markup', methods=['POST'])
def markup_sink():
    user_input = request.form.get('input', '')
    page = markup.page()
    page += markup.div(user_input)
    return str(page)

@app.route('/pyyaml', methods=['POST'])
def pyyaml_sink():
    user_input = request.form.get('input', '')
    yaml_data = "---\n" + user_input
    loaded = yaml.load(yaml_data, Loader=yaml.Loader)
    return f"YAML loaded: {loaded}"

if __name__ == '__main__':
    app.run(debug=True)

As you can see, markupsafe.Markup is successfully recognized as a sink, but markup.page() and markup.div() are not detected, due to the missing .qll definition for the markup library.

I have summarized the detailed information in the Notion document linked below for your reference:

Thank you very much for your time and consideration.

Best regards,
SooHyun Kim

@shyun020 shyun020 requested a review from a team as a code owner April 28, 2025 16:05
@shyun020
Copy link
Author

@erik-krogh

I am unsure why the merge was blocked.
Would you be able to clarify what the problem might be?

*/

private import python
private import semmle.python.dataflow.new.DataFlow

Check warning

Code scanning / CodeQL

Redundant import Warning

Redundant import, the module is already imported inside
semmle.python.ApiGraphs
.
@shyun020
Copy link
Author

@erik-krogh

knock knock

@erik-krogh
Copy link
Contributor

knock knock

No.

I'm not the one to review this, the relevant team should have been pinged, but you need to be more patient.

Why did you ping me?

@shyun020
Copy link
Author

knock knock

No.

I'm not the one to review this, the relevant team should have been pinged, but you need to be more patient.

Why did you ping me?

I'm sorry about that. I'll be more patient and wait a bit longer.

May I kindly ask which team would be the right one to reach out to?

Apologies again for the inconvenience.

@erik-krogh
Copy link
Contributor

Why did you ping specifically me?

@hvitved
Copy link
Contributor

hvitved commented Apr 30, 2025

@yoff : Would you perhaps be able to review this?

@shyun020
Copy link
Author

Why did you ping specifically me?

I apologize for the ping — I had mistakenly thought you were the maintainer.

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

Successfully merging this pull request may close these issues.

3 participants