Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aOX0tZDPCf_KJVGk@raf.org>
Date: Wed, 8 Oct 2025 16:20:53 +1100
From: raf <musl@....org>
To: musl@...ts.openwall.com
Subject: Re: [PATCH] fnmatch: fix infinite loop when pattern is
 non-character byte

On Wed, Oct 08, 2025 at 12:56:49PM +1100, raf <musl@....org> wrote:

> On Mon, Oct 06, 2025 at 09:40:36PM -0400, Rich Felker <dalias@...c.org> wrote:
> 
> > On Sat, Oct 04, 2025 at 10:54:13PM +1000, raf wrote:
> > > Hi,
> > > 
> > > fnmatch() enters an infinite loop if the pattern is the non-character byte
> > > '\xf8'. This patch fixes it by stepping over the invalid byte.
> > 
> > This does not sound like a correct fix. The UNMATCHABLE return from
> > pat_next should produce an immediate FNM_NOMATCH return, not resume
> > parsing in an invalid location.
> 
> There are 2 calls to pat_next where the UNMATCHABLE return value
> results in the caller returning FNM_NOMATCH. These calls are in
> fnmatch_internal().
> 
> There are 2 more calls to pat_next() in fnmatch_internal() and 1
> more in fnmatch() where the caller doesn't check for UNMATCHABLE
> and then return FNM_NOMATCH.
> 
> Attached is a patch that adds the 3 missing checks for UNMATCHABLE.
> 
> > For future submissions, could you also please include patch
> > descriptions in the same email as the patch itself rather than
> > degenerate 2-mail threads? That helps make it so we can reply to both
> > the description/motivation and the actual code changes in a single
> > reply mail without manually copy-pasting from multiple mails and
> > picking a place to stitch it into the thread.
> 
> Sure. I was using git send-email. I won't do that anymore.
> 
> > Rich
> 
> cheers,
> raf

Perhaps it's only the call to pat_next() in fnmatch() that needed
to be changed. There is a comment in fnmat_internal() explaining
why the last two calls to pat_next() did not require checking for
UNMATCHABLE:

	/* Past this point we need not check for UNMATCHABLE in pat,
	 * because all of pat has already been parsed once. */

Attach is an alternative patch that only adds one check for UNMATCHABLE
in fnmatch() if you'd prefer that.

cheers,
raf


View attachment "0001-fnmatch-fix-infinite-loop-when-pattern-is-non-charac.patch" of type "text/x-diff" (955 bytes)

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.