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

[leighmcculloch]

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 <alexander.wagner@aisec.fraunhofer.de> and Lea Themint <lea.thiemt@tum.de> 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]

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

• Add cargo-deny for license checks and dep duplicate avoidance #1685

[willemneal]

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

Alternative crate.

Slip10 looks abandoned

[github-actions]

This issue is stale because it has been assigned for 30 days with no activity. It will be closed in 90 days unless the stale label is removed, and the assignee is removed or updated.

[leighmcculloch]

Did this already get fixed? I haen't seen the warning anymore.

[Ifropc]

We can close the issue, current version of stellar-ledger (22.2.0) depends on slipped10 now that is not vulnerable

[[package]]
name = "stellar-ledger"
version = "22.2.0"
dependencies = [
 "async-trait",
 "bollard",
 "byteorder 1.5.0",
 "ed25519-dalek",
 "env_logger",
 "futures",
 "hex",
 "home",
 "httpmock",
 "ledger-transport",
 "ledger-transport-hid",
 "log",
 "once_cell",
 "phf",
 "pretty_assertions",
 "reqwest",
 "sep5",
 "serde",
 "serde_derive",
 "serde_json",
 "serial_test",
 "sha2 0.10.8",
 "slipped10",
 "soroban-spec",
 "stellar-rpc-client",
 "stellar-strkey 0.0.11",
 "stellar-xdr",
 "test-case",
 "testcontainers",
 "thiserror",
 "tokio",
 "tracing",
]