Skip to content

CPP: TlsSettingsMisconfiguration.ql performance and cleanup - #2249

Merged
jbj merged 7 commits into
github:masterfrom
geoffw0:tlsperf
Nov 11, 2019
Merged

CPP: TlsSettingsMisconfiguration.ql performance and cleanup#2249
jbj merged 7 commits into
github:masterfrom
geoffw0:tlsperf

Conversation

@geoffw0

@geoffw0 geoffw0 commented Nov 4, 2019

Copy link
Copy Markdown
Contributor

This query was performing really poorly, due to some haphazard use of dataflow. This PR makes some improvements, not all of which are exactly behaviour preserving - but the tests are not affected, nor are results on any snapshot I've found.

@geoffw0 geoffw0 added the C++ label Nov 4, 2019
@geoffw0
geoffw0 requested a review from a team as a code owner November 4, 2019 14:26

@jbj jbj left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The changes LGTM, but I think there is some more low-hanging fruit to pick. The from ... select contains lots of restrictions that I think could be moved into the isSource definition. A rule of thumb in data flow queries is that everything that can be phrased as a unary predicate on source or sink should be moved into isSource and isSink respectively.

Concretely, I think we want a predicate isProtocolSource(Expr protocolSource, ConstructorCall cc, Expr e, string msg), and isSource is then isProtocolSource(source.asExpr(), _, _, _).

@geoffw0 geoffw0 added this to the 1.23 milestone Nov 7, 2019
@geoffw0

geoffw0 commented Nov 7, 2019

Copy link
Copy Markdown
Contributor Author

Thanks for the suggestions - I will do a follow-up PR after this one is merged.

The main thing I wanted to fix here was ExistsAnyFlowConfig, which had source any() and sink any() before these changes - big red flag.

@jbj

jbj commented Nov 8, 2019

Copy link
Copy Markdown
Contributor

The qlformat check has failed.

@geoffw0

geoffw0 commented Nov 8, 2019

Copy link
Copy Markdown
Contributor Author

Fixed format.

@jbj
jbj merged commit ec79bfa into github:master Nov 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants