skey test failure from sph_dec32le_aligned UB
Nobody has claimed this yet.
- Dominant language
- C
- Stars
- 13.6k
- Forks
- 2.6k
- PR merge metrics
- No merged PRs in 30d
Description
Hi!
Downstream in Gentoo, toralf reported a test failure for johntheripper-jumbo (at commit b27f951a8e191210685c8421c90ca610cdd39dce; I've checked subsequent commits and they shouldn't affect this) looking like:
Testing: skey, S/Key [MD4/MD5/SHA1/RMD160 32/64]... FAILED (cmp_all(1))
[...]
1 out of 758 tests have FAILED
* ERROR: app-crypt/johntheripper-jumbo-1.9.0_p20250703::gentoo failed (test phase):
* (no error message)
toralf hit this with trunk GCC (so to-be-16) with -O2 -pipe -march=native, where -march=native is whatever it returns (vagueness for a reason, see below) on a AMD Ryzen 9 5950X.
I couldn't reproduce that with -march=znver2 or -march=znver3 (or -march=native) on my 3950X but decided not to worry about that, because I could reproduce with -O3 -flto and just debugged that instead. It also passes with -fno-strict-aliasing.
Running the failing test under Valgrind:
/var/tmp/portage/app-crypt/johntheripper-jumbo-1.9.0_p20250703/work/john-b27f951a8e191210685c8421c90ca610cdd39dce/test # valgrind ../run/john --config=john.conf --test --format=skey==164563== Memcheck, a memory error detector
==164563== Copyright (C) 2002-2024, and GNU GPL'd, by Julian Seward et al.
==164563== Using Valgrind-3.26.0.GIT and LibVEX; rerun with -h for copyright info
==164563== Command: ../run/john --config=john.conf --test --format=skey
==164563==
--164563-- Warning: <14704> DW_TAG_subprogram with no DW_AT_name and no DW_AT_specification or DW_AT_abstract_origin in /var/tmp/portage/app-crypt/johntheripper-jumbo-1.9.0_p20250703/work/john-b27f951a8e191210685c8421c90ca610cdd39dce/run/john
--164563-- Warning: zero subprog, missing DW_AT_abstract_origin in DW_TAG_inlined_subroutine in /var/tmp/portage/app-crypt/johntheripper-jumbo-1.9.0_p20250703/work/john-b27f951a8e191210685c8421c90ca610cdd39dce/run/john
Benchmarking: skey, S/Key [MD4/MD5/SHA1/RMD160 32/64]... ==164563== Conditional jump or move depends on uninitialised value(s)
==164563== at 0x44C6C0A: is_key_right.part.0 (formats.c:596)
==164563== by 0x44CCFE9: UnknownInlinedFun (formats.c:1312)
==164563== by 0x44CCFE9: UnknownInlinedFun (formats.c:1329)
==164563== by 0x44CCFE9: fmt_self_test (formats.c:2079)
==164563== by 0x44C3673: benchmark_all (bench.c:882)
==164563== by 0x44DB627: UnknownInlinedFun (john.c:1684)
==164563== by 0x44DB627: main (john.c:2110)
==164563==
FAILED (cmp_all(1))
I won't bore you with the snaking through, but it ended up being that formats.o and SKEY_fmt_plug.o were entirely innocent and the issue was in ripemd.o. In the end, it was ripemd160_round, specifically its sph_dec32le_aligned call from sph_types.h at https://github.com/openwall/john/blob/cb0c337e7ffbe235f55c5c7a57fcab5010d0c0ba/src/sph_types.h#L1621-L1625.
sph_dec32le_aligned breaks C aliasing rules by reading unsigned char* as sph_u32*. You can alias anything with unsigned char*, but not the other way around.
Marking sph_u32 (and friends) with __attribute__((may_alias)) fixes the issue for me, though that's not standard C.
(Returning to toralf's environment: given the specifics of the problem in the end and where the aliasing happens (in the same TU), I'm reasonably confident it's the same problem that he hit given he wasn't using LTO.)
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with sph_types.h at the reported sph_dec32le_aligned definition and ripemd160_round in ripemd.c, then reproduce the failure with the skey format test under the reported optimization settings. Trace the aliasing-related uninitialized value and verify that the skey self-test passes without the reported failure.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- c
- Domain
- cryptography
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100