Re: make(1): heap corruption via recursive archive spec parsing

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.
>