Integration with cargo-audit/RustSec?

[tarcieri]

cargo-audit is a utility which compares dependencies in Cargo.lock agains the RustSec Advisory Database. I've opened an issue proposing a potential integration with Siderophile here:

rustsec/rustsec#89

Recently we published an advisory high severity but low exploitability vulnerability to the database which resulted in false positive alerts for many users. The issue linked above goes into details about why this is an interesting case of where a call graph analysis would've helped.

We've done some work on collecting paths to affected vulnerabilities already to support this kind of analysis, and have the ability to collect this sort of information in advisories. Here's an example:

https://github.com/RustSec/advisory-db/blob/a8e2ec8/crates/safe-transmute/RUSTSEC-2018-0013.toml#L21

[affected_paths]
">= 0.4.0, <= 0.10.0"  = ["safe_transmute::guarded_transmute_vec_permissive"]
"= 0.10.0"             = ["safe_transmute::guarded_transmute_to_bytes_vec"]

(sidebar: looking that again, it feels like we should swap these so the path is on the left and the impacted versions are on the right)

What we need out of a call graph analysis tool is something that can both compute the call graph for --all-features, and then a way of testing if particular paths exist in the call graph. Compared to what Siderophile already does, this seems fairly simple.

If this sounds like a good idea, I'm curious what you think the best way to integrate cargo-audit and Siderophile would be. Should we invoke it as a subprocess, or is there a way to use it as a library/crate dependency? Is there a particular crate we can use that provides the call graph analysis functionality in isolation?

[tarcieri]

I'm also curious if it might actually make sense to integrate with the full Siderophile, and what it might be able to tell you about this sort of vulnerability given the path to a known-vulnerable node in the call graph. It seems like it might be interesting if the affected function exists in the call graph to optionally fuzz it and see if it's possible to conclusively determine it's vulnerable.

[rozbb]

Since the tool gets to see the whole callgraph, all these analysis steps shouldn't be too difficult to implement. This project needs a bit more staffing, but this is doable.

As for integrating into another tool, this aligns with one of our goals, which is to strip out all the Python scripts and RWIIR. This way we can expose all this functionality as a library.

Leaving this issue open

[laudiacay]

Is this still something you're interested in? As of today, things are much readier for exposing some sort of callgraph library.

Also, @ issue #60

[tarcieri]

Absolutely! As soon as we can consume call graph analysis in library form we have data (i.e. paths to functions impacted by an advisory) necessary to perform this kind of analysis.

[tarcieri]

@laudiacay any progress on extracting a call graph library?

[laudiacay]

Working on this tonight. Hoping to have something for you by the morning? Sorry, responsibilities changed at work and I haven't had much time for this lately.

[tarcieri]

Haha, no rush! But that would be fantastic!

[laudiacay]

Ok- #78. This actually ended up going WAY quicker than I expected.

Caveats: I am not an experienced Rust developer, neither am I used to making my code play nice with others'... so please let me know what else I should do to get this into a usable place for y'all.

Tldr: Currently, I'm exposing 5 things. You can see how I call them in src/main.rs.

1. configure_rustup_toolchain is a function that needs to be run before the useful functions get used, to set the RUST_TOOLCHAIN variable and make sure everything's using the right versions (I didn't write the original solution for this, but it's documented more in issue 22 on this repo. hoping to refactor this out at some point)
2. simplify_trait_paths is a function that gets called on function names after demangling to simplify trait paths, so that we can check when functions are actually the same even though they're written differently? It's in utils.rs with tests included so you can see what it actually does. I have a hunch it will come in handy for Rustsec's usage of this, though.
3. The CallGraph datatype- basically it is an adjacency matrix in label_to_caller_labels with some extra records-keeping of when we've used different names for the same thing (like in simplify_trait_paths).
4. gen_callgraph generates a CallGraph, by taking in a cargo workspace (as created with the cargo library) and a crate name. It calls cargo to clean the workspace and emit bitcode, scans through the workspace files to find the emitted bitcode, and then parses the bitcode with the llvm-ir library. Finally, it traces different kinds of function calls that can be statically figured out, and builds a CallGraph.
5. trace_unsafety takes a CallGraph, a crate name, and a list of tainted function names, and it does the callgraph analysis and function scoring as described in the siderophile readme. This function is a good example for how to properly traverse the callgraph and how to treat function names correctly re: demangling/simplifying them for comparisons- you might not do exactly this, but you'll probably do something really similar.

[laudiacay]

Took another look at your specific use case before I log off for tonight-

What we need out of a call graph analysis tool is something that can both compute the call graph for --all-features, and then a way of testing if particular paths exist in the call graph. Compared to what Siderophile already does, this seems fairly simple.

basically what you should do is put the "particular path" into trace_unsafety as the one tainted function name. then anything with a nonzero score that comes out, will be in the crate_name provided to the function, and has a path to the thing you loaded into trace_unsafety.

[tarcieri]

@laudiacay yeah, nice! Just exposing the core types from a [[lib]] crate in the same package is all we really need to get started

[laudiacay]

Awesome- if I merge that PR, are y'all set?

[tarcieri]

Should be enough to get started, I hope!

[laudiacay]

🎊 🎊 merged it into master! i'm going to close this issue if you're all set, but feel free to open another if y'all need anything else.

[tarcieri]

Thank you!