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

stellar-ledger crate depends on slip10 crate that depends on dependencies with security advisories #1706

Open
leighmcculloch opened this issue Nov 6, 2024 · 2 comments
Labels
bug Something isn't working

Comments

@leighmcculloch
Copy link
Member

What version are you using?

8163f30

What did you do?

cargo deny check advisories

What did you expect to see?

Swish success.

What did you see instead?

That the version of curve25519-dalek and ed25519-dalek required by the stellar-ledger crate, via the slip10 crate, have known security issues.

    curve25519-dalek v3.2.0
      └── ed25519-dalek v1.0.1
          └── slip10 v0.4.3
              └── stellar-ledger v21.5.0
error[vulnerability]: Timing variability in `curve25519-dalek`'s `Scalar29::sub`/`Scalar52::sub`
    ┌─ /Users/leighmcculloch/Code/stellar-cli/Cargo.lock:101:1
    │
101 │ curve25519-dalek 3.2.0 registry+https://github.com/rust-lang/crates.io-index
    │ ---------------------------------------------------------------------------- security vulnerability detected
    │
    = ID: RUSTSEC-2024-0344
    = Advisory: https://rustsec.org/advisories/RUSTSEC-2024-0344
    = 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:
      
      - 32-bit (see L106): <https://godbolt.org/z/zvaWxzvqv>
      - 64-bit (see L48): <https://godbolt.org/z/PczYj7Pda>
      
      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:
      
      - 32-bit: <https://godbolt.org/z/jc9j7eb8E>
      - 64-bit: <https://godbolt.org/z/x8d46Yfah>
      
      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: https://github.com/dalek-cryptography/curve25519-dalek/pull/659
    = Solution: Upgrade to >=4.1.3 (try `cargo update -p curve25519-dalek`)
    = curve25519-dalek v3.2.0
      └── ed25519-dalek v1.0.1
          └── slip10 v0.4.3
              └── stellar-ledger v21.5.0

error[vulnerability]: Double Public Key Signing Function Oracle Attack on `ed25519-dalek`
    ┌─ /Users/leighmcculloch/Code/stellar-cli/Cargo.lock:135:1
    │
135 │ ed25519-dalek 1.0.1 registry+https://github.com/rust-lang/crates.io-index
    │ ------------------------------------------------------------------------- security vulnerability detected
    │
    = ID: RUSTSEC-2022-0093
    = Advisory: https://rustsec.org/advisories/RUSTSEC-2022-0093
    = Versions of `ed25519-dalek` prior to v2.0 model private and public keys as
      separate types which can be assembled into a `Keypair`, and also provide APIs
      for serializing and deserializing 64-byte private/public keypairs.
      
      Such APIs and serializations are inherently unsafe as the public key is one of
      the inputs used in the deterministic computation of the `S` part of the signature,
      but not in the `R` value. An adversary could somehow use the signing function as
      an oracle that allows arbitrary public keys as input can obtain two signatures
      for the same message sharing the same `R` and only differ on the `S` part.
      
      Unfortunately, when this happens, one can easily extract the private key.
      
      Revised public APIs in v2.0 of `ed25519-dalek` do NOT allow a decoupled
      private/public keypair as signing input, except as part of specially labeled
      "hazmat" APIs which are clearly labeled as being dangerous if misused.
    = Announcement: https://github.com/MystenLabs/ed25519-unsafe-libs
    = Solution: Upgrade to >=2 (try `cargo update -p ed25519-dalek`)
    = ed25519-dalek v1.0.1
      └── slip10 v0.4.3
          └── stellar-ledger v21.5.0

Discussion

The stellar-cli does not currently use the stellar-ledger crate, and to this date we have not actually published the crate either. Before it is used, we should address these advisories.

cc @janewang @fnando @elizabethengelman @willemneal

@leighmcculloch leighmcculloch added the bug Something isn't working label Nov 6, 2024
@github-project-automation github-project-automation bot moved this to Backlog (Not Ready) in DevX Nov 6, 2024
@leighmcculloch
Copy link
Member Author

Until this issue is addressed I'm marking the stellar-ledger crate as publish = false in:

@willemneal
Copy link
Member

https://crates.io/crates/hd-wallet

Alternative crate.

Slip10 looks abandoned

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Backlog (Not Ready)
Development

No branches or pull requests

2 participants