Skip to content

main: reject explicit parameters when decrypting - #437

Open
woahwhattheheck wants to merge 2 commits into
Tarsnap:masterfrom
woahwhattheheck:fix/dec-explicit-params
Open

woahwhattheheck wants to merge 2 commits into
Tarsnap:masterfrom
woahwhattheheck:fix/dec-explicit-params

Conversation

@woahwhattheheck

@woahwhattheheck woahwhattheheck commented Sep 6, 2026

Copy link
Copy Markdown

Fixes #436.

scrypt dec accepted --logN, -r and -p, passed them through to
scryptdec_file_prep(), and died on that function's assertion with SIGABRT
(exit 134, core dumped).

Explicit parameters are encryption-only — scrypt.1 already states that when
decrypting, the values always come from the encryption header. So this is
documented misuse; the bug is that it aborts instead of producing a diagnostic.
The existing checks in main() enforce only the all-or-none rule among the three
parameters, so an incomplete set already gives a clean error and exit 1, while a
complete set on dec reached the assertion.

This rejects the parameters in main(), alongside the checks that are already
there. Testing params.logN alone is sufficient because the all-or-none checks
above have already run, so at that point the three are either all zero or all
non-zero.

Before:

$ scrypt dec -f --logN 10 -r 1 -p 1 --passphrase dev:stdin-once good.enc out.txt
scrypt: lib/scryptenc/scryptenc.c:779: scryptdec_file_prep:
    Assertion `(P->logN == 0) && (P->r == 0) && (P->p == 0)' failed.
Aborted (core dumped)                                        # exit 134

After:

$ scrypt dec -f --logN 10 -r 1 -p 1 --passphrase dev:stdin-once good.enc out.txt
scrypt: --logN, -r and -p cannot be used when decrypting     # exit 1

The test goes into tests/09-explicit-params.sh next to the existing
partially-specified-parameter checks, rather than a new file, to keep the change
small and avoid touching Makefile.am.

I did not change scrypt info, which also ignores explicit parameters but exits 0
rather than crashing; whether it should warn seems like a separate question.

Checks. Build and the full test suite pass on macOS, Ubuntu/clang and
Ubuntu/gcc, with 09-explicit-params... SUCCESS! on all three:
https://github.com/woahwhattheheck/scrypt/actions/runs/34066975743

woahwhattheheck added a commit to woahwhattheheck/scrypt that referenced this pull request Sep 13, 2026
Replay the focused Tarsnap#437 fix onto current fork master while preserving the fork's newer regressions.
@tokenjunkielabs

Copy link
Copy Markdown

Maintainer review requested for #437 (fixes #436). The current carrier is OPEN and mergeable at 45d7c23e96392cc86ccb64d8e20b45fc315f5af7; the patch keeps the change to main.c plus the existing tests/09-explicit-params.sh regression. The report reproduces a SIGABRT / core dump from ordinary explicit decrypt parameters, while the patched path exits cleanly with a diagnostic. Cross-platform build/full-suite validation was previously published from the owner fork.

Please review the current head, approve the fork workflow if you want a fresh upstream run, and confirm bounty classification/payment for reporter woahwhattheheck under the crash-bug schedule. No additional source changes are pending maintainer feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

[bug bounty] "scrypt dec" aborts on an assertion when given explicit --logN/-r/-p

2 participants