search

tkhq / qos

QuorumOS is a computation layer for running applications inside Trusted Execution Environments (TEEs)
GNU Affero General Public License v3.0
5 stars 2 forks source link

Update vulnerable deps #464

Closed taylorjdawson closed 4 months ago

taylorjdawson commented 5 months ago

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

Pre merge check list

james-callahan commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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 commented 4 months ago

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.