Skip to content

opendkim: heap-allocate keydata/tmpdata in dkimf_add_signrequest() (fixes #411) - #431

Merged
thegushi merged 1 commit into
trusteddomainproject:developfrom
thegushi:fix/issue-411-signreq-stack-overflow
Jul 22, 2026
Merged

opendkim: heap-allocate keydata/tmpdata in dkimf_add_signrequest() (fixes #411)#431
thegushi merged 1 commit into
trusteddomainproject:developfrom
thegushi:fix/issue-411-signreq-stack-overflow

Conversation

@thegushi

@thegushi thegushi commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes #411. dkimf_add_signrequest() stack-allocated two MAXBUFRSZ + 1 (65,537-byte) locals, keydata and tmpdata, totaling over 128 KB.

  • Harmless on Linux, where pthreads default to an 8 MB stack.
  • macOS ARM64 libmilter callback threads default to a 512 KB stack. Combined with libmilter's own frames plus the mlfi_eoh()dkimf_apply_signtable()dkimf_add_signrequest() call chain, an active KeyTable could overrun the stack (SIGBUS, ___chkstk_darwin in a crash trace) during completely ordinary sign-mode operation.

Both buffers are now malloc()'d, with free() added on every return path (including the two new allocation-failure paths this introduces). No behavior change otherwise — sizeof keydata/sizeof tmpdata references were all converted to explicit MAXBUFRSZ/MAXBUFRSZ + 1 since those would otherwise silently shrink to sizeof(char *) once the locals become pointers.

The smaller locals in the same function (domain, selector, signalgstr, err; ~3.3 KB total) are unchanged — they're not part of the reported overflow risk.

Test plan

  • CC=clang ./configure --enable-lua CFLAGS="-g -O0 -Wall -Wno-pointer-sign" + make: 0 errors/warnings touching this function.
  • make check: 207/207 passing, including t-sign-rs-tables and t-sign-rs-lua — the two paths the issue calls out as exercising dkimf_add_signrequest() (via dkimf_apply_signtable() and the Lua dkimf_xs_signfor() hook, respectively).
  • Not yet verified on actual macOS ARM64 hardware under the real 512 KB libmilter thread stack (the reported crash condition) — the project's build-test-macos GitHub Actions job (macos-latest, make check) should exercise this on the next push/PR run.

Fixes issue trusteddomainproject#411. dkimf_add_signrequest() stack-allocated two
MAXBUFRSZ+1 (65,537-byte) locals, keydata and tmpdata, totaling over
128 KB. Harmless on Linux (8 MB default pthread stack), but macOS
ARM64 libmilter callback threads default to a 512 KB stack; combined
with libmilter's own frames and the mlfi_eoh() -> dkimf_apply_signtable()
call chain, an active KeyTable could overrun the stack during ordinary
sign-mode operation.

Both buffers are now malloc'd, with free() added on every return path
(including the two new allocation-failure paths). No behavior change.

Verified with a clang -Wall build (0 warnings/errors touching this
function) and `make check` (207/207 passing, including
t-sign-rs-tables and t-sign-rs-lua, the paths the issue calls out as
exercising this code).
@thegushi
thegushi merged commit 19c431d into trusteddomainproject:develop Jul 22, 2026
2 checks 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