From 78edaf383ab994c6f143a2c7406d70530cb3e0ca Mon Sep 17 00:00:00 2001 From: Fredrik Dahlgren Date: Fri, 26 Jun 2026 16:37:24 +0200 Subject: [PATCH] Reject invalid API inputs before unsafe work --- mldsa/mldsa_native.h | 13 +++--- mldsa/src/sign.c | 35 ++++++++++++---- mldsa/src/sign.h | 19 +++++---- test/src/test_alloc.c | 97 ++++++++++++++++++++++++++++++++++++++++++- test/src/test_mldsa.c | 41 ++++++++++++++++++ 5 files changed, 181 insertions(+), 24 deletions(-) diff --git a/mldsa/mldsa_native.h b/mldsa/mldsa_native.h index 4310f2aae..50d558676 100644 --- a/mldsa/mldsa_native.h +++ b/mldsa/mldsa_native.h @@ -360,7 +360,7 @@ int MLD_API_NAMESPACE(signature_internal)( * @param[in] m Pointer to message to be signed. * @param mlen Length of message. * @param[in] ctx Pointer to context string. May be NULL if ctxlen == 0. - * @param ctxlen Length of context string. Should be <= 255. + * @param ctxlen Length of context string. Must be <= 255. * @param[in] sk Bit-packed secret key. * @param context Application context. Only present when * MLD_CONFIG_CONTEXT_PARAMETER is defined; type set by @@ -436,9 +436,10 @@ int MLD_API_NAMESPACE(signature_extmu)( * MLDSA{44,65,87}_BYTES + mlen bytes); can be equal to m. * @param[out] smlen Pointer to output length of signed message. * @param[in] m Pointer to message to be signed. - * @param mlen Length of message. - * @param[in] ctx Pointer to context string. - * @param ctxlen Length of context string. + * @param mlen Length of message. Must be <= SIZE_MAX - + * MLDSA{44,65,87}_BYTES. + * @param[in] ctx Pointer to context string. May be NULL if ctxlen == 0. + * @param ctxlen Length of context string. Must be <= 255. * @param[in] sk Bit-packed secret key. * @param context Application context. Only present when * MLD_CONFIG_CONTEXT_PARAMETER is defined; type set by @@ -835,8 +836,8 @@ int MLD_API_NAMESPACE(verify_pre_hash_shake256)( * @param[in] ph Pointer to pre-hashed message (ignored for pure * ML-DSA). * @param phlen Length of pre-hashed message (ignored for pure ML-DSA). - * @param[in] ctx Pointer to context string (may be NULL). - * @param ctxlen Length of context string. + * @param[in] ctx Pointer to context string. May be NULL if ctxlen == 0. + * @param ctxlen Length of context string. Must be <= 255. * @param hashalg Hash algorithm constant (MLD_PREHASH_NONE for pure * ML-DSA, or MLD_PREHASH_* for HashML-DSA). * diff --git a/mldsa/src/sign.c b/mldsa/src/sign.c index 276c5a5f5..86cb0cc85 100644 --- a/mldsa/src/sign.c +++ b/mldsa/src/sign.c @@ -915,6 +915,14 @@ int mld_sign_signature_internal(uint8_t sig[MLDSA_CRYPTO_BYTES], size_t *siglen, uint8_t *rho, *tr, *key, *mu, *rhoprime; uint16_t nonce = 0; const uint16_t nonce_limit = mld_get_max_signing_attempts(); + + if (externalmu && mlen != MLDSA_CRHBYTES) + { + *siglen = 0; + mld_memset(sig, 0, MLDSA_CRYPTO_BYTES); + return MLD_ERR_FAIL; + } + MLD_ALLOC(seedbuf, uint8_t, 2 * MLDSA_SEEDBYTES + MLDSA_TRBYTES + 2 * MLDSA_CRHBYTES, context); MLD_ALLOC(mat, mld_polymat, 1, context); @@ -1052,6 +1060,14 @@ int mld_sign_signature(uint8_t sig[MLDSA_CRYPTO_BYTES], size_t *siglen, { size_t pre_len; int ret; + + if (ctxlen > 255 || (ctx == NULL && ctxlen != 0)) + { + *siglen = 0; + mld_memset(sig, 0, MLDSA_CRYPTO_BYTES); + return MLD_ERR_FAIL; + } + MLD_ALLOC(pre, uint8_t, MLD_DOMAIN_SEPARATION_MAX_BYTES, context); MLD_ALLOC(rnd, uint8_t, MLDSA_RNDBYTES, context); @@ -1153,6 +1169,12 @@ int mld_sign(uint8_t *sm, size_t *smlen, const uint8_t *m, size_t mlen, int ret; size_t i; + if (mlen > SIZE_MAX - MLDSA_CRYPTO_BYTES) + { + *smlen = 0; + return MLD_ERR_FAIL; + } + for (i = 0; i < mlen; ++i) __loop__( assigns(i, object_whole(sm)) @@ -1187,6 +1209,11 @@ int mld_sign_verify_internal(const uint8_t *sig, size_t siglen, int ret, cmp; unsigned int i; + if (siglen != MLDSA_CRYPTO_BYTES || (externalmu && mlen != MLDSA_CRHBYTES)) + { + return MLD_ERR_FAIL; + } + MLD_ALLOC(buf, uint8_t, (MLDSA_K * MLDSA_POLYW1_PACKEDBYTES), context); MLD_ALLOC(mu, uint8_t, MLDSA_CRHBYTES, context); MLD_ALLOC(c, uint8_t, MLDSA_CTILDEBYTES, context); @@ -1204,12 +1231,6 @@ int mld_sign_verify_internal(const uint8_t *sig, size_t siglen, goto cleanup; } - if (siglen != MLDSA_CRYPTO_BYTES) - { - ret = MLD_ERR_FAIL; - goto cleanup; - } - mld_memcpy(c, sig, MLDSA_CTILDEBYTES); mld_polyvecl_unpack_z(z, sig + MLDSA_SIG_Z_OFFSET); @@ -1607,7 +1628,7 @@ size_t mld_prepare_domain_separation_prefix( uint8_t prefix[MLD_DOMAIN_SEPARATION_MAX_BYTES], const uint8_t *ph, size_t phlen, const uint8_t *ctx, size_t ctxlen, int hashalg) { - if (ctxlen > 255) + if (ctxlen > 255 || (ctx == NULL && ctxlen != 0)) { return 0; } diff --git a/mldsa/src/sign.h b/mldsa/src/sign.h index cc2ddc6dc..7e264b3de 100644 --- a/mldsa/src/sign.h +++ b/mldsa/src/sign.h @@ -253,7 +253,7 @@ __contract__( * @param[in] m Pointer to message to be signed. * @param mlen Length of message. * @param[in] ctx Pointer to context string. May be NULL if ctxlen == 0. - * @param ctxlen Length of context string. Should be <= 255. + * @param ctxlen Length of context string. Must be <= 255. * @param[in] sk Bit-packed secret key. * @param context Application context. Only present when * MLD_CONFIG_CONTEXT_PARAMETER is defined; type set by @@ -281,7 +281,7 @@ __contract__( requires(memory_no_alias(sig, MLDSA_CRYPTO_BYTES)) requires(memory_no_alias(siglen, sizeof(size_t))) requires(memory_no_alias(m, mlen)) - requires(ctxlen <= MLD_MAX_BUFFER_SIZE) + requires(ctxlen <= 255) requires(ctxlen == 0 || memory_no_alias(ctx, ctxlen)) requires(memory_no_alias(sk, MLDSA_CRYPTO_SECRETKEYBYTES)) assigns(memory_slice(sig, MLDSA_CRYPTO_BYTES)) @@ -342,9 +342,10 @@ __contract__( * MLDSA_CRYPTO_BYTES + mlen bytes); can be equal to m. * @param[out] smlen Pointer to output length of signed message. * @param[in] m Pointer to message to be signed. - * @param mlen Length of message. - * @param[in] ctx Pointer to context string. - * @param ctxlen Length of context string. + * @param mlen Length of message. Must be <= SIZE_MAX - + * MLDSA_CRYPTO_BYTES. + * @param[in] ctx Pointer to context string. May be NULL if ctxlen == 0. + * @param ctxlen Length of context string. Must be <= 255. * @param[in] sk Bit-packed secret key. * @param context Application context. Only present when * MLD_CONFIG_CONTEXT_PARAMETER is defined; type set by @@ -370,8 +371,8 @@ __contract__( requires(memory_no_alias(sm, MLDSA_CRYPTO_BYTES + mlen)) requires(memory_no_alias(smlen, sizeof(size_t))) requires(m == sm || memory_no_alias(m, mlen)) - requires(ctxlen <= MLD_MAX_BUFFER_SIZE) - requires(memory_no_alias(ctx, ctxlen)) + requires(ctxlen <= 255) + requires(ctxlen == 0 || memory_no_alias(ctx, ctxlen)) requires(memory_no_alias(sk, MLDSA_CRYPTO_SECRETKEYBYTES)) assigns(memory_slice(sm, MLDSA_CRYPTO_BYTES + mlen)) assigns(object_whole(smlen)) @@ -787,8 +788,8 @@ __contract__( * @param[in] ph Pointer to pre-hashed message (ignored for pure * ML-DSA). * @param phlen Length of pre-hashed message (ignored for pure ML-DSA). - * @param[in] ctx Pointer to context string (may be NULL). - * @param ctxlen Length of context string. + * @param[in] ctx Pointer to context string. May be NULL if ctxlen == 0. + * @param ctxlen Length of context string. Must be <= 255. * @param hashalg Hash algorithm constant (MLD_PREHASH_NONE for pure * ML-DSA, or MLD_PREHASH_* for HashML-DSA). * diff --git a/test/src/test_alloc.c b/test/src/test_alloc.c index fc3953605..31696ceb2 100644 --- a/test/src/test_alloc.c +++ b/test/src/test_alloc.c @@ -336,6 +336,29 @@ void custom_free(test_ctx_t *ctx, void *p, size_t sz, const char *file, } \ } while (0) +#define TEST_NO_ALLOC_FAILURE(test_name, call, expected_rc) \ + do \ + { \ + int rc; \ + ctx->high_mark = 0; \ + reset_all(ctx); \ + ctx->fail_on_counter = 0; \ + rc = call; \ + if (rc != (expected_rc)) \ + { \ + fprintf(stderr, "ERROR: %s failed with %d (expected %d)\n", test_name, \ + rc, expected_rc); \ + return 1; \ + } \ + if (ctx->alloc_counter != 0 || ctx->alloc_stack_top != 0 || \ + ctx->offset != 0) \ + { \ + fprintf(stderr, "ERROR: %s performed allocation before rejection\n", \ + test_name); \ + return 1; \ + } \ + } while (0) + /* Keygen tests */ #if !defined(MLD_CONFIG_NO_KEYPAIR_API) @@ -377,6 +400,49 @@ static int test_sign_alloc_failure(test_ctx_t *ctx) return 0; } +static int test_sign_invalid_context_preflight(test_ctx_t *ctx) +{ + uint8_t sig[CRYPTO_BYTES]; + uint8_t ctx_bytes[256] = {0}; + size_t siglen = CRYPTO_BYTES; + + TEST_NO_ALLOC_FAILURE( + "mld_signature invalid context length", + mld_signature(sig, &siglen, (const uint8_t *)TEST_VECTOR_MSG, + TEST_VECTOR_MSG_LEN, ctx_bytes, sizeof(ctx_bytes), + test_vector_sk, ctx), + MLD_ERR_FAIL); + return 0; +} + +static int test_sign_null_context_preflight(test_ctx_t *ctx) +{ + uint8_t sig[CRYPTO_BYTES]; + size_t siglen = CRYPTO_BYTES; + + TEST_NO_ALLOC_FAILURE( + "mld_signature null nonempty context", + mld_signature(sig, &siglen, (const uint8_t *)TEST_VECTOR_MSG, + TEST_VECTOR_MSG_LEN, NULL, 1, test_vector_sk, ctx), + MLD_ERR_FAIL); + return 0; +} + +static int test_signature_internal_invalid_externalmu_preflight(test_ctx_t *ctx) +{ + uint8_t sig[CRYPTO_BYTES]; + uint8_t mu[MLDSA_CRHBYTES] = {0}; + uint8_t rnd[MLDSA_RNDBYTES] = {0}; + size_t siglen = CRYPTO_BYTES; + + TEST_NO_ALLOC_FAILURE( + "mld_signature_internal invalid external mu length", + mld_signature_internal(sig, &siglen, mu, MLDSA_CRHBYTES - 1, NULL, 0, rnd, + test_vector_sk, 1, ctx), + MLD_ERR_FAIL); + return 0; +} + static int test_sign_combined_alloc_failure(test_ctx_t *ctx) { uint8_t sm[CRYPTO_BYTES + TEST_VECTOR_MSG_LEN]; @@ -420,6 +486,28 @@ static int test_signature_pre_hash_shake256_alloc_failure(test_ctx_t *ctx) #endif /* !MLD_CONFIG_NO_SIGN_API */ #if !defined(MLD_CONFIG_NO_VERIFY_API) +static int test_verify_invalid_siglen_preflight(test_ctx_t *ctx) +{ + TEST_NO_ALLOC_FAILURE( + "mld_verify_internal invalid signature length", + mld_verify_internal(test_vector_sig, CRYPTO_BYTES - 1, + (const uint8_t *)TEST_VECTOR_MSG, TEST_VECTOR_MSG_LEN, + (const uint8_t *)TEST_VECTOR_CTX, TEST_VECTOR_CTX_LEN, + test_vector_pk, 0, ctx), + MLD_ERR_FAIL); + return 0; +} + +static int test_verify_invalid_externalmu_preflight(test_ctx_t *ctx) +{ + TEST_NO_ALLOC_FAILURE( + "mld_verify_internal invalid external mu length", + mld_verify_internal(test_vector_sig_extmu, CRYPTO_BYTES, test_vector_mu, + MLDSA_CRHBYTES - 1, NULL, 0, test_vector_pk, 1, ctx), + MLD_ERR_FAIL); + return 0; +} + static int test_verify_alloc_failure(test_ctx_t *ctx) { TEST_ALLOC_FAILURE( @@ -517,16 +605,21 @@ int main(void) r |= test_pk_from_sk_alloc_failure(&ctx); #endif - /* Sign tests */ #if !defined(MLD_CONFIG_NO_SIGN_API) + /* Sign tests */ r |= test_sign_alloc_failure(&ctx); + r |= test_sign_invalid_context_preflight(&ctx); + r |= test_sign_null_context_preflight(&ctx); + r |= test_signature_internal_invalid_externalmu_preflight(&ctx); r |= test_sign_combined_alloc_failure(&ctx); r |= test_signature_extmu_alloc_failure(&ctx); r |= test_signature_pre_hash_shake256_alloc_failure(&ctx); #endif /* !MLD_CONFIG_NO_SIGN_API */ - /* Verify tests */ #if !defined(MLD_CONFIG_NO_VERIFY_API) + /* Verify tests */ + r |= test_verify_invalid_siglen_preflight(&ctx); + r |= test_verify_invalid_externalmu_preflight(&ctx); r |= test_verify_alloc_failure(&ctx); r |= test_verify_extmu_alloc_failure(&ctx); r |= test_verify_pre_hash_shake256_alloc_failure(&ctx); diff --git a/test/src/test_mldsa.c b/test/src/test_mldsa.c index 63a18d62a..2777d5cfb 100644 --- a/test/src/test_mldsa.c +++ b/test/src/test_mldsa.c @@ -4,6 +4,7 @@ */ #include +#include #include #include #include "../notrandombytes/notrandombytes.h" @@ -19,6 +20,8 @@ MLD_API_NAMESPACE(signature_pre_hash_shake256) #define crypto_sign_verify_pre_hash_shake256 \ MLD_API_NAMESPACE(verify_pre_hash_shake256) +#define crypto_sign_prepare_domain_separation_prefix \ + MLD_API_NAMESPACE(prepare_domain_separation_prefix) #define crypto_sign_pk_from_sk MLD_API_NAMESPACE(pk_from_sk) #ifndef NTESTS @@ -134,6 +137,42 @@ static int test_sign_unaligned(void) return test_sign_core(pk + 1, sk + 1, sm + 1, m + 1, m2 + 1, ctx + 1); } +static int test_sign_oversized_message_rejected(void) +{ + uint8_t pk[CRYPTO_PUBLICKEYBYTES]; + uint8_t sk[CRYPTO_SECRETKEYBYTES]; + uint8_t sm[CRYPTO_BYTES]; + uint8_t m[1] = {0}; + uint8_t ctx[CTXLEN]; + size_t smlen = CRYPTO_BYTES; + int rc; + + CHECK(crypto_sign_keypair(pk, sk) == 0); + CHECK(randombytes(ctx, sizeof(ctx)) == 0); + MLD_CT_TESTING_SECRET(ctx, sizeof(ctx)); + + rc = crypto_sign(sm, &smlen, m, SIZE_MAX, ctx, sizeof(ctx), sk); + MLD_CT_TESTING_DECLASSIFY(&rc, sizeof(rc)); + MLD_CT_TESTING_DECLASSIFY(&smlen, sizeof(smlen)); + + CHECK(rc == MLD_ERR_FAIL); + CHECK(smlen == 0); + + return 0; +} + +static int test_prepare_prefix_null_context_rejected(void) +{ + uint8_t prefix[MLD_DOMAIN_SEPARATION_MAX_BYTES]; + + CHECK(crypto_sign_prepare_domain_separation_prefix(prefix, NULL, 0, NULL, 0, + MLD_PREHASH_NONE) == 2); + CHECK(crypto_sign_prepare_domain_separation_prefix(prefix, NULL, 0, NULL, 1, + MLD_PREHASH_NONE) == 0); + + return 0; +} + static int test_sign_extmu(void) { uint8_t pk[CRYPTO_PUBLICKEYBYTES]; @@ -509,6 +548,8 @@ int main(void) !defined(MLD_CONFIG_NO_VERIFY_API) r |= test_sign(); r |= test_sign_unaligned(); + r |= test_sign_oversized_message_rejected(); + r |= test_prepare_prefix_null_context_rejected(); r |= test_wrong_pk(); r |= test_wrong_sig(); r |= test_wrong_ctx();