Skip to content

Reject invalid API inputs before unsafe work #1223

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.

Sign up for GitHub

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

Closed
fegge wants to merge 1 commit into from
+181 −24
Closed

Reject invalid API inputs before unsafe work #1223

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions mldsa/mldsa_native.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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).
*
Expand Down
35 changes: 28 additions & 7 deletions mldsa/src/sign.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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);
Expand All @@ -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);

Expand Down Expand Up @@ -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;
}
Expand Down
19 changes: 10 additions & 9 deletions mldsa/src/sign.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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
Expand All @@ -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))
Expand Down Expand Up @@ -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).
*
Expand Down
97 changes: 95 additions & 2 deletions test/src/test_alloc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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];
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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);
Expand Down
41 changes: 41 additions & 0 deletions test/src/test_mldsa.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*/

#include <stddef.h>
#include <stdint.h>
#include <stdio.h>
#include <string.h>
#include "../notrandombytes/notrandombytes.h"
Expand All @@ -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
Expand Down Expand Up @@ -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];
Expand Down Expand Up @@ -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();
Expand Down