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.

Sign up for GitHub

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

Jump to bottom

panick when running 'cargo deny check advisories' on project ripasso #122

Closed
alexanderkjall opened this issue Feb 2, 2020 · 4 comments · Fixed by #129
Closed

panick when running 'cargo deny check advisories' on project ripasso #122

alexanderkjall opened this issue Feb 2, 2020 · 4 comments · Fixed by #129
Labels
bug Something isn't working

Comments

@alexanderkjall
Copy link

Describe the bug
cargo-deny panicked when trying to check advisories.

To Reproduce
Steps to reproduce the behavior:

  1. cargo install cargo-deny
  2. git clone git@github.com:cortex/ripasso.git
  3. cd ripasso
  4. cargo deny check advisories

Expected behavior
Get a list of security problems

Instead got this output:

2020-02-02 15:53:48 [WARN] config path '/tmp/ripasso/deny.toml' doesn't exist, falling back to default config
thread '<unnamed>' panicked at 'internal error: entered unreachable code: the advisory database report contained an advisory 
                    that somehow matched a crate we don't know about:
Metadata {
    id: Id {
        kind: RUSTSEC,
        year: Some(
            2019,
        ),
        string: "RUSTSEC-2019-0006",
    },
    package: Name(
        "ncurses",
    ),
    date: Date(
        "2019-06-15",
    ),
    aliases: [],
    references: [],
    collection: Some(
        Crates,
    ),
    categories: [],
    keywords: [],
    cvss: None,
    informational: None,
    obsolete: false,
    url: Some(
        "https://github.com/RustSec/advisory-db/issues/106",
    ),
    title: "Buffer overflow and format vulnerabilities in functions exposed without unsafe",
    description: "`ncurses` exposes functions from the ncurses library which:\n\n- Pass buffers without length to C functions that may write an arbitrary amount of\n  data, leading to a buffer overflow. (`instr`, `mvwinstr`, etc)\n- Passes rust &str to strings expecting C format arguments, allowing hostile\n  input to execute a format string attack, which trivially allows writing\n  arbitrary data to stack memory (functions in the `printw` family).\n",
    patched_versions: [],
    unaffected_versions: [],
}', /home/capitol/.cargo/registry/src/github.com-1ecc6299db9ec823/cargo-deny-0.6.2/src/advisories/mod.rs:229:17
stack backtrace:
   0: std::sys_common::backtrace::print
   1: std::panicking::default_hook::{{closure}}
   2: std::panicking::default_hook
   3: std::panicking::rust_panic_with_hook
   4: std::panicking::continue_panic_fmt
   5: std::panicking::begin_panic_fmt
   6: cargo_deny::advisories::check::{{closure}}
   7: cargo_deny::advisories::check
   8: <std::panic::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once
   9: std::panicking::try::do_call
  10: __rust_maybe_catch_panic
  11: <rayon_core::job::HeapJob<BODY> as rayon_core::job::Job>::execute
  12: rayon_core::registry::WorkerThread::wait_until_cold
  13: rayon_core::registry::ThreadBuilder::run
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

Device:

  • OS: Ubuntu 19.10
  • Version 0.6.2
@alexanderkjall alexanderkjall added the bug Something isn't working label Feb 2, 2020
@Jake-Shadle
Copy link
Member

Thanks for the repro! Hopefully will have time to look at this tomorrow.

@Jake-Shadle
Copy link
Member

Ok, so I dug a little bit into this, it turns out it's due to how you structured your workspace!

You have a Cargo.toml in your root for ripasso itself, and then additional workspace members are added in the same root toml. So when we get the metadata with that given root crate, cargo says that ripasso is the root of the resolution tree, which krates considers to mean that you've specified a particular crate in a workspace to gather metadata from, in which case it will only add other workspace members that are actually used by that root, which is the exact opposite in your case, as it seems each of the other workspace members depend on the root ripasso, not the other way around.

The rustsec crate works purely on the lockfile, so it says there is an advisory for ncurses, but at that point we have filtered ncurses out of the graph due the aforementioned behavior, resulting in this unreachable case (which will need to be an error instead at least).

I'm actually not sure the what the cleanest way to fix this would be, as I absolutely do want to preserve the behavior of specifying a particular member of a workspace and using that as the singular root of the graph versus the default of using all of the workspace members as root, I just had never checked this against a workspace that was organized this way and missed it.

@Jake-Shadle
Copy link
Member

In the meantime, you can just specify the subdirectory of each of the other workspace members if you wish to check them.

@alexanderkjall
Copy link
Author

Thanks for the digging and workaround, specifying the members is not a problem, and it showed me a real security problem in one of my dependencies, so it was really useful.

@Jake-Shadle Jake-Shadle mentioned this issue Feb 5, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Canonicalized args EmbarkStudios/cargo-deny
2 participants
@alexanderkjall @Jake-Shadle