You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
It looks like taint flows through the conversions with the AST taint lib, but not with the IR taint lib or DefaultTaintTracking - because only taint from a function call parameter to the return value is considered, and in the case of c_str() it comes from the object / qualifier. The issue originally came up with ExecTainted.ql which is DefaultTaintTracking.
This is not a regression as far as I'm aware.
@jbj do you think it's worth improving DefaultTaintTracking, or maybe moving the query across to semmle.code.cpp.dataflow.TaintTracking? Which of the taint libs have a long term future?
The medium-term goal is to use semmle.code.cpp.ir.dataflow.DataFlow as the underlying implementation for everything. DefaultTaintTracking is a layer on top of that which we used to migrate the security pack queries away from their original library. I don't think we plan to do more work that's specific to DefaultTaintTracking, but it will benefit from improvements to semmle.code.cpp.ir.dataflow.DataFlow. For queries that could use more customization than DefaultTaintTracking allows, we could either use semmle.code.cpp.ir.dataflow.DataFlow directly or use semmle.code.cpp.ir.dataflow.TaintTracking.
#3587 should add those qualifier flows to all of the libraries I mentioned.
#3587 has been merged into master, and I've just merged master into this. The conflicts were in .expected files, but as far as I can tell are not consequential.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
For https://github.com/github/codeql-c-analysis-team/issues/88.
It looks like taint flows through the conversions with the AST taint lib, but not with the IR taint lib or DefaultTaintTracking - because only taint from a function call parameter to the return value is considered, and in the case of
c_str()it comes from the object / qualifier. The issue originally came up withExecTainted.qlwhich is DefaultTaintTracking.This is not a regression as far as I'm aware.
@jbj do you think it's worth improving DefaultTaintTracking, or maybe moving the query across to
semmle.code.cpp.dataflow.TaintTracking? Which of the taint libs have a long term future?