Skip to content

Check SLHDSA_SHA2_128S_sign return value in SLH-DSA sign#14

Open
mohammadmseet-hue wants to merge 1 commit into
tink-crypto:mainfrom
mohammadmseet-hue:fix/slh-dsa-sign-missing-return-check
Open

Check SLHDSA_SHA2_128S_sign return value in SLH-DSA sign#14
mohammadmseet-hue wants to merge 1 commit into
tink-crypto:mainfrom
mohammadmseet-hue:fix/slh-dsa-sign-missing-return-check

Conversation

@mohammadmseet-hue

Copy link
Copy Markdown

Summary

SlhDsaSignBoringSsl::Sign (slh_dsa_sign_boringssl.cc:79) does not check the return value of SLHDSA_SHA2_128S_sign, which returns int (0 on failure, 1 on success per BoringSSL's include/openssl/slhdsa.h).

If the signing operation fails, the function returns a buffer of uninitialized memory as the "signature" — the buffer was allocated with ResizeStringUninitialized on line 74.

This is inconsistent with:

  • ML-DSA sign (ml_dsa_sign_boringssl.cc:126-133) — correctly checks if (!MLDSA65_sign(...)) { return InternalError; }
  • Commit 0f1fe03 — fixed the same pattern in EciesHkdfX25519SendKemBoringSsl::GenerateKey

Impact

If SLHDSA_SHA2_128S_sign returns 0 (e.g., due to RNG failure), Tink returns an uninitialized memory buffer as a valid StatusOr<string>. This is:

  1. Potential uninitialized memory disclosure (the "signature" contains heap garbage)
  2. Silent signing failure (caller thinks signing succeeded)

Fix

Follow the ML-DSA sign pattern: capture CallWithCoreDumpProtection return status, check SLHDSA_SHA2_128S_sign return value inside the lambda, propagate errors.

Test plan

  • Existing SLH-DSA sign tests continue to pass (signing with valid keys still works)
  • The failure path (RNG failure) cannot be easily triggered in a unit test without mocking BoringSSL internals

@google-cla

google-cla Bot commented Mar 30, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@mohammadmseet-hue
mohammadmseet-hue force-pushed the fix/slh-dsa-sign-missing-return-check branch from 598a86f to 7ad12a8 Compare March 30, 2026 12:04
The refactored Sign() dispatches through a SignFunc (int (*)(...)) function
pointer but ignores its return value, so a signing failure would return an
uninitialized/partial signature buffer as success. Propagate an InternalError
when the sign function returns 0.
@mohammadmseet-hue
mohammadmseet-hue force-pushed the fix/slh-dsa-sign-missing-return-check branch from 7ad12a8 to 9c61987 Compare July 24, 2026 13:52
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

Successfully merging this pull request may close these issues.

1 participant