Bug 14185 - fnmatch() fails when '*' wildcard is applied on the file name containing multi-byte character(s)
fnmatch() fails when '*' wildcard is applied on the file name containing mult...
Status: NEW
Product: glibc
Classification: Unclassified
Component: libc
unspecified
: P2 normal
: ---
Assigned To: Not yet assigned to anyone
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-05-30 19:01 UTC by Patsy Franklin
Modified: 2012-06-18 17:17 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments
simple testcase (214 bytes, text/x-csrc)
2012-05-30 19:01 UTC, Patsy Franklin
Details
patch file (2.63 KB, patch)
2012-05-31 20:18 UTC, Patsy Franklin
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Patsy Franklin 2012-05-30 19:01:45 UTC
Created attachment 6427 [details]
simple testcase

fnmatch tries to convert the string (filename) from multibyte to a wide character representation via mbsrtowcs.  When the encoding is invalid (say \366 when using en_US.UTF-8), mbsrtowcs returns EILSEQ (illegal byte sequence).
Comment 1 Rich Felker 2012-05-31 00:29:51 UTC
While this is not a conformance issue (actually, the most correct/conformant thing may be to reject as non-matching or give an error on invalid strings like this), I believe it is a quality-of-implementation issue. It makes it unnecessarily difficult to do things like deleting junk files created by an obnoxious user or extracting a corrupt archive file (or just one created by someone with poor taste in character encoding); "rm -f *" will fail.

As a related (same fundamental cause: the conversion to a wchar_t string) quality-of-implementation issue, fnmatch("*", huge_string, 0) fails on glibc even though it should obviously match without even having to inspect huge_string, much less make a 4x-size copy of it.

Unfortunately glibc's fnmatch implementation is just really ugly, and I don't think issues like this can be fixed without piling on ugly hacks, or just replacing the implementation with something saner...
Comment 2 law 2012-05-31 02:49:09 UTC
AFAICT there are no defined errors from fnmatch.  It's defined as returning 0 for a match, FNM_NOMATCH for no match and another non-zero value for an error.  However, no specific error codes are defined.

But what doesn't make sense about the current implementation is if pattern is something like *.csv and the string is <invalid>.csv, then we return an error.  To me that seems plainly wrong as we can get a positive match regardless of the invalid character in the string.

As you note, it makes rm -f * fail as will "find" and a variety of other tools which rely upon fnmatch.  This actually came to our attention because "find" was missing an obvious match because of an invalid character in a filename.

It's worth noting that gnulib's fnmatch has been fixed to address this problem.


Yes, there's an additional issue with handling out of memory conditions with large strings.  We don't have a fix for that and I think it should be tracked as a separate and distinct issue.
Comment 3 Patsy Franklin 2012-05-31 20:18:10 UTC
Created attachment 6429 [details]
patch file
Comment 4 Rich Felker 2012-06-01 02:32:05 UTC
This patch fixes the bug in the test case, but I'm not convinced that it doesn't create other (possibly much worse!) bugs in the process. For instance, the pattern "[á]*" should not match "\xa1.txt", but with fallback to treating everything as bytes, it does match. Basically this patch would make "rm -f pattern" potentially delete lots of files with illegal byte sequences in their names whenever pattern contains a bracket expression with multibyte characters in it. (Note that the effect becomes even more severe with a range expression like [à-á]...)
Comment 5 law 2012-06-18 17:17:34 UTC
Rich,
Yes, the proposed patch may have an issue around pattern "[á]*" should not match "\xa1.txt"; not sure if that's a worse situation or not.

The problem the patch fixes has been reported multiple times over the last 10 years or so without anyone taking action in glibc.  gnulib fixed it in effectively the same way as the patch above in 2005 and I don't think anyone's reported problems.

SuSE had the patch attached to this BZ in their distro for a while, but pulled it for reasons unknown.