Skip to content

Fix latent bugs: 3DES OOB decrypt length, format-string usage, missing test include#2

Merged
brainx merged 3 commits into
mainfrom
cursor/fix-latent-bugs-92e7
Jun 17, 2026
Merged

Fix latent bugs: 3DES OOB decrypt length, format-string usage, missing test include#2
brainx merged 3 commits into
mainfrom
cursor/fix-latent-bugs-92e7

Conversation

@brainx

@brainx brainx commented Jun 17, 2026

Copy link
Copy Markdown
Owner

Summary

Three real bugs found while auditing the modernized/vintage C sources. All are low-risk, localized fixes; no behavior change on the happy path.

1. Out-of-bounds length in pgp_3desdecrypt (Src/pgpget.c)

The 3DES decrypt passed in->length - 10 + mdc as the ciphertext body length:

des_ede3_cfb64_encrypt(in->data + 10 + mdc, out->data, in->length - 10 + mdc, ...)

When mdc == 1 (OpenPGP MDC packets, used by modern GnuPG/PGP) this reads 2 bytes past the valid ciphertext (in->data + 10 + mdc only holds in->length - 10 - mdc bytes) and writes 2 bytes past out->length. Evidence it's wrong:

  • Every sibling cipher uses - mdc: IDEA (- 10 - mdc), CAST5 (- 10 - mdc), Blowfish (- 10 - mdc), AES (- 18 - mdc). 3DES is the lone outlier.
  • The MDC layout written by pgp_3desencrypt places the body at offset 10 + mdc with the decrypt buffer sized in->length - 10 - mdc, so - mdc is the only consistent length.

Fixed to in->length - 10 - mdc. Mitigated in practice by the 128-byte buffer over-allocation (so sanitizers don't trip), but a genuine out-of-bounds access in crypto code.

2. Format-string usage flagged by -Wformat-security (Src/rem.c, Src/menustats.c)

fprintf(mbox, line) in logmail() and printw(ALLPINGERSURL) in the stats menu passed runtime/config-derived strings directly as format strings. Changed to fprintf(mbox, "%s", line) and printw("%s", ALLPINGERSURL). These were the only two -Wformat-security hits in the tree.

3. Missing <string.h> in date-parse test (Src/tests/test-parse_yearmonthday.c)

The test calls memset() without including <string.h>, which is a hard compile error under modern clang/gcc (implicit function declaration). Added the include.

Verification

  • ✅ Full build clean: mixmaster + mixremailer compile, 0 remaining -Wformat-security warnings across the tree.
  • cc -DHAVE_SETENV Src/tests/test-parse_yearmonthday.c && ./a.outOK. (previously failed to compile without an -include workaround).
  • ✅ PGP conventional 3DES + MDC round-trip via the real pgp_encrypt/pgp_decrypt recovers the plaintext (err=0), confirming the decrypt-length fix preserves correctness:

pgp_3des_roundtrip.log

  • ✅ End-to-end remailer flow (client encrypt → remailer decrypt → deliver) still recovers plaintext after the changes — no regression.

To show artifacts inline, enable in settings.

Open in Web Open in Cursor 

brainx and others added 3 commits June 17, 2026 14:56
The 3DES decrypt passed in->length - 10 + mdc as the ciphertext body
length, reading and writing 2 bytes past the intended region when mdc=1
(OpenPGP MDC packets). Every sibling cipher (IDEA, CAST5, Blowfish, AES)
uses - mdc, and the MDC ciphertext layout produced by pgp_3desencrypt
makes - mdc the correct length. Mitigated in practice by buffer slack,
but a genuine out-of-bounds access.

Co-authored-by: BrainX <brainx@users.noreply.github.com>
fprintf(mbox, line) in logmail() and printw(ALLPINGERSURL) in the stats
menu passed runtime/config-derived strings as format strings. Pass them
as %s arguments instead.

Co-authored-by: BrainX <brainx@users.noreply.github.com>
test-parse_yearmonthday.c calls memset() without including <string.h>,
which fails to compile as an error under modern clang/gcc.

Co-authored-by: BrainX <brainx@users.noreply.github.com>
@brainx
brainx marked this pull request as ready for review June 17, 2026 15:05
@brainx
brainx merged commit 57b8900 into main Jun 17, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant