On 03/04/2026 20:51, Marc Espie wrote:
On Fri, Apr 03, 2026 at 11:26:29AM +0200, Renaud Allard wrote: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; }I'll have a closer look.
Nice, thanks
The fact that a bug has been in make for 6 years is no excuse.
I was not stating it as a real excuse, but more something like we have dead code in the sense that it is never used in real life.
I kind-of inherited this code that's horrible and is not going anywhere. There are lots and lots of fires to extinguish in that code base, some of them are way way worse than what you've stumbled upon, so basically consider that what's working is working (kind-of) and everything else is broken.
smime.p7s
Description: S/MIME Cryptographic Signature
