Skip to content

Fix SAKKE OOB write and correctness gap in sakke_hash_to_range #10442

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

Open
JeremiahM37 wants to merge 4 commits into
base: master
Choose a base branch
from
+107 −16
Open

Fix SAKKE OOB write and correctness gap in sakke_hash_to_range #10442

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f60f8cd
Clamp sakke_xor_in_v write to buffer length
JeremiahM37 May 8, 2026
065cde2
Fix sakke_xor_in_v write offset and read base
JeremiahM37 May 8, 2026
cfb4954
Reject ssvSz=0 in SAKKE public APIs
JeremiahM37 May 12, 2026
d75fdcb
Add SSV size coverage to sakke_op_test
JeremiahM37 May 12, 2026
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
56 changes: 40 additions & 16 deletions wolfcrypt/src/sakke.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6162,20 +6162,30 @@ static int sakke_calc_h_v(SakkeKey* key, enum wc_HashType hashType,
static void sakke_xor_in_v(const byte* v, word32 hashSz, byte* out, word32 idx,
word32 n)
{
int o;
word32 i;
word32 skip;
word32 off;
word32 len;

/* RFC 6508, Section 5.1: output is the low n octets of
* v_1||v_2||...||v_l (the concatenation of l hash outputs taken
* modulo 2^(n*8)). When n is not a multiple of hashSz, drop the
* leading 'skip' high bytes of the first hash output. */
skip = n % hashSz;
skip = (skip == 0) ? 0 : (hashSz - skip);

if (idx == 0) {
i = hashSz - (n % hashSz);
if (i == hashSz) {
i = 0;
}
off = 0;
Copy link
Copy Markdown
Contributor

@SparkiDev SparkiDev May 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for 'off' here.

len = hashSz - skip;
Copy link
Copy Markdown
Contributor

@SparkiDev SparkiDev May 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to assign to 'len' here - just put this into the xorbuf call.

xorbuf(out + off, v + skip, len);
}
else {
i = 0;
off = idx - skip;
len = n - off;
if (len > hashSz) {
len = hashSz;
}
xorbuf(out + off, v, len);
}
o = (int)i;
xorbuf(out + idx + i - o, v + i, hashSz - i);
}

/*
Expand Down Expand Up @@ -6682,8 +6692,9 @@ static int sakke_compute_point_r(SakkeKey* key, const byte* id, word16 idSz,
* @param [out] auth Authentication data.
* @param [out] authSz Size of authentication data in bytes.
* @return 0 on success.
* @return BAD_FUNC_ARG when key, ssv or encSz is NULL, ssvSz is to big or
* encSz is too small.
* @return BAD_FUNC_ARG when key, ssv or authSz is NULL, ssvSz is 0 or
* larger than the curve modulus byte length, or *authSz is too
* small when encapsulating.
* @return BAD_STATE_E when identity not set.
* @return LENGTH_ONLY_E when auth is NULL. authSz contains required size of
* auth in bytes.
Expand All @@ -6699,7 +6710,7 @@ int wc_MakeSakkeEncapsulatedSSV(SakkeKey* key, enum wc_HashType hashType,
word16 outSz = 0;
byte a[WC_MAX_DIGEST_SIZE];

if ((key == NULL) || (ssv == NULL) || (authSz == NULL)) {
if ((key == NULL) || (ssv == NULL) || (authSz == NULL) || (ssvSz == 0)) {
err = BAD_FUNC_ARG;
}
if ((err == 0) && (key->idSz == 0)) {
Expand All @@ -6718,7 +6729,14 @@ int wc_MakeSakkeEncapsulatedSSV(SakkeKey* key, enum wc_HashType hashType,
/* Uncompressed point */
outSz = (word16)(1 + 2 * n);

if ((auth != NULL) && (*authSz < outSz)) {
/* RFC 6508, Section 6.2.1, Step 1 places SSV in 0..2^n-1, so
* ssvSz must be <= n. Enforced on both the encapsulation and
* size-query paths so callers cannot probe authSz with an
* invalid ssvSz. */
if (ssvSz > n) {
err = BAD_FUNC_ARG;
}
else if ((auth != NULL) && (*authSz < outSz)) {
err = BAD_FUNC_ARG;
}
}
Expand Down Expand Up @@ -6813,7 +6831,7 @@ int wc_GenerateSakkeSSV(SakkeKey* key, WC_RNG* rng, byte* ssv, word16* ssvSz)
if (err == 0) {
n = (word16)WC_BITS_TO_BYTES(mp_count_bits(&key->params.prime));

if ((ssv != NULL) && (*ssvSz > n)) {
if ((ssv != NULL) && ((*ssvSz == 0) || (*ssvSz > n))) {
err = BAD_FUNC_ARG;
}
}
Expand Down Expand Up @@ -6857,7 +6875,8 @@ int wc_GenerateSakkeSSV(SakkeKey* key, WC_RNG* rng, byte* ssv, word16* ssvSz)
* @param [in] auth Authentication data.
* @param [in] authSz Size of authentication data in bytes.
* @return 0 on success.
* @return BAD_FUNC_ARG when key, ssv or auth is NULL.
* @return BAD_FUNC_ARG when key, ssv or auth is NULL, ssvSz is 0 or
* larger than the curve modulus byte length.
* @return BAD_STATE_E when RSK or identity not set.
* @return SAKKE_VERIFY_FAIL_E when calculated R doesn't match the encapsulated
* data's R.
Expand All @@ -6876,7 +6895,7 @@ int wc_DeriveSakkeSSV(SakkeKey* key, enum wc_HashType hashType, byte* ssv,
byte* test = NULL;
byte a[WC_MAX_DIGEST_SIZE] = {0};

if ((key == NULL) || (ssv == NULL) || (auth == NULL)) {
if ((key == NULL) || (ssv == NULL) || (auth == NULL) || (ssvSz == 0)) {
err = BAD_FUNC_ARG;
}
if ((err == 0) && (!key->rsk.set || (key->idSz == 0))) {
Expand All @@ -6895,6 +6914,11 @@ int wc_DeriveSakkeSSV(SakkeKey* key, enum wc_HashType hashType, byte* ssv,
if (authSz != 2 * n + 1) {
err = BAD_FUNC_ARG;
}
/* RFC 6508, Section 6.2.1: SSV is in 0..2^n-1, so ssvSz must
* be <= n. */
else if (ssvSz > n) {
err = BAD_FUNC_ARG;
}
}
if (err == 0) {
err = sakke_load_base_point(key);
Expand Down
67 changes: 67 additions & 0 deletions wolfcrypt/test/test.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58430,6 +58430,73 @@ static wc_test_ret_t sakke_op_test(SakkeKey* priv, SakkeKey* pub, WC_RNG* rng,
authSz);
if (ret != 0)
return WC_TEST_RET_ENC_EC(ret);

{
/* All three SSV entry points reject ssvSz=0 and ssvSz>n. */
word16 zeroSz = 0;
word16 hugeSz = 0xFFFF;

ret = wc_GenerateSakkeSSV(pub, rng, ssv, &zeroSz);
if (ret != WC_NO_ERR_TRACE(BAD_FUNC_ARG))
return WC_TEST_RET_ENC_NC;
zeroSz = 0;
ret = wc_MakeSakkeEncapsulatedSSV(pub, WC_HASH_TYPE_SHA256, ssv, zeroSz,
auth, &authSz);
if (ret != WC_NO_ERR_TRACE(BAD_FUNC_ARG))
return WC_TEST_RET_ENC_NC;
ret = wc_DeriveSakkeSSV(priv, WC_HASH_TYPE_SHA256, ssv, zeroSz, auth,
authSz);
if (ret != WC_NO_ERR_TRACE(BAD_FUNC_ARG))
return WC_TEST_RET_ENC_NC;
ret = wc_GenerateSakkeSSV(pub, rng, ssv, &hugeSz);
if (ret != WC_NO_ERR_TRACE(BAD_FUNC_ARG))
return WC_TEST_RET_ENC_NC;
hugeSz = 0xFFFF;
ret = wc_MakeSakkeEncapsulatedSSV(pub, WC_HASH_TYPE_SHA256, ssv, hugeSz,
auth, &authSz);
if (ret != WC_NO_ERR_TRACE(BAD_FUNC_ARG))
return WC_TEST_RET_ENC_NC;
ret = wc_DeriveSakkeSSV(priv, WC_HASH_TYPE_SHA256, ssv, hugeSz, auth,
authSz);
if (ret != WC_NO_ERR_TRACE(BAD_FUNC_ARG))
return WC_TEST_RET_ENC_NC;
}

{
/* Round-trip encap/derive across ssvSz shapes that exercise the
* multi-iteration paths in sakke_xor_in_v. SHA-256 hashSz=32:
* 48: 2 iterations, partial first block.
* 72: 3 iterations, exercises the len-clamp on the middle iter.
* 64: 2 iterations, exact-multiple (skip==0). */
static const word16 extSzs[3] = { 48, 72, 64 };
byte ssvExt[72];
byte ssvRef[72];
word16 extAuthSz;
int i;

for (i = 0; i < 3; i++) {
word16 extSz = extSzs[i];

ret = wc_RNG_GenerateBlock(rng, ssvRef, extSz);
if (ret != 0)
return WC_TEST_RET_ENC_EC(ret);
XMEMCPY(ssvExt, ssvRef, extSz);

extAuthSz = 257;
ret = wc_MakeSakkeEncapsulatedSSV(pub, WC_HASH_TYPE_SHA256, ssvExt,
extSz, auth, &extAuthSz);
if (ret != 0)
return WC_TEST_RET_ENC_EC(ret);

ret = wc_DeriveSakkeSSV(priv, WC_HASH_TYPE_SHA256, ssvExt, extSz,
auth, extAuthSz);
if (ret != 0)
return WC_TEST_RET_ENC_EC(ret);
if (XMEMCMP(ssvExt, ssvRef, extSz) != 0)
return WC_TEST_RET_ENC_NC;
}
}

return 0;
}

Expand Down
Loading