-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Add Actix framework modeling and import to Frameworks.qll #19461
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds modeling support for the Actix framework by introducing a new CodeQL library file and ensuring it is imported in the main frameworks file.
- Introduces
Actix.qll
with a class to identify Actix handler parameters - Imports the new Actix model in
Frameworks.qll
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
File | Description |
---|---|
rust/ql/lib/codeql/rust/frameworks/Actix.qll | Adds Actix handler parameter modeling class |
rust/ql/lib/codeql/rust/Frameworks.qll | Imports the new Actix framework model |
Comments suppressed due to low confidence (2)
rust/ql/lib/codeql/rust/frameworks/Actix.qll:1
- No QL tests were added to validate the new Actix modeling. Consider adding
.ql
test cases to cover theActixHandlerParam
logic.
/**
rust/ql/lib/codeql/rust/frameworks/Actix.qll:12
- The
RemoteSource
module is not imported or fully qualified. It should extendDataFlow::RemoteSource::Range
to reference the correct class from the DataFlow module.
private class ActixHandlerParam extends RemoteSource::Range {
I think this is a very narrow model, but it's a decent starting point, and any models of remote taint sources are high value. I'm aware that you have a specific database this change is targeted at, I assume you've tested it works there, but we really should have at least one test case in the repo as well. I've created #19466 to address this shortfall and also track how we're doing on other web frameworks. |
#19466 has been merged into Also, CI says that Feel free to DM me if you need any help. |
I'll need to put this on hold while I catch-up on some other stuff. I'll reach out probably next week to see how the test works. |
Pull Request checklist
All query authors
.qhelp
. See the documentation in this repository.Internal query authors only
.ql
,.qll
, or.qhelp
files. See the documentation (internal access required).