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

BUG. Defective RIST255_MULT and RIST255_MULBASE implementations. #1428

Open
dta90d opened this issue Dec 11, 2024 · 16 comments
Open

BUG. Defective RIST255_MULT and RIST255_MULBASE implementations. #1428

dta90d opened this issue Dec 11, 2024 · 16 comments

Comments

@dta90d
Copy link

dta90d commented Dec 11, 2024

Hello!

I think I've stumbled upon a bug in implementation of Ristretto scalar multiplication.

For some reason the n parameter (Scalar) is being preprocessed like that:

auto n = stack.pop_int() % get_ristretto256_l();

auto n = stack.pop_int() % get_ristretto256_l();

Value of get_ristretto256_l() (l onwards) is 2^252 + 27742317777372353535851937790883648493 so that means that if the Scalar passed is less than l it goes directly in n (n == scalar_passed) and thus we get correct results, but if Scalar is equal or greater than l we end up with some small value in n and thus get wrong final results.

Here's a real-life example of what we should get (tested via curve25519_dalek, code below) and what we get in reality from TVM.

Let's take some point on a curve X and Scalar n that is greater than l where:

X = 0xf245308737c2a888ba56448c8cdbce9d063b57b147e063ce36c580194ef31a63
n = 0xd32fcc5ae91ba05704da9df434f22fd4c2c373fdd8294bbb58bf27292aeec00a

The results we should get are (B being RISTRETTO_BASEPOINT):

X * n = 0x5e727d972b11f6490b0b1ba8147775bceb1a2cb523b381fa22d5a5c0e97d4744 // MUL
B * n = 0x9a30709d72de12d67f7af1cd8695ff16214d2d4600ae5f478873d2e7ed0ece73 // MULBASE

But when processed through TVM we get different results:

PUSHINT 109581956733909874354130828266673789621712875798453783900027869828650322041443 // X
PUSHINT 95522463270312866099681725115318615928183786721526257398762142880656593436682 // n
RIST255_MUL // => 86949080096112004834615039891007688035110909213144700491546856921768921685808 == 0xc03b6f72e4205f5da1a7b1f8028c04927fe3558d6186050889c85a4c01085f30

PUSHINT 95522463270312866099681725115318615928183786721526257398762142880656593436682 // n
RIST255_MULBASE // => 19216342509223456543606312405417350433746860378776904565137185386174840323083 == 0x2a7c107e4a0e1fbf425fda5a46e95f8fd60e4b365120a3e8a49497f19d443c0b

So as we saw the resulting numbers are not correct.
Please fix this if it's indeed a bug cause it may pose real threat for some applications' security and working capacity.

Additionally, I attach a snippet of code for Rust script to quickly plug it in and test this issue yourself.

use curve25519_dalek::{Scalar, RistrettoPoint};
use curve25519_dalek::ristretto::{CompressedRistretto};

fn main() {
    let X: RistrettoPoint = CompressedRistretto::from_slice(hex::decode("f245308737c2a888ba56448c8cdbce9d063b57b147e063ce36c580194ef31a63").unwrap().as_slice()).unwrap().decompress().unwrap();
    let n: Scalar = Scalar::from_canonical_bytes(hex::decode("d32fcc5ae91ba05704da9df434f22fd4c2c373fdd8294bbb58bf27292aeec00a").unwrap().try_into().unwrap()).unwrap();
    let R: RistrettoPoint = X * n;
    let R2: RistrettoPoint = RistrettoPoint::mul_base(&n);
    println!("{}", format!("{}", ::hex::encode(R.compress().to_bytes().as_ref())));
    println!("{}", format!("{}", ::hex::encode(R2.compress().to_bytes().as_ref())));
}

Looking forward for your reply.
Thanks!

@EmelyanenkoK
Copy link
Member

Thank you! We will check.

@dta90d
Copy link
Author

dta90d commented Dec 12, 2024

@EmelyanenkoK Do you have an estimate when to expect the response?

@EmelyanenkoK
Copy link
Member

EmelyanenkoK commented Dec 13, 2024

TVM expects scalar in non-canonical form. You can compare results with the following rust code (from_canonical_bytes -> from_bytes_mod_order):

