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

Update vulnerable deps #464

Closed
wants to merge 2 commits into from
Closed

Conversation

taylorjdawson
Copy link
Contributor

@taylorjdawson taylorjdawson commented Jun 25, 2024

Summary & Motivation (Problem vs. Solution)

This PR addresses security vulnerabilities by updating key dependencies to their latest, patched versions, ensuring SOC2 compliance.

Vulnerable Package Updates

Package Module Old Version New Version Vulnerability Severity
shlex qos_enclave 1.1.0 1.3.0 GHSA-r7qv-8r2h-pg27 HIGH
mio root, qos_enclave root@0.8.8, qos_enclave@0.8.6 0.8.11 CVE-2024-27308, CVE-2024-27308 HIGH
vmm-sys-util qos_enclave 0.11.1 0.12.1 CVE-2023-50711 MEDIUM
openssl qos_enclave 0.10.52 0.10.64 GHSA-xcf7-rvmh-g6q4, GHSA-xphf-cx8h-7q9g MEDIUM
time qos_client 0.1.45 0.3.36 CVE-2020-26235 MEDIUM
rustix qos_enclave 0.37.15 0.38.34 GHSA-c827-hfw6-qwvm MEDIUM

Required Package Updates

Package Module Old Version New Version Reason
tokio root, qos_enclave 1.28.0 1.38.0 Update due to vulnerable dependencies: mio
libc root, qos_enclave 0.2.148 0.2.155 Necessary update following Tokio upgrade
nitro-cli qos_enclave 1.2.2 1.3.1 Update due to vulnerable dependencies: shlex, vmm-sys-util, openssl, rustix

Irremediable

rsa - MEDIUM

Work is ongoing to resolve this vulnerability. rsa is a dependency of yubikey, and it is anticipated that yubikey will update their rsa dependency to the latest patched version once available.

atty - LOW

As of now, there is no known remediation for atty, and the package appears to be unmaintained, according to GitHub vulnerability information. atty is a dependency of clap, which is a Rust command line parser used by nitro-cli. Newer versions of clap have removed atty as a dependency.

How I Tested These Changes

  • Running make test & make build in affected modules

Pre merge check list

  • Update CHANGELOG.MD

@taylorjdawson taylorjdawson requested a review from a team as a code owner June 25, 2024 22:19
@taylorjdawson taylorjdawson requested review from r-n-o and cr-tk June 26, 2024 20:03
@james-callahan
Copy link
Contributor

This PR addresses security vulnerabilities by updating key dependencies to their latest, patched versions, ensuring SOC2 compliance.

This is a huge amount of new code.

Has it been reviewed? I believe the policy for qos is to review every dependency line by line.

@lrvick
Copy link
Contributor

lrvick commented Jul 3, 2024

It could take a day or more to review the diffs between all these libraries for supply chain attacks, so should probably flag this for approval to spend that much time.

Are we actually patching any vulns that impact any actual codepaths we use?

CC @jack-kearney

@jack-kearney
Copy link
Contributor

jack-kearney commented Jul 3, 2024

What I'd like to propose is the following:

  1. @taylorjdawson: Could you please break this PR into a series of smaller PRs that address individual dependencies that can be more granularly reviewed?
  2. @james-callahan / @cr-tk / @r-n-o: Could you help review the diffs of these updated PRs?
  3. @lrvick / @cr-tk / @r-n-o: Let's spend some time (could be at the offsite, could be beforehand) discussing the higher-level strategy here. At the very least we should have some guidance / education for how to review for supply-chain attacks. Ideally we'd include a bit of tooling to help with the review process.

@cr-tk
Copy link
Collaborator

cr-tk commented Jul 15, 2024

Quick update on this:

as I understand it,

Also note that some already-merged previous PRs have partially improved the situation by bumping versions for known problems in some sub-crates, for example the problematic time version (that leads to a potential segfault DOS) is now only used in src/qos_enclave/Cargo.lock.

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.

5 participants