Skip Menu |
Report information
Id: 130900
Status: resolved
Priority: 0/
Queue: perl6

Owner: cpan [at] zoffix.com
Requestors: zefram [at] fysh.org
Cc:
AdminCc:

Severity: (no value)
Tag: Bug
Platform: (no value)
Patch Status: (no value)
VM: (no value)



Date: Thu, 2 Mar 2017 10:29:01 +0000
To: rakudobug [...] perl.org
From: Zefram <zefram [...] fysh.org>
Subject: [BUG] nul in pathname
Download (untitled) / with headers
text/plain 1.2k
Show quoted text
> spurt("foo\x[0]bar", "wibble")
True The above writes to a file named "foo". It is less than awesome that Perl 6 accepts "foo\x[0]bar" as a pathname, when it doesn't resemble any pathname valid on the OS, and silently treats it as a quite different pathname. It would be better for it to signal an error, that "foo\x[0]bar" isn't a pathname. It's not an outright bug that it aliases pathnames in this manner, but it is liable to confuse other code that manipulates pathnames. Code like this: Show quoted text
> $*SPEC.splitpath("foo/bar\x[0]baz/quux").perl
("", "foo/bar\0baz/", "quux") This *is* an outright bug. If the string containing a nul is to be accepted as a way of stating a pathname, as it is by spurt(), then IO::Spec needs to know about that rule, and needs to parse pathnames accordingly. The above should behave the same as splitting "foo/bar", and thus should return ("", "foo/", "bar"). It could optionally retain the nul and everything following it as part of the basename component of the output, but though technically correct that would be confusing. Of course, if spurt() et al did not accept nuls in pathnames, then splitpath ought to error on such inputs too. Watch out for "\x[0]\x[308]" when trying to detect nuls. -zefram
Download (untitled) / with headers
text/plain 288b
On Thu, 02 Mar 2017 02:29:27 -0800, zefram@fysh.org wrote: Show quoted text
> Watch out for "\x[0]\x[308]" when trying to detect nuls. > > -zefram
Thankfully, \0 won't accept diacritics, though i'd have to look at the NFG algo to see if that's valid for every diacritic in unicode, or just some subset.
To: perl6-compiler <perl6-compiler [...] perl.org>
From: Brandon Allbery <allbery.b [...] gmail.com>
Subject: Re: [perl #130900] [BUG] nul in pathname
CC: bugs-bitbucket [...] rt.perl.org
Date: Thu, 2 Mar 2017 08:52:51 -0500
Download (untitled) / with headers
text/plain 818b

On Thu, Mar 2, 2017 at 5:29 AM, Zefram <perl6-bugs-followup@perl.org> wrote:
Show quoted text
It is less than awesome that
Perl 6 accepts "foo\x[0]bar" as a pathname, when it doesn't resemble
any pathname valid on the OS

It's also less than awesome that POSIX, at least, doesn't let you confirm that a filename is syntactically valid, either on the OS or on a specific filesystem. And hardcoding that knowledge into Perl (any version) would be a severe mistake. The code is a "bug" but Perl cannot be prescient for you, or hack the kernel and filesystem drivers to find out what is valid.

--
brandon s allbery kf8nh                               sine nomine associates
allbery.b@gmail.com                                  ballbery@sinenomine.net
unix, openafs, kerberos, infrastructure, xmonad        http://sinenomine.net
RT-Send-CC: perl6-compiler [...] perl.org
Download (untitled) / with headers
text/plain 1014b
FWIW, this bug makes at least mild exploitation possible, depending on how the program validates input: Setup: dir and input check to ensure user-supplied path is not outside of it: 13:41 IOninja m: "/tmp/root/tmp".IO.mkdir; "/tmp/root/secret.txt".IO.spurt: 'p4sswrd'; 13:41 camelia rakudo-moar 9d138b: ( no output ) Detection is triggered when `..` is supplied as the path: 13:41 IOninja m: chdir "/tmp/root/tmp"; my $user-input = ".."; $user-input.IO.resolve.starts-with: $*CWD.resolve or die "hax!"; .say for dir $user-input 13:41 camelia rakudo-moar 9d138b: OUTPUT: «hax!␤ in block <unit> at <tmp> line 1␤␤» But fails when we follow it with a nul byte, causing display of contents of parent directory: 13:41 m: chdir "/tmp/root/tmp"; my $user-input = "..\0"; $user-input.IO.resolve.starts-with: $*CWD.resolve or die "hax!"; .say for dir $user-input 13:41 camelia rakudo-moar 9d138b: OUTPUT: «"..␀/tmp".IO␤"..␀/secret.txt".IO␤» 13:41 IOninja oops
To: perl6-compiler <perl6-compiler [...] perl.org>
From: Brandon Allbery <allbery.b [...] gmail.com>
Subject: Re: [perl #130900] [BUG] nul in pathname
Date: Thu, 2 Mar 2017 08:56:57 -0500
CC: bugs-bitbucket [...] rt.perl.org
Download (untitled) / with headers
text/plain 625b
To be clear: the POSIX spec does, specifically, disallow NUL. *Only* NUL.

Which then leaves:

- any character disallowed by specific filesystems (consider CIFS)
- the sequence `//` which is specifically undefined by POSIX

Among others. Is it correct to disallow NUL and thereby have a special case where all other implementation-undefined filenames are not caught --- and not catchable? 

--
brandon s allbery kf8nh                               sine nomine associates
allbery.b@gmail.com                                  ballbery@sinenomine.net
unix, openafs, kerberos, infrastructure, xmonad        http://sinenomine.net
To: perl6-compiler <perl6-compiler [...] perl.org>
From: Brandon Allbery <allbery.b [...] gmail.com>
Subject: Re: [perl #130900] [BUG] nul in pathname
Date: Thu, 2 Mar 2017 08:58:37 -0500
CC: bugs-bitbucket [...] rt.perl.org
Download (untitled) / with headers
text/plain 538b

On Thu, Mar 2, 2017 at 8:56 AM, Brandon Allbery <allbery.b@gmail.com> wrote:
Show quoted text
- the sequence `//` which is specifically undefined by POSIX

In the interests of heading off incoming pedanticness, possibly to "defend" the indefensible: this is only undefined as a prefix. It is an error as an infix.


--
brandon s allbery kf8nh                               sine nomine associates
allbery.b@gmail.com                                  ballbery@sinenomine.net
unix, openafs, kerberos, infrastructure, xmonad        http://sinenomine.net
Date: Thu, 2 Mar 2017 15:14:56 +0100
To: perl6-compiler [...] perl.org
Subject: Re: [perl #130900] [BUG] nul in pathname
From: Joachim Durchholz <jo [...] durchholz.org>
On 02.03.2017 14:56, Brandon Allbery wrote: Show quoted text
> - any character disallowed by specific filesystems (consider CIFS)
Wouldn't these characters be caught by the file system (whichever that is)? It would be pretty hard to do a reliable validation as soon as mounted filesystems come into play. Show quoted text
> Among others. Is it correct to disallow NUL and thereby have a special > case where all other implementation-undefined filenames are not caught > --- and not catchable?
Yes. NUL is a precondition - you are not supposed to pass in a file name with NULs in it, in theory that's nasal demon land, in practice most file systems will truncate the file name at the first NUL (which gives rise to all kinds of security issues for programs that are not aware of this kind of behaviour and don't check for NULs in the string). For the other invalid characters, the error will (well: should) be caught by the filesystem. Trying to check that in advance is a security hole in waiting - somebody might mount or unmount filesystems between check and use.
CC: perl6-compiler <perl6-compiler [...] perl.org>
Date: Thu, 2 Mar 2017 09:25:07 -0500
Subject: Re: [perl #130900] [BUG] nul in pathname
From: Brandon Allbery <allbery.b [...] gmail.com>
To: Joachim Durchholz <jo [...] durchholz.org>
Download (untitled) / with headers
text/plain 1021b

On Thu, Mar 2, 2017 at 9:14 AM, Joachim Durchholz <jo@durchholz.org> wrote:
Show quoted text
For the other invalid characters, the error will (well: should) be caught by the filesystem.
Trying to check that in advance is a security hole in waiting - somebody might mount or unmount filesystems between check and use.

This, sadly, is not always true. The // prefix is a specific case in point.
CIFS/NTFS *does* catch invalid characters... unless the kernel filesystem is buggy, which Linux's has been. Multiple times. The result of that is files that you can write but fail to read back, sometimes on Linux and sometimes on the remote / the Windows dual-boot, as appropriate. Again, this knowledge *should not* be coded into perl; we can't be nannies for Linux filesystem driver developers.

--
brandon s allbery kf8nh                               sine nomine associates
allbery.b@gmail.com                                  ballbery@sinenomine.net
unix, openafs, kerberos, infrastructure, xmonad        http://sinenomine.net
Date: Thu, 2 Mar 2017 14:29:28 +0000
To: Brandon Allbery via RT <perl6-bugs-followup [...] perl.org>
From: Zefram <zefram [...] fysh.org>
Subject: Re: [perl #130900] [BUG] nul in pathname
Download (untitled) / with headers
text/plain 929b
Brandon Allbery via RT wrote: Show quoted text
>It's also less than awesome that POSIX, at least, doesn't let you confirm >that a filename is syntactically valid,
At the syscall interface it's very simple: any sequence of non-nul octets may be used as a pathname. Since the pathname is passed in nul-delimited form, this is not so much a rule about what's permitted as about what's representable. Any rules beyond that about filename format (limited length, limited character repertoire, or whatever) are enforced by the kernel, and use of invalid names will produce appropriate syscall errors (such as ENAMETOOLONG). Perl 6 doesn't need to know about those rules that are enforced by the kernel. The syscall errors are sufficient. What Perl 6 *does* need to know about is the syntax at the syscall interface: that a pathname cannot contain a nul, and the manner in which a pathname splits into filenames at directory separators. -zefram
Date: Thu, 2 Mar 2017 14:54:51 +0000
From: Zefram <zefram [...] fysh.org>
Subject: Re: [perl #130900] [BUG] nul in pathname
To: Timo Paulssen via RT <perl6-bugs-followup [...] perl.org>
Download (untitled) / with headers
text/plain 402b
Timo Paulssen via RT wrote: Show quoted text
>Thankfully, \0 won't accept diacritics,
Ah, quite right. I forgot about that rule. The text segmentation standard says that there's a grapheme cluster boundary before and after every control character, except in the middle of a CR-LF sequence. Hence a nul codepoint (and thus a nul octet in UTF-8) cannot arise from any grapheme cluster other than a bare nul. -zefram
RT-Send-CC: perl6-compiler [...] perl.org
Download (untitled) / with headers
text/plain 1.9k
On Thu, 02 Mar 2017 02:29:27 -0800, zefram@fysh.org wrote: Show quoted text
> > spurt("foo\x[0]bar", "wibble")
> True > > The above writes to a file named "foo". It is less than awesome that > Perl 6 accepts "foo\x[0]bar" as a pathname, when it doesn't resemble > any pathname valid on the OS, and silently treats it as a quite > different pathname. It would be better for it to signal an error, that > "foo\x[0]bar" isn't a pathname. It's not an outright bug that it aliases > pathnames in this manner, but it is liable to confuse other code that > manipulates pathnames. Code like this: >
> > $*SPEC.splitpath("foo/bar\x[0]baz/quux").perl
> ("", "foo/bar\0baz/", "quux") > > This *is* an outright bug. If the string containing a nul is to be > accepted as a way of stating a pathname, as it is by spurt(), then > IO::Spec needs to know about that rule, and needs to parse pathnames > accordingly. The above should behave the same as splitting "foo/bar", > and thus should return ("", "foo/", "bar"). It could optionally retain > the nul and everything following it as part of the basename component > of the output, but though technically correct that would be confusing. > Of course, if spurt() et al did not accept nuls in pathnames, then > splitpath ought to error on such inputs too. > > Watch out for "\x[0]\x[308]" when trying to detect nuls. > > -zefram
Thank you for the report. This is now fixed in high-level IO routines and clases: Fix: https://github.com/rakudo/rakudo/commit/e6814986c0 Tests: https://github.com/perl6/roast/commit/b16fbd3b24 For the case of $*SPEC.splitpath: it's a low-level routine that most users won't be using, unless they're developing their own path manipulation routines. So, in that case, I'm leaving them without any nul detection, documenting the fact that it needs to be done by the users of these low-level routines in: https://github.com/perl6/doc/commit/d436f3cc7896799f3358f99321b40b3b5512a72b - IO grant


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

For issues related to this RT instance (aka "perlbug"), please contact perlbug-admin at perl.org