use curve25519_dalek::{Scalar, RistrettoPoint};
use curve25519_dalek::ristretto::{CompressedRistretto};

fn main() {
    let X: RistrettoPoint = CompressedRistretto::from_slice(hex::decode("f245308737c2a888ba56448c8cdbce9d063b57b147e063ce36c580194ef31a63").unwrap().as_slice()).unwrap().decompress().unwrap();
    let n: Scalar = Scalar::from_bytes_mod_order(hex::decode("d32fcc5ae91ba05704da9df434f22fd4c2c373fdd8294bbb58bf27292aeec00a").unwrap().try_into().unwrap());

    let R: RistrettoPoint = X * n;
    let R2: RistrettoPoint = RistrettoPoint::mul_base(&n);
    println!("{}", format!("{}", ::hex::encode(R.compress().to_bytes().as_ref())));
    println!("{}", format!("{}", ::hex::encode(R2.compress().to_bytes().as_ref())));
}

Seems like canonical and non-canonical forms can be calculated from each other but it requires non-trivial tinkering with higher bits.

@dta90d
Copy link
Author

dta90d commented Dec 17, 2024

Hello, please don't close the issue, I had no time to work on it and come up with the answer.

@dta90d
Copy link
Author

dta90d commented Dec 23, 2024

@EmelyanenkoK

Hello! I had time recently to explore the topic, so here are the results of my research.

First of all, I'm not a mathematician or Rust/C++ dev, I just happen to implement an algorithm for smart contract from the official specification that has test vectors and results of calculations of each step and I can't do it, because the values coming from the TVM's RIST255_MULT and RIST255_MULBASE functions just do not match the verified test values from the specification. So it's safe to assume there is something wrong with the implementation.

Secondly, I've tried out your code with from_bytes_mod_order method that can receive unreduced scalar and reduces it automatically. It outputs exactly the same results as my original code, meaning that reducing (or converting from non-canonical to canonical scalar format) should output the original value when the reduced scalar is being passed (and that's not happening for TVM according to your reply).

Thirdly, it is obvious from the curve25519_dalek::scalar docs that scalar that is being passed to the MUL and MULBASE functions MUST be canonical and that also means that converting already canonical scalar to canonical format should take no effects on value:

    /// Check whether this `Scalar` is the canonical representative mod \\(\ell\\). This is not
    /// public because any `Scalar` that is publicly observed is reduced, by scalar invariant #2.
    fn is_canonical(&self) -> Choice {
        self.ct_eq(&self.reduce())
    }

https://github.com/dalek-cryptography/curve25519-dalek/blob/43a16f03d4c635a8836c23ac07244c116ea3aab8/curve25519-dalek/src/scalar.rs#L1134

So that means that functions inside TVM should work the same way, which is not the case.

Also I've tried out libsodium library that TVM uses and it gives the exact same results as dalek and specification give, I can provide the code if you need more proof.

Looking forward for your reply.
Thanks!

@EmelyanenkoK
Copy link
Member

Secondly, I've tried out your code with from_bytes_mod_order method that can receive unreduced scalar and reduces it automatically. It outputs exactly the same results as my original code

Let's start with that

What your

use curve25519_dalek::{Scalar, RistrettoPoint};
use curve25519_dalek::ristretto::{CompressedRistretto};

fn main() {
    let X: RistrettoPoint = CompressedRistretto::from_slice(hex::decode("f245308737c2a888ba56448c8cdbce9d063b57b147e063ce36c580194ef31a63").unwrap().as_slice()).unwrap().decompress().unwrap();
    let n: Scalar = Scalar::from_canonical_bytes(hex::decode("d32fcc5ae91ba05704da9df434f22fd4c2c373fdd8294bbb58bf27292aeec00a").unwrap().try_into().unwrap()).unwrap();
    let R: RistrettoPoint = X * n;
    let R2: RistrettoPoint = RistrettoPoint::mul_base(&n);
    println!("{}", format!("{}", ::hex::encode(R.compress().to_bytes().as_ref())));
    println!("{}", format!("{}", ::hex::encode(R2.compress().to_bytes().as_ref())));
}

and "ours"

use curve25519_dalek::{Scalar, RistrettoPoint};
use curve25519_dalek::ristretto::{CompressedRistretto};

fn main() {
    let X: RistrettoPoint = CompressedRistretto::from_slice(hex::decode("f245308737c2a888ba56448c8cdbce9d063b57b147e063ce36c580194ef31a63").unwrap().as_slice()).unwrap().decompress().unwrap();
    let n: Scalar = Scalar::from_bytes_mod_order(hex::decode("d32fcc5ae91ba05704da9df434f22fd4c2c373fdd8294bbb58bf27292aeec00a").unwrap().try_into().unwrap());

    let R: RistrettoPoint = X * n;
    let R2: RistrettoPoint = RistrettoPoint::mul_base(&n);
    println!("{}", format!("{}", ::hex::encode(R.compress().to_bytes().as_ref())));
    println!("{}", format!("{}", ::hex::encode(R2.compress().to_bytes().as_ref())));
}

snippets output on your side?

@dta90d
Copy link
Author

dta90d commented Dec 23, 2024

@EmelyanenkoK

Here's output of "my" snippet:

5e727d972b11f6490b0b1ba8147775bceb1a2cb523b381fa22d5a5c0e97d4744
9a30709d72de12d67f7af1cd8695ff16214d2d4600ae5f478873d2e7ed0ece73

Here's output of "your" snippet:

5e727d972b11f6490b0b1ba8147775bceb1a2cb523b381fa22d5a5c0e97d4744
9a30709d72de12d67f7af1cd8695ff16214d2d4600ae5f478873d2e7ed0ece73

@dta90d
Copy link
Author

dta90d commented Dec 23, 2024

@EmelyanenkoK Here's full code that you can verify.

use curve25519_dalek::{Scalar, RistrettoPoint};
use curve25519_dalek::ristretto::{CompressedRistretto};

fn main1() {
    let X: RistrettoPoint = CompressedRistretto::from_slice(hex::decode("f245308737c2a888ba56448c8cdbce9d063b57b147e063ce36c580194ef31a63").unwrap().as_slice()).unwrap().decompress().unwrap();
    let n: Scalar = Scalar::from_canonical_bytes(hex::decode("d32fcc5ae91ba05704da9df434f22fd4c2c373fdd8294bbb58bf27292aeec00a").unwrap().try_into().unwrap()).unwrap();
    let R: RistrettoPoint = X * n;
    let R2: RistrettoPoint = RistrettoPoint::mul_base(&n);
    println!("{}", format!("{}", ::hex::encode(R.compress().to_bytes().as_ref())));
    println!("{}", format!("{}", ::hex::encode(R2.compress().to_bytes().as_ref())));
}


fn main2() {
    let X: RistrettoPoint = CompressedRistretto::from_slice(hex::decode("f245308737c2a888ba56448c8cdbce9d063b57b147e063ce36c580194ef31a63").unwrap().as_slice()).unwrap().decompress().unwrap();
    let n: Scalar = Scalar::from_bytes_mod_order(hex::decode("d32fcc5ae91ba05704da9df434f22fd4c2c373fdd8294bbb58bf27292aeec00a").unwrap().try_into().unwrap());

    let R: RistrettoPoint = X * n;
    let R2: RistrettoPoint = RistrettoPoint::mul_base(&n);
    println!("{}", format!("{}", ::hex::encode(R.compress().to_bytes().as_ref())));
    println!("{}", format!("{}", ::hex::encode(R2.compress().to_bytes().as_ref())));
}

fn main() {
    println!("Main 1:");
    main1();
    println!("\nMain 2:");
    main2();
}

Output:

Main 1:
5e727d972b11f6490b0b1ba8147775bceb1a2cb523b381fa22d5a5c0e97d4744
9a30709d72de12d67f7af1cd8695ff16214d2d4600ae5f478873d2e7ed0ece73

Main 2:
5e727d972b11f6490b0b1ba8147775bceb1a2cb523b381fa22d5a5c0e97d4744
9a30709d72de12d67f7af1cd8695ff16214d2d4600ae5f478873d2e7ed0ece73

@dta90d
Copy link
Author

dta90d commented Dec 24, 2024

