Skip to content

Conversation

hvitved
Copy link
Contributor

@hvitved hvitved commented Nov 1, 2022

Just like PathNode::getASuccessor is restricted to nodes that can reach a sink, it also makes sense to only expose nodes that can reach a sink. This means that the exposed PathNode class and the nodes predicate contain the same elements.

The reason for the updated C++ test output is because DefaultTaintTracking.qll did not previously restrict the nodes relation to nodes that can reach a sink.

@hvitved hvitved force-pushed the dataflow/path-node-reach-charpred branch from 315e92f to 2198eaa Compare November 1, 2022 09:56
@hvitved hvitved added the depends on internal PR This PR should only be merged in sync with an internal Semmle PR label Nov 1, 2022
@hvitved hvitved force-pushed the dataflow/path-node-reach-charpred branch from 2198eaa to edbf7e5 Compare November 1, 2022 11:05
@hvitved hvitved added the no-change-note-required This PR does not need a change note label Nov 1, 2022
@hvitved hvitved marked this pull request as ready for review November 1, 2022 14:02
@hvitved hvitved requested review from a team as code owners November 1, 2022 14:02
@hvitved hvitved force-pushed the dataflow/path-node-reach-charpred branch from 4dd2747 to 1711efc Compare November 3, 2022 14:52
Copy link
Contributor

@jketema jketema left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

C++ expected test output changes 👍


/**
* A `Node` augmented with a call context (except for sinks), an access path, and a configuration.
* Only those `PathNode`s that are reachable from a source, and which can reach a sink, are generated.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not entirely accurate. Nodes that are used in a subpath are also included.

Copy link
Contributor

@aschackmull aschackmull left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor nit re. a qldoc, otherwise LGTM.

@hvitved hvitved merged commit 587e673 into github:main Nov 4, 2022
@hvitved hvitved deleted the dataflow/path-node-reach-charpred branch November 4, 2022 09:17
owen-mc added a commit to owen-mc/codeql that referenced this pull request Nov 29, 2022
owen-mc added a commit to owen-mc/codeql that referenced this pull request Nov 29, 2022
owen-mc added a commit to owen-mc/codeql that referenced this pull request Nov 29, 2022
owen-mc added a commit to owen-mc/codeql that referenced this pull request Nov 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C# C++ DataFlow Library depends on internal PR This PR should only be merged in sync with an internal Semmle PR Java no-change-note-required This PR does not need a change note Python Ruby Swift
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants