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

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

Closed
guidovranken opened this issue Apr 12, 2021 · 1 comment
Closed

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

guidovranken opened this issue Apr 12, 2021 · 1 comment

Comments

@guidovranken
Copy link

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.

@mlindgren mlindgren added the investigate Needs further investigation label Apr 26, 2021
@samuel-lee-msft
Copy link
Contributor

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.

@mlindgren mlindgren removed the investigate Needs further investigation label Jun 30, 2021
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

3 participants