Rust: Add some flow source models #18069
Conversation
geoffw0
added
no-change-note-required
| */ | ||
| private class StdEnvArgs extends CommandLineArgsSource::Range { | ||
| StdEnvArgs() { | ||
| this.asExpr().getExpr().(CallExpr).getExpr().(PathExpr).getPath().getResolvedPath() = ["crate::env::args", "crate::env::args_os"] |
There was a problem hiding this comment.
@redsun82 these paths don't look right - I assume the correct path would be something like std::env::args or crate::std::env::args? They do match both what we get in the tests and what we see in real databases at the moment.
There was a problem hiding this comment.
(fixing this will probably be follow-up work, what we have works well enough right now and is needed to get queries working)
There was a problem hiding this comment.
hmm, this might have to do with the prelude, I'll have a look
There was a problem hiding this comment.
ah, no, currently that is expected.
The thing is that a path only makes sense in the context of a crate (there is no such thing as a global concept of path across crates). So that path is the correct one relative to the std crate.
The other piece of information is the (highly non-standard and prone to be changed) getCrateOrigin, which gives lang:std for the std lib.
we could theoretically combine the two, it's just that while I'm quite confident of the correctness of the resolved path (up to the fact that it may not cover all entities yet), I'm still unsure about the effectiveness of getCrateOrigin.
There was a problem hiding this comment.
OK. It sounds like we could check getCrateOrigin for models where we're worried about ambiguity ("crate::get" makes me nervous), but perhaps not check it for cases where we aren't (e.g. "crate::env::vars") since that will be more robust if getCrateOrigin is potentially prone to change. Alternatively we could fall back on other details such as types to ensure our models are matching the right things - to the extent that we extract enough information.
There was a problem hiding this comment.
I've push a commit adding the restriction I described above.
I've also added an agenda item next week to discuss crate origins, how we can get reliable information and how they will interact with models-as-data.
| // --- stubs for the "reqwest" library --- | ||
|
|
||
| /* | ||
| --- we don't seem to have a way to use this, hence we currently test against the real reqwest library |
There was a problem hiding this comment.
@redsun82 I tried to stub reqwest, it has a much simpler interface than sqlx. But I ran into a problem that it's not possible (I think) to emulate the correct paths for the library and import it into a test.rs at the moment. When we can, we can switch to a stubbing approach for reqwest at least.
There was a problem hiding this comment.
(fixing this will probably be follow-up work, what we have works well enough right now)
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
I believe this is just waiting for approval. update: new conflicts... |
Add some flow source models:
reqwestlibraryThese were chosen to support writing simple test cases such as those we have for the SQL Injection query. We will need more and different sources to find much of real world security interest.
I've also added a bit more wiring in
Concepts.qll, and created a new query and test to expose the sources directly.