Skip to content

Conversation

FilipLaurentiu
Copy link
Contributor

Pull Request type

Please add the labels corresponding to the type of changes your PR introduces:

  • Feature

What is the current behavior?

Felt implement the Copy trait and it could leak the secret information into memory.
Felt is not zeroized on drop, it need to be manually zeroized.

Resolves: #NA

What is the new behavior?

New type SecretFelt is introduced for this specific purpose. This type could be used to represent sensible information that will not be leaked into memory at the end of the program

Does this introduce a breaking change?

No - Only if a project use the zeroize function on Felt and upgrade to the latest version of this library, but from what I know, nobody use it (starknet-rs doesn't use it)

@tdelabro
Copy link
Collaborator

No - Only if a project use the zeroize function on Felt and upgrade to the latest version of this library, but from what I know, nobody use it (starknet-rs doesn't use it)

So yes haha ;)

…e is a must.

Add `inner_value` function that returnes a safe copy of the inner felt. The value will be zeroized automatically when is out of scope.
@tdelabro
Copy link
Collaborator

@FilipLaurentiu CI is failing

@FilipLaurentiu FilipLaurentiu requested a review from tdelabro July 29, 2025 15:13
@FilipLaurentiu FilipLaurentiu marked this pull request as draft July 30, 2025 11:33
@FilipLaurentiu FilipLaurentiu marked this pull request as ready for review July 30, 2025 15:45
@tdelabro
Copy link
Collaborator

tdelabro commented Aug 4, 2025

@FilipLaurentiu ok we have this secret felt now. But we can't do much with it. Just move it around in memory safely.

What operations can we safely implement on this type? Arithmetic (Add, Mul, and so on) seems out of reach due to the way the internal representation of Felt handles it.

    /// Field addition. Never overflows/underflows.
    impl ops::Add<Felt> for Felt {
        type Output = Felt;

        fn add(self, rhs: Felt) -> Self::Output {
            Self(self.0 + rhs.0)
        }
    }

    /// Field addition. Never overflows/underflows.
    impl ops::Add<&Felt> for Felt {
        type Output = Felt;

        fn add(self, rhs: &Felt) -> Self::Output {
            Self(self.0 + rhs.0)
        }
    }

A copy of the internal value will be created if I don't mistake.

We could add a constant time comparison for the secret felt, to add one more security measure against timing attacks.

@FilipLaurentiu FilipLaurentiu marked this pull request as draft August 4, 2025 15:00
…le` crate.

- Implement `from_random` for `SecretFelt` to generate a secret Felt using a CSPRNG
- Improve the `from_hex_string` example
- Change `from_bytes_be`, `from_bytes_le` function signature to accept a `[u8; 32]` instead of `Vec<u8>`. No conversion needed and the function can't fail anymore.
@FilipLaurentiu FilipLaurentiu marked this pull request as ready for review August 7, 2025 14:10
@FilipLaurentiu
Copy link
Contributor Author

@FilipLaurentiu ok we have this secret felt now. But we can't do much with it. Just move it around in memory safely.

What operations can we safely implement on this type? Arithmetic (Add, Mul, and so on) seems out of reach due to the way the internal representation of Felt handles it.

    /// Field addition. Never overflows/underflows.
    impl ops::Add<Felt> for Felt {
        type Output = Felt;

        fn add(self, rhs: Felt) -> Self::Output {
            Self(self.0 + rhs.0)
        }
    }

    /// Field addition. Never overflows/underflows.
    impl ops::Add<&Felt> for Felt {
        type Output = Felt;

        fn add(self, rhs: &Felt) -> Self::Output {
            Self(self.0 + rhs.0)
        }
    }

A copy of the internal value will be created if I don't mistake.

We could add a constant time comparison for the secret felt, to add one more security measure against timing attacks.

I manage to implement a constant time comparison function for the SecretFelt. I don't see a way to add arithmetic functions to it, the reason will be, as you said, the way Felt work under the hood.

One big usage for SecretFelt I see is to use as a secret key for signatures. For example sign function takes a referance to a Felt and SecretFelt could be used here

@tdelabro
Copy link
Collaborator

@FilipLaurentiu I think once you add Eq and fix the failing CI we will be good for merging

@FilipLaurentiu
Copy link
Contributor Author

@FilipLaurentiu I think once you add Eq and fix the failing CI we will be good for merging

Done, ready to go 👍

@tdelabro tdelabro merged commit 820c306 into starknet-io:main Aug 20, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants