code-423n4 / 2024-03-phala-network-findings

0 stars 0 forks source link

ECDSA verify funciton allow 0x00 signatures as a parameter, which makes it possible to construct a two zeros bypass ECDSA signature check. #74

Closed c4-bot-6 closed 3 months ago

c4-bot-6 commented 3 months ago

Lines of code

https://github.com/code-423n4/2024-03-phala-network/blob/main/phala-blockchain/crates/pink/chain-extension/src/lib.rs#L217

Vulnerability details

Impact

This discovery is similar to the CVE-2022-21449 vulnerability in cryptography, which is an ECDSA signature bypass vulnerability.

A signature is composed of r and s, and signature = (r, s).

In an ECDSA signature, r and s are two parameters generated using the private key. To verify the signature, r' and s' need to be calculated using the public key and the message. If r' and s' are equal to r and s, the signature is valid.

r is generated using the following formula:

r = k * G

s is generated using the following formula:

s = k^-1 (H(m) + r * d)

k is the random number used in generating r. H(m) is the hash value of message m. d is the private key.

u1 = H(m) s^-1 (mod n) u2 = r s^-1 (mod n)

r' = u1 G + u2 Q

Q is the public key.

if r == 0 and s == 0 then u1 == u2 == 0

After extracting r and s from the signature, there is no range check, which makes it possible to use the (0, 0) signature to pass the entire signature verification operation.

Proof of Concept

In test_chain_extensions files add this function

#[test]
fn test_ecdsa() {
    use pink::chain_extension::signing as sig;
    use pink::chain_extension::SigType;

    pub type EcdsaSignature = [u8; 65];

    mock_all_ext();

    let privkey = sig::derive_sr25519_key(b"a spoon of salt");
    let privkey = &privkey[..32];
    let pubkey = sig::get_public_key(&privkey, SigType::Ecdsa);
    let message = b"hello world";
    //let signature = sig::sign(message, &privkey, SigType::Ecdsa);
    let signature: EcdsaSignature = [
    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
    0x00, 0x00, 0x00, 0x00, 0x00,];

    let pass = sig::verify(message, &pubkey, &signature, SigType::Ecdsa);
    println!("The verify function ran successfully");
    assert!(pass);
    let pass = sig::verify(b"Fake", &pubkey, &signature, SigType::Ecdsa);
    assert!(!pass);
}

I don't know why, but this poc failed in the end, it didn't bypass the signature check, but successfully ran the verify function, which indicates that the verify function accepts the 0x00 signature as a parameter, which is very likely to cause r,s == 0, So there is a potential risk of bypassing ECDSA check.

Tools Used

vscode

Recommended Mitigation Steps

Add signature verification, do not allow 0x00 as its parameter.

Assessed type

Other

c4-pre-sort commented 3 months ago

141345 marked the issue as sufficient quality report

141345 commented 3 months ago

seems invalid need to take a close look at POC

kvinwang commented 3 months ago

Since the PoC failed, it should not be a problem. In fact, the underlying implementation of verify would take care of that.

c4-sponsor commented 3 months ago

kvinwang (sponsor) disputed

c4-judge commented 3 months ago

OpenCoreCH marked the issue as unsatisfactory: Insufficient proof