microsoft / SymCrypt

Cryptographic library
MIT License
660 stars 68 forks source link

OSS-Fuzz #33189: ECDSA signing produces invalid signature #15

Closed guidovranken closed 3 years ago

guidovranken commented 3 years ago

https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=33189

Curve: secp384r1 Nonce = 1 Msg = Curve base point X Private key = Curve order - 1

#include <stdlib.h>
#include <symcrypt.h>
#include <symcrypt_low_level.h>
#include <stdint.h>
#include <stdio.h>

#define CF_CHECK_EQ(expr, res) if ( (expr) != (res) ) { goto end; }
#define CF_CHECK_NE(expr, res) if ( (expr) == (res) ) { goto end; }

void SymCryptFatal(UINT32 fatalCode) {
    (void)fatalCode;

    abort();
}
PVOID SymCryptCallbackAlloc( SIZE_T nBytes ) {
    return malloc(nBytes);
}

VOID SymCryptCallbackFree( VOID * pMem ) {
    free(pMem);
}
SYMCRYPT_CPU_FEATURES
SymCryptCpuFeaturesNeverPresent(void) {
    return 0;
}
SYMCRYPT_ERROR SymCryptCallbackRandom(PBYTE out, SIZE_T size) {
    for (size_t i = 0; i < size; i++) {
        out[i] = rand();
    }
    return SYMCRYPT_NO_ERROR;
}

int main(void)
{
    const uint8_t msg[] = {0xaa, 0x87, 0xca, 0x22, 0xbe, 0x8b, 0x05, 0x37, 0x8e, 0xb1, 0xc7, 0x1e, 0xf3, 0x20, 0xad, 0x74,
 0x6e, 0x1d, 0x3b, 0x62, 0x8b, 0xa7, 0x9b, 0x98, 0x59, 0xf7, 0x41, 0xe0, 0x82, 0x54, 0x2a, 0x38,
 0x55, 0x02, 0xf2, 0x5d, 0xbf, 0x55, 0x29, 0x6c, 0x3a, 0x54, 0x5e, 0x38, 0x72, 0x76, 0x0a, 0xb7};

    SYMCRYPT_ECURVE* curve = NULL;
    SYMCRYPT_ECKEY* key = NULL;
    SYMCRYPT_INT* nonce = NULL;

    CF_CHECK_NE(curve = SymCryptEcurveAllocate(SymCryptEcurveParamsNistP384, 0), NULL);
    CF_CHECK_NE(key = SymCryptEckeyAllocate(curve), NULL);
    CF_CHECK_NE(nonce = SymCryptIntAllocate(SymCryptEcurveDigitsofScalarMultiplier(curve)), NULL);
    {
        const uint8_t nonce_bytes[] = {0x01};
        CF_CHECK_EQ(SymCryptIntSetValue(nonce_bytes, sizeof(nonce_bytes), SYMCRYPT_NUMBER_FORMAT_MSB_FIRST, nonce), SYMCRYPT_NO_ERROR);
    }

    {
        const uint8_t priv_bytes[] = {
            0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xc7, 0x63, 0x4d, 0x81, 0xf4, 0x37, 0x2d, 0xdf, 0x58, 0x1a, 0x0d, 0xb2, 0x48, 0xb0, 0xa7, 0x7a, 0xec, 0xec, 0x19, 0x6a, 0xcc, 0xc5, 0x29, 0x72};
        CF_CHECK_EQ(SymCryptEckeySetValue(
                priv_bytes, sizeof(priv_bytes),
                NULL, 0,
                SYMCRYPT_NUMBER_FORMAT_MSB_FIRST, SYMCRYPT_ECPOINT_FORMAT_XY,
                0, key), SYMCRYPT_NO_ERROR);
    }

    {
        const size_t sigHalfSize = SymCryptEcurveSizeofScalarMultiplier(curve);
        const size_t sigSize = sigHalfSize * 2;
        uint8_t sig_bytes[sigSize];

        CF_CHECK_EQ(SymCryptEcDsaSignEx(
                    key,
                    msg,
                    sizeof(msg),
                    nonce,
                    SYMCRYPT_NUMBER_FORMAT_MSB_FIRST,
                    0,
                    sig_bytes,
                    sigSize), SYMCRYPT_NO_ERROR);

        const size_t pubSize = SymCryptEckeySizeofPublicKey(key, SYMCRYPT_ECPOINT_FORMAT_XY);

        const size_t pubHalfSize = pubSize / 2;
        uint8_t pub_bytes[pubSize];

        CF_CHECK_EQ(SymCryptEckeyGetValue(
                    key,
                    NULL, 0,
                    pub_bytes, pubSize,
                    SYMCRYPT_NUMBER_FORMAT_MSB_FIRST, SYMCRYPT_ECPOINT_FORMAT_XY,
                    0), SYMCRYPT_NO_ERROR);

        printf("Pub X:\n");
        for (size_t i = 0; i < pubHalfSize; i++) {
            printf("%02X ", pub_bytes[i]);
        }
        printf("\n");

        printf("Pub Y:\n");
        for (size_t i = 0; i < pubHalfSize; i++) {
            printf("%02X ", pub_bytes[pubHalfSize+i]);
        }
        printf("\n");

        printf("Sig R:\n");
        for (size_t i = 0; i < sigHalfSize; i++) {
            printf("%02X ", sig_bytes[i]);
        }
        printf("\n");

        printf("Sig S:\n");
        for (size_t i = 0; i < sigHalfSize; i++) {
            printf("%02X ", sig_bytes[sigHalfSize+i]);
        }
        printf("\n");
    }

end:
    if ( key ) {
        /* noret */ SymCryptEckeyFree(key);
    }
    if ( curve ) {
        /* noret */ SymCryptEcurveFree(curve);
    }
    if ( nonce ) {
        /* noret */ SymCryptIntFree(nonce);
    }

    return 0;
}

Output:

Pub X:
AA 87 CA 22 BE 8B 05 37 8E B1 C7 1E F3 20 AD 74 6E 1D 3B 62 8B A7 9B 98 59 F7 41 E0 82 54 2A 38 55 02 F2 5D BF 55 29 6C 3A 54 5E 38 72 76 0A B7 
Pub Y:
C9 E8 21 B5 69 D9 D3 90 A2 61 67 40 6D 6D 23 D6 07 0B E2 42 D7 65 EB 83 16 25 CE EC 4A 0F 47 3E F5 9F 4E 30 E2 81 7E 62 85 BC E2 84 6F 15 F1 A0 
Sig R:
AA 87 CA 22 BE 8B 05 37 8E B1 C7 1E F3 20 AD 74 6E 1D 3B 62 8B A7 9B 98 59 F7 41 E0 82 54 2A 38 55 02 F2 5D BF 55 29 6C 3A 54 5E 38 72 76 0A B7 
Sig S:
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 

S = 0, which makes the signature invalid. SymCryptEcDsaSignEx should fail instead.

samuel-lee-msft commented 3 years ago

Thanks for reporting this! 👍

As with #13 SymCryptEcDsaSignEx is not really intended for use by callers who are generating their own piK and generating signatures - it's intended for use for testing the signing code with known answer tests.

I went ahead with a small fix to return SYMCRYPT_INVALID_ARGUMENT in the case when a non-NULL piK generates a 0 signature anyway, in case there is some use case where this API is useful for callers beyond performing known answer tests.