Skip to content
New issue

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

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

Java: Add experimental queries #80

Merged
merged 16 commits into from
Nov 20, 2024
Merged

Conversation

michaelnebel
Copy link
Collaborator

@michaelnebel michaelnebel commented Nov 8, 2024

In this PR all the java experimental queries from the codeql repo is added to the community pack.

Some things to consider when review'ing.

  • It appears that some of the experimental queries already exist in some form in the community repo for CWE-078, which have diverged from the implementation in the codeql repo. It looks like there has been made some improvements to the queries in the codeql repo, which haven't been included in the Community repo. I have tried to "merge" the implementations. Looking at the history of the community repo it appears that changes were mostly made to avoid deprecation warnings (re-factor to use parameterized dataflow modules). Furthermore, it appears that there exist test versions of the queries as well. Why are they needed?
  • Most of the test output copied from main needed to be updated as Data flow: Order provenance output by textual representation github/codeql#17887 is not included in 2.19.2.
  • A copy of the TestUtilities needed to be included as they currently live in the codeql test qlpack (which is not published). Maybe we should consider to move these into a pack where they can be referenced from this repo.
  • The experimental queries contain lots of local query variants. Should we keep these or should they be deleted (threat models can be used to enable local models)?
  • The experimental queries in the CodeQL repo also uses query specific MaD models (called experimental models). We should consider to add these as ordinary models in this repo instead (then they will also be shared across all the queries).

Reviewing on a commit by commit basis is encouraged.

@michaelnebel michaelnebel force-pushed the java/addexperimentalqueries branch 4 times, most recently from 237274b to 9efb681 Compare November 12, 2024 12:58
@michaelnebel michaelnebel force-pushed the java/addexperimentalqueries branch from 9efb681 to 2856834 Compare November 12, 2024 14:30
@michaelnebel michaelnebel changed the title (WIP) Java: Add experimental queries Java: Add experimental queries Nov 12, 2024
@michaelnebel michaelnebel marked this pull request as ready for review November 12, 2024 15:16
@pwntester
Copy link

Furthermore, it appears that there exist test versions of the queries as well. Why are they needed?

Not sure, seems like these are combining local and remote sources. Can they be removed @GeekMasher ?

A copy of the TestUtilities needed to be included as they currently live in the codeql test qlpack (which is not published). Maybe we should consider to move these into a pack where they can be referenced from this repo.

You mean publishing them into a codeql/ pack so they can be referenced from these packs and the official ones, right? Sounds like a good idea to me.

The experimental queries contain lots of local query variants. Should we keep these or should they be deleted (threat models can be used to enable local models)?

I think the field/PS teams used them for PoCs. @GeekMasher do you think using Local Threat Models can get you what you were using these queries for?

The experimental queries in the CodeQL repo also uses query specific MaD models (called experimental models). We should consider to add these as ordinary models in this repo instead (then they will also be shared across all the queries).

Agreed

Reviewing on a commit by commit basis is encouraged.

I wish my commits were as clean and organized as yours, it was a breeze to review!

@michaelnebel
Copy link
Collaborator Author

@pwntester : Thank you very much for the review and the kinds comments :-)

  • A PR is on the way for moving the TestUtilities into a codeql pack (from the Code QL repo) and then we can use that when it is released.
  • I will file a PR (soon) for converting the experimental models to "production" models.

@michaelnebel michaelnebel merged commit 00ea65f into main Nov 20, 2024
13 checks passed
@michaelnebel michaelnebel deleted the java/addexperimentalqueries branch November 20, 2024 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants