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

Vulnerability dependency #214

Open
AlexD10S opened this issue Jun 20, 2024 · 0 comments
Open

Vulnerability dependency #214

AlexD10S opened this issue Jun 20, 2024 · 0 comments
Labels
dependencies Pull requests that update a dependency file

Comments

@AlexD10S
Copy link
Collaborator

Issue open to track an issue with one of our dependencies when running cargo deny check advisories:

Advisory: https://rustsec.org/advisories/RUSTSEC-2024-0344

Description

Timing variability of any kind is problematic when working with potentially secret values such as
elliptic curve scalars, and such issues can potentially leak private keys and other secrets. Such a
problem was recently discovered in curve25519-dalek.

The Scalar29::sub (32-bit) and Scalar52::sub (64-bit) functions contained usage of a mask value
inside a loop where LLVM saw an opportunity to insert a branch instruction (jns on x86) to
conditionally bypass this code section when the mask value is set to zero as can be seen in godbolt:

A similar problem was recently discovered in the Kyber reference implementation:

https://groups.google.com/a/list.nist.gov/g/pqc-forum/c/hqbtIGFKIpU/m/cnE3pbueBgAJ

As discussed on that thread, one portable solution, which is also used in this PR, is to introduce a
volatile read as an optimization barrier, which prevents the compiler from optimizing it away.

The fix can be validated in godbolt here:

The problem was discovered and the solution independently verified by
Alexander Wagner [email protected] and Lea Themint [email protected] using
their DATA tool:

https://github.com/Fraunhofer-AISEC/DATA

Announcement: dalek-cryptography/curve25519-dalek#659
Solution: Upgrade to >=4.1.3

curve25519-dalek v3.2.0
      └── ed25519-zebra v3.1.0
          └── sp-core v31.0.0
              ├── contract-extrinsics v4.1.1
              │   └── pop-contracts v0.2.0
              │       └── pop-cli v0.2.0
              ├── pop-cli v0.2.0 (*)
              ├── pop-contracts v0.2.0 (*)
              ├── sp-application-crypto v33.0.0
              │   └── sp-runtime v34.0.0
              │       ├── contract-extrinsics v4.1.1 (*)
              │       └── subxt v0.35.3
              │           ├── contract-extrinsics v4.1.1 (*)
              │           ├── pop-contracts v0.2.0 (*)
              │           ├── subxt-signer v0.35.3
              │           │   ├── pop-contracts v0.2.0 (*)
              │           │   └── zombienet-orchestrator v0.2.5
              │           │       └── zombienet-sdk v0.2.5
              │           │           └── pop-parachains v0.2.0
              │           │               └── pop-cli v0.2.0 (*)
              │           ├── zombienet-orchestrator v0.2.5 (*)
              │           └── zombienet-sdk v0.2.5 (*)
              ├── sp-io v33.0.0
              │   ├── sp-application-crypto v33.0.0 (*)
              │   └── sp-runtime v34.0.0 (*)
              ├── sp-keystore v0.37.0
              │   └── sp-io v33.0.0 (*)
              ├── sp-runtime v34.0.0 (*)
              ├── sp-state-machine v0.38.0
              │   └── sp-io v33.0.0 (*)
              ├── sp-trie v32.0.0
              │   ├── sp-io v33.0.0 (*)
              │   └── sp-state-machine v0.38.0 (*)
              ├── subxt v0.35.3 (*)
              └── zombienet-orchestrator v0.2.5 (*)
@AlexD10S AlexD10S added the dependencies Pull requests that update a dependency file label Jun 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

No branches or pull requests

1 participant