xJonathanLEI / starknet-rs

Complete Starknet library in Rust™
https://starknet.rs
Apache License 2.0
286 stars 101 forks source link

bug(felt): Felt is not zeroized for sensitive cryptographic operations #667

Open Trantorian1 opened 1 month ago

Trantorian1 commented 1 month ago

Is there an existing issue?

Motivation

Felt is not zeroized on drop which might leak sensitive information into RAM, notably private keys used by Signers such as LocalWallet.

This is especially an issue when considering block signing which uses Felt in p2p context or for the future Tendermint consensus system in Starknet, where an adversarial validator could sign faulty blocks with the pk of another sequencer, resulting in a slashing attacks.

This is still not very serious, as there certainly are simpler ways for a pk to leak, but it seems like this should at least be an option for secutiry.

Request

Support zeroing for sensitive information in starknet-signers, starknet-crypto and starknet-core.

Solution

Since the use of Felt is based on starknet_types_core, a relevant issue has been opened there. An associated issue has also been opened on lambdaworks-math. Note that it would still be possible to manually implement zeroing on Felt through a custom trait, but this would be less clean and more error-prone since it would not be called automatically on drop.

Are you willing to help with this request?

Yes!

xJonathanLEI commented 1 month ago

It kinda sounds like mostly a concern of starknet_types_core if it wouldn't change its public API?

Assuming it will be implemented as a crate feature there, I guess we can have transitive features here on starknet-rs crates to enable the said new feature, though it seems not absolutely necessary, as downstream libs and apps can always directly turn starknet_types_core features on.

Of course, if turning on the feature causes API changes it will be a different story and starknet-rs must add direct support in that case.

Will leave this open to see what happens upstream.