On 4/3/26 11:05 AM, Omar Polo wrote:
Hello, Renaud Allard <[email protected]> wrote:make(1) aborts on malformed input containing nested archive specs with variable references. A 36-byte input reproduces the crash on all architectures with the stock system binary, no special flags needed: printf 'h:)C)($$\017*)))(($$$$\t))))()\n' > crash.mk /usr/bin/make -nf crash.mk Abort trap (core dumped) Confirmed on OpenBSD 7.9/amd64, 7.9/i386, and 7.8/arm64. Root cause: Arch_ParseArchive() in arch.c uses a static buffer shared across recursive calls. When an archive member contains $-variables, parse_archive() recurses into Arch_ParseArchive() (line 313), which calls Buf_Reinit() on the same static buffer, corrupting the outer call's state. malloc detects the corruption and calls abort().have to admit that make internals are a bit out of my comfort zone, but as a simple fix your diff makes sense to me. since we're going to recurse, we could also as well just initialize the buffer once and reuse it thrugh the recursive calls, but again, I'm a bit out of my comfort zone here so won't propose to do so. I'm cc'ing espie since he knows surely better.
Yes, I forgot to cc him in my mail.But let's be honest, this bug has been unseen for about 6 years. So that would mean, no port uses ar nesting and nothing in base either. So, the question is: should we really support ar nesting? Less code, less attack and bug surface.
Introduced in arch.c r1.91 (2020-01-13). Fix: revert the buffer to a local variable, restoring the Buf_Init/Buf_Destroy pair that r1.91 removed. All 48 regression tests pass on amd64, i386, and arm64. Fuzzed for 5 minutes with AFL++ on amd64: 0 crashes. Found by AFL++ fuzzing. Index: usr.bin/make/arch.c =================================================================== RCS file: /cvs/src/usr.bin/make/arch.c,v retrieving revision 1.95 diff -u -p -r1.95 arch.c --- usr.bin/make/arch.c 26 Jan 2025 13:06:30 -0000 1.95 +++ usr.bin/make/arch.c @@ -193,10 +193,11 @@ bool Arch_ParseArchive(const char **line, Lst nodes, SymTable *ctxt) { bool result; - static BUFFER expand; + BUFFER expand; - Buf_Reinit(&expand, MAKE_BSIZE); + Buf_Init(&expand, MAKE_BSIZE); result = parse_archive(&expand, line, nodes, ctxt); + Buf_Destroy(&expand); return result; }
smime.p7s
Description: S/MIME Cryptographic Signature