@EmelyanenkoK Hey, in case you wondering I've figured out proper reducing of the scalar using libsodium library and I can send you code with examples as proof that my theory is right. It works differently compared with dalek rust lib. You cannot pass 32 byte value in there, you have to explicitly pass 64 byte value into reduce function (crypto_core_ristretto255_scalar_reduce), otherwise the output will be not correct, even though there will be an output. It is slightly stated in the docs, but there is no warning or anything.

Note that s is much larger than r (64 bytes vs 32 bytes). Bits of s can be left to 0, but the interval s is sampled from should be at least 317 bits to ensure almost uniformity of r over L.

@ me if you need code with examples as proof and I will send it to you.

@Marodero56

This comment was marked as spam.

@dta90d
Copy link
Author

dta90d commented Jan 9, 2025

@EmelyanenkoK Hello! Are there any news concerning the issue? This issue is very significant for the project I'm working on.
Please, update us on the current status for this issue, thanks!

@EmelyanenkoK
Copy link
Member

Hi. We will return to this issue in a couple of weeks.

@Bratane

This comment was marked as spam.

@Bratane

This comment was marked as spam.

@dta90d
Copy link
Author

dta90d commented Feb 5, 2025

@EmelyanenkoK Hello! Hope you're doing well! Could you please give us a quick status update on the issue? We are planning on launching our product soon, so I'm curious if we have a possibility to resolve this issue beforehand 🙏🙏🙏

@dta90d
Copy link
Author

dta90d commented Mar 14, 2025

@EmelyanenkoK, Hello!

First of all, I would like to express my gratitude for your time engaging with this issue — I know how swamped core teams can be.

I have great news concerning the issue:

After thorough investigation, I can confirm that there was no error in the blockchain’s Ristretto implementation. The root cause of the problem indeed layed in the input values passed to the RIST255_MUL and RIST255_MULBASE functions. This has allowed us to refine the approach and achieve full compatibility with the blockchain’s Ristretto implementation to implement fully secure ECVRF smart contract.

So now as the issue is resolved I can announce that last week we've successfully launched our ToloTon App, a fairness-first TMA built on TON Blockchain. At its core lies our ECVRF smart contract, which guarantees cryptographically verifiable randomness for all draws.

During our research phase, we rigorously evaluated existing VRF implementations, and found only one solution: https://github.com/ProgramCrafter/ecvrf-verified-randomness/. Critical deviations from the RFC 9381 ECVRF standard rendered it unsuitable:

  1. Hash-Curve Mismatch: The ECVRF standard mandates SHA-512 with Curve25519 to mitigate multi-target attacks and ensure long-term security. Solutions using SHA-256 (128-bit collision resistance) with Curve25519 fail to meet the safety margins required for high-stakes applications.
  2. Non-Standard Domain Separation: Custom domain tags (e.g. "ecvrf.encodecurve.back", "ecvrf.beta.back", absence of front domain separation) bypass the RFC-specified format. This undermines interoperability and introduces ambiguity in cryptographic proofs.
  3. Serious deviations from RFC ECVRF_encode_to_curve algorithm: "ecvrf::rist255::encode_to_curve" function is not compliant with any of the algorithms listed in c2sp.org/vrf-r255, RFC-9381 and RFC-9380. This deviation from standarts leaves the implementation’s security unproven.
  4. Unverified Test Vectors: The absence of standardized test vectors makes it impossible to validate correctness against authoritative references.

These deviations create unquantified risks, particularly for applications demanding mathematically enforceable fairness. Adhering strictly to the standard (namely c2sp.org/vrf-r255 for Ristretto compatibility), our ECVRF contract resolves these shortcomings, with all outputs logged on-chain for public verification. While the code remains closed-source temporarily, we are developing an open-source validator tool that will be live soon.

We would be very gratefull if TON could support our project and help us with smart contract audit. Our ECVRF smart contract could become a universal infrastuctural and easy-to-use solution for all applications that need provable RNG functionality on TON network.

Thank you once again for your support in resolving this issue. With ToloTon now live, we’re excited to advance verifiable fairness within the TON ecosystem. This thread may be closed, but we welcome further dialogue on ECVRF best practices via our project channels.

Warm regards,
dta90d

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

No branches or pull requests

4 participants