Bug 166553

Summary: find(1): find -delete documentation is misleading
Product: Documentation Reporter: Richard Kettlewell <rjk>
Component: Manual PagesAssignee: freebsd-bugs (Nobody) <bugs>
Status: Open ---    
Severity: Affects Only Me CC: doc
Priority: Normal    
Version: Latest   
Hardware: Any   
OS: Any   

Description Richard Kettlewell 2012-04-01 10:30:12 UTC
The documentation for 'find -delete' includes the following remark:

    It will not attempt to delete a filename with a / character in its pathname relative to . for security reasons. 

This reflects a defensive test early in f_delete. However, the test will not succeed, since find does not use FTS_NOCHDIR, so entry->fts_accpath should never contain "/".

I suggest that this remark be deleted from the man page.
Comment 1 Matthew Story 2012-04-01 18:24:03 UTC
On Sun, Apr 1, 2012 at 5:24 AM, Richard Kettlewell <rjk@greenend.org.uk> wr=
ote:
>
> The documentation for 'find -delete' includes the following remark:
>
> =A0 =A0It will not attempt to delete a filename with a =93/=94 character =
in its pathname relative to =93.=94 for security reasons.
>
> This reflects a defensive test early in f_delete. However, the test will =
not succeed, since find does not use FTS_NOCHDIR, so entry->fts_accpath sho=
uld never contain "/".

$ # show test succeeds
$ mkdir -p foo/bar/baz
$ find ./foo -delete
find: -delete: ./foo: relative path potentially not safe

The reason that fts_accpath contains a leading ``./'' in this case is
that fts, even without FTS_NOCHDIR, does not chdir into the parent
directory of depth 0 directories on the first pass, fts_accpath will
always equal fts_pathname on depth 0 FTSENTs.

regards,
matt
Comment 2 Richard Kettlewell 2012-04-01 18:25:55 UTC
> The reason that fts_accpath contains a leading ``./'' in this case is
> that fts, even without FTS_NOCHDIR, does not chdir into the parent
> directory of depth 0 directories on the first pass, fts_accpath will
> always equal fts_pathname on depth 0 FTSENTs.

Light dawns l-)

Still, the wording could be improved: it doesn't really describe well 
when -delete will succeed and when it won't.

ttfn/rjk
Comment 3 Matthew Story 2012-04-01 20:25:00 UTC
> Still, the wording could be improved: it doesn't really describe well when
> -delete will succeed and when it won't.

Yes, the wording is confusing, and it's not actually a correct
assessment of what the code is doing ... it also doesn't note that
this condition is fatal:

$ # show that an absolute path also triggers the condition
$ ./find /usr/home/user1/foo -depth 0 -delete
find: -delete: /usr/home/user1/foo: relative path potentially not safe
$ echo $?
1

The code here is actually just fatalling out if FTSENT->fts_accpath
contains a ``/'', wether or not it is preceded by a ``.''  There is an
additional misleading comment in the code to the same end as the
manage.

/* Potentially unsafe - do not accept relative paths whatsoever */

--=20
regards,
matt
Comment 4 Mark Linimon freebsd_committer freebsd_triage 2012-04-02 07:23:23 UTC
Responsible Changed
From-To: freebsd-bugs->freebsd-doc

reclassify.
Comment 5 Jim Riggs 2012-04-24 20:57:37 UTC
I'm not sure that I would classify this as a documentation bug. It seems =
to be a code bug unless the documentation is just completely unclear as =
to how this is to behave. It seems that the intent is that you can't use =
-delete with ./ or ../ in the path; however, it completely blocks =
anything with / anywhere in the path. For example, this will fail (see =
bin/48271):

# cd /
# mkdir testdir
# mkdir testdir/testdir2
# find /testdir -delete
find: -delete: /testdir: relative path potentially not safe

Now, what is "relative" in the path '/testdir'? So, the question is, =
what is the code trying to stop? Is there a security issue with an =
_absolute_ path, a _relative_ path, or both? Right now, it is blocking =
both. Is -delete only supposed to be used in cwd (i.e. '.')? If so, why =
the restriction? I am just trying to understand what is "potentially not =
safe."

This issue is also hosing my `make release' on 9.0-RELEASE with =
WITHOUT_GAMES defined in src.conf:

...

install -o root -g wheel -m 444  =
/usr/src/etc/../sys/amd64/conf/GENERIC.hints  =
/usr/obj/release/dist/base/boot/device.hints
find //usr/obj/release/dist/doc -empty -delete
find //usr/obj/release/dist/games -empty -delete
find: -delete: //usr/obj/release/dist/games: relative path potentially =
not safe
*** Error code 1

Stop in /usr/src.
*** Error code 1

Stop in /usr/src.
*** Error code 1

Stop in /usr/src/release.
*** Error code 1

Stop in /usr/src/release.
Comment 6 Jilles Tjoelker freebsd_committer freebsd_triage 2014-03-02 22:38:25 UTC
In PR docs/166553, Richard Kettlewell wrote:
> The documentation for 'find -delete' includes the following remark:

> It will not attempt to delete a filename with a "/" character in its
> pathname relative to "." for security reasons.

> This reflects a defensive test early in f_delete. However, the test
> will not succeed, since find does not use FTS_NOCHDIR, so
> entry->fts_accpath should never contain "/".

> I suggest that this remark be deleted from the man page.

In SVN r246628 and r253886 (both are in 10.0-RELEASE), I removed the two
most common causes for the confusing error message about relative
pathnames: if "." cannot be opened (except if find(1)'s own open
succeeds but fts(3)'s open fails) and if an attempt is made to delete a
pathname given as operand to find(1). The former now gives a clear error
message and the latter is now allowed.

-- 
Jilles Tjoelker
Comment 7 Eitan Adler freebsd_committer freebsd_triage 2017-12-31 08:01:36 UTC
For bugs matching the following criteria:

Status: In Progress Changed: (is less than) 2014-06-01

Reset to default assignee and clear in-progress tags.

Mail being skipped