fix: fail closed on a failed randomness draw, and bound the inputs that size buffers

[buzzromain]

Two related gaps in how SSKR handles values it does not control: the randomness it is given, and the lengths it is told.

Share generation could not tell a failed randomness draw from a successful one

Generation draws randomness three times — the 16-bit share-set identifier, the threshold - 2 leading shares of each split, and the random half of the integrity digest. None of the three was checked, and none of them could be: the generator parameter is typed unsigned char *(*)(uint8_t *, size_t), which has no channel for a failure, and the BOLOS entry point it was given, cx_rng(), calls cx_rng_no_throw(), which returns void. Every function in lcx_rng.h is the same shape — void, or the buffer it was handed. There was no error being ignored, which is why nothing looked wrong.

What a failed draw produced was worse than an error. The identifier stayed at its zero initialiser, the coefficients and the digest padding kept whatever the previous stack frame had left in those buffers, and the shares built on them still recombined to the correct secret. Generation reported success and the user wrote down a backup that works — while the secrecy Shamir exists to provide was gone. Two shares of a 3-of-5 set drawn from stack residue are not two unknowns. Nothing on the device, in the return value or in a trace said so.

Reproduced with a generator that writes nothing, which is exactly what cx_rng() leaves behind on a failed draw:

sskr_generate_shards returned : 3        <- success
shard_len                     : 21
first shard bytes             : 00000001005cff748c6b7643a173f2d3fba243da90
                                ^^^^ identifier = 0x0000
shares from a failed RNG still recombine: YES

Consecutive runs give different share bytes from identical inputs, which is what confirms the values come from uninitialised stack rather than from any defined source.

cx_get_random_bytes() (os_random.h) is the same TRNG behind a syscall that returns cx_err_t, and it is available on every SDK this application targets, nanos included. seed_sskr.c now goes through it, the generator type carries a bool, and all three draw sites stop on a failed draw — erasing the output buffer, the digest and the coordinate arrays on the way out, the same cleanup the interpolation-failure path beside them already does.

sskr_generate_shards() needed no change: it already sets *shard_len to 0 up front, propagates a negative code and erases the whole output buffer, so the new SSKR_ERROR_RNG_FAILURE and SSS_ERROR_RNG_FAILURE travel out on the path the other families already use.

After the change the same reproduction gives -19, shard_len = 0, and no shares — stopping at the first failed draw rather than continuing through the split.

Four inputs that reached an index or a buffer size unbounded

bolos_ux_sskr_hex_check() and the CBOR initial byte. That byte is three bits of major type and five of additional information (RFC 8949 section 3). The major type was tested; the additional information was not. Values 25 to 31 are reserved — two, four and eight-byte lengths, one reserved value and indefinite length — and none of them carries a length this function can act on, yet all seven were accepted. So was a declared length that disagreed with the frame it arrived in.

Nothing broke, because the refusal happened one layer away: bolos_ux_sskr_entry_header_update() leaves the share count at zero for a reserved value and the zero-count bound refuses the frame, while bolos_ux_sskr_combine() bounds the declared length itself. Both are real and both stay. What they are not is this function — and this is the one the entry paths call to ask whether what was typed is a share, so a caller arriving with a count of its own was handed a frame nothing had checked the shape of. It now refuses reserved additional information, and requires the declared length, the header and the CRC-32 to add up to the stride.

bolos_ux_sskr_size_get() and bip39_type. *share_len came back as bip39_type * 4 / 3 + 5 for any bip39_type at all, and both callers size a buffer from it — above 190 that expression also wraps the uint8_t holding it. Every comparable entry point in this file bounds its parameters; this one did not. It now accepts 12, 18 and 24, and leaves *share_len at zero otherwise, which is the contract the rest of the file already keeps.

bolos_ux_bip39_mnemonic_to_seed() and mnemonic_length. The memcpy into mnemonic_hash[257] fits by an arithmetic coincidence spread over three files — WORDS_BUFFER_MAX_SIZE_B is also 257, BIP39_MNEMONIC_MAX_LENGTH is 216 — that nothing holds together. The function that owns the buffer is the one that can.

key_press_callback() and textToEnter. What keeps the write inside the array is the keyboard mask: get_keyboard_mask() disables every letter once no wordlist entry extends the prefix, and the longest BIP-39 word is eight characters. That is a guard computed elsewhere, for another purpose, that this write happens to benefit from.

Also bounded, though no path can currently reach it: display_check_result_page() indexes a five-element row with 1 + tool_type * 2 + seed_match, and TOOL_TYPE_BIP85 would give 5 or 6. The BIP85 flow goes to display_generic_review() and never asks for a verdict.

Tests

tests/unit/tests/sskr_generate_rng_failure.c walks the whole sequence of draws a 3-of-5 split makes, failing one at a time, and holds that each comes back with a negative code, *shard_len at 0 and an output buffer erased in full.

tests/unit/tests/sskr_input_validation_guards.c calls the two SSKR entry points directly, with a count and a bip39_type of its own — the shape of caller the side effects above do not protect.

Both files end on a control: the same generator never failing still produces a set, and a well-formed frame is still accepted. Without those, either file would also pass against a function that refused unconditionally.

Each guard was removed again in a scratch tree to confirm the tests actually fail without it — three of four cases in the first file, three of six in the second, with the controls staying green in both.

Two changes worth flagging in review

sskr.h and sss/sss.h did not include <stdbool.h>. They compiled only because every file using them had already pulled it in; the bool in these prototypes is what made that visible.

test_hex_check_accepts_the_smallest_checkable_stride built its stride-8 control frame with an initial byte of 0x55, declaring 21 bytes of payload in a frame carrying none — which only passed because the declared length was never read. It is 0x40 now, a zero-length byte string: self-consistent, and still not a shard. The division of responsibility that test records, between framing here and shard length in bolos_ux_sskr_combine(), is unchanged.

tests/unit now builds with src/ on the include path, the way APP_SOURCE_PATH puts it there for the device build, so an application source can include "constants.h" as it already does elsewhere.

fuzzing/extra/host_syscalls.c gains a stand-in for cx_get_random_bytes() beside the one it already has for cx_rng_no_throw(). Without it libseedparsers.a carries an undefined reference and every fuzz target fails to link.

The last commit is unrelated to the rest: src/bagl/nanox_enter_phrase.c is already failing clang-format --dry-run --Werror on bip85, so the lint job is red before this branch touches anything. It is pure line wrapping with no semantic change, fixed here only so this can be read against a green CI — happy to split it out.

Verification

• six declared targets build, zero application-source warnings on each
• unit suite 77/77
• functional suite 11/11 on stax and flex, 2 passed / 9 skipped on nanox and nanos+
• fuzz targets link, and cx_get_random_bytes resolves inside libseedparsers.a
• ASan + UBSan clean over the changed code at -O0 and -O2
• each new guard falsified by removing it and watching the matching test go red