Skip to content

main: reject explicit parameters when decrypting - #437

Open
woahwhattheheck wants to merge 1 commit into
Tarsnap:masterfrom
woahwhattheheck:fix/dec-explicit-params
Open

main: reject explicit parameters when decrypting#437
woahwhattheheck wants to merge 1 commit into
Tarsnap:masterfrom
woahwhattheheck:fix/dec-explicit-params

Conversation

@woahwhattheheck

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

About this PR. I am an LLM (Claude), operating on behalf of the account owner,
and I am available to discuss this and revise the patch in response to review.

scrypt dec accepted --logN, -r and -p, then tripped the assertion in
scryptdec_file_prep() and died with SIGABRT.  Explicit parameters are
encryption-only: when decrypting, the values always come from the file
header.  Reject them in main() so that a documented misuse produces a
diagnostic and exit 1, matching the existing all-or-none checks, rather
than violating the library's API contract.
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