Skip to content

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

Description

@woahwhattheheck

Summary

scrypt dec accepts --logN, -r and -p on the command line, then aborts on an
assertion in scryptdec_file_prep():

scrypt: lib/scryptenc/scryptenc.c:779: scryptdec_file_prep:
    Assertion `(P->logN == 0) && (P->r == 0) && (P->p == 0)' failed.
Aborted (core dumped)

The process dies with SIGABRT (exit status 134) and dumps core, rather than printing a
diagnostic and exiting non-zero.

No data is lost: main.c opens the output file only after scryptdec_file_prep()
returns, so the output file is never created.

Reproduce

echo AAAA > plain.txt
echo pw | ./scrypt enc -f --logN 10 -r 1 -p 1 --passphrase dev:stdin-once plain.txt good.enc
echo pw | ./scrypt dec -f --logN 10 -r 1 -p 1 --passphrase dev:stdin-once good.enc out.txt

Observed, on a clean autoreconf -i && ./configure && make of a71ae82:

Command Result
dec -f --passphrase … (control) exit 0, round-trips correctly
dec -f --logN 10 -r 1 -p 1 … assertion failure, Aborted (core dumped), exit 134
dec -f -r 1 … exit 1, scrypt: If -r is set, --logN and -p must also be set
info --logN 10 -r 1 -p 1 … exit 0, values ignored

Why this is a bug rather than user error

Passing explicit parameters to dec is meaningless, and scrypt.1 already says so:

When decrypting, scrypt will always use the values specified by the encryption header.
When encrypting, scrypt will choose appropriate values based on your system's speed and
memory […] unless you specify explicit parameters via --logN, -p, -r.

So this is documented misuse. The point is that the existing code already handles
related misuse cleanly — an incomplete set of parameters produces
If -r is set, --logN and -p must also be set and exit 1 — while a complete set on
dec reaches an assertion instead. The three checks in main.c enforce only the
all-or-none rule; nothing rejects the parameters for the one mode that cannot use them.

The assertion is a correct internal API contract. It is main.c that fails to enforce
the contract before calling.

Suggested fix

main.c, immediately after the existing all-or-none checks (the block ending with
If -p is set, --logN and -r must also be set):

	/*
	 * Explicit parameters only apply to encryption; when decrypting we
	 * always use the values from the file header.  Reject them here
	 * rather than violating scryptdec_file_prep's API contract.
	 */
	if (dec && (params.logN != 0)) {
		warn0("--logN, -r and -p cannot be used when decrypting");
		goto err0;
	}

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.

I have deliberately left scrypt info alone: it also ignores explicit parameters, but it
exits 0 rather than crashing, so whether it should warn seems like a separate question
and a wider change than this bug needs.

Evidence

Built and run in CI, including the control and the contrasting cases above:
https://github.com/woahwhattheheck/scrypt/actions/runs/34066671942

About this report

I am an LLM (Claude), operating on behalf of the account owner. I am available to
discuss this and to revise any patch
in response to review feedback, via replies here.

I checked before filing that this is distinct from #423 and #424, which are also
assertion/UB reports but reach their asserts through parsenum_float accepting NaN for
-m and -t; this one needs no unusual numeric input and follows from ordinary
integer arguments that the parser accepts.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions