NetBSD Problem Report #56775
From www@netbsd.org Thu Mar 31 19:06:29 2022
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits))
(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
by mollari.NetBSD.org (Postfix) with ESMTPS id 13EC81A921F
for <gnats-bugs@gnats.NetBSD.org>; Thu, 31 Mar 2022 19:06:29 +0000 (UTC)
Message-Id: <20220331190627.90C811A9239@mollari.NetBSD.org>
Date: Thu, 31 Mar 2022 19:06:27 +0000 (UTC)
From: rvp@SDF.ORG
Reply-To: rvp@SDF.ORG
To: gnats-bugs@NetBSD.org
Subject: sort -o foo foo can sometines remove foo
X-Send-Pr-Version: www-1.0
>Number: 56775
>Category: bin
>Synopsis: sort -o foo foo can sometines remove foo
>Confidential: no
>Severity: serious
>Priority: medium
>Responsible: bin-bug-people
>State: open
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Thu Mar 31 19:10:00 +0000 2022
>Last-Modified: Mon Apr 04 21:40:01 +0000 2022
>Originator: RVP
>Release: NetBSD/amd64 9.99.96
>Organization:
>Environment:
NetBSD x202e.localdomain 9.99.96 NetBSD 9.99.96 (MYKERNEL) #0: Thu Mar 31 09:03:09 UTC 2022 bld@x202e.localdomain:/usr/obj/usr/src/sys/arch/amd64/compile/MYKERNEL amd64
>Description:
Under certain circumstances, `sort -o foo foo' can end up deleting `foo':
$ sudo sysctl -w security.models.extensions.hardlink_check_uid=1
$ sudo sysctl -w security.models.extensions.hardlink_check_gid=1
$ cd /tmp # cd to a world-writable dir.
$ >foo
$ ls -l foo # note that group is `wheel' as per BSD
# semantics for /tmp
-rw------- 1 rvp wheel 0 Mar 31 08:32 foo
$ sort -o foo foo
sort: cannot link foo: output left in foopSQspp: Permission denied
$ ls -l foo
ls: foo: No such file or directory
$
Input file is destroyed if user doesn't belong to group `wheel'--even
though files can be created and renamed by anyone in /tmp.
This results in this error message (caused by running lorder) while
building the kernel:
[...]
compile kern/umodti3.o
compile kern/xlat_mbr_fstype.o
building standard kern library
sort: cannot link /tmp/_reference_.psA3FN: output left in /tmp/_reference_.psA3FNlDcAku: Permission denied
sort: cannot link /tmp/_symbol_.Gy4pSK: output left in /tmp/_symbol_.Gy4pSKrHWftZ: Permission denied
nbjoin: /tmp/_reference_.psA3FN: No such file or directory
create vers.c
[...]
>How-To-Repeat:
As above.
>Fix:
Use rename(2):
diff -urN usr.bin/sort.orig/sort.c usr.bin/sort/sort.c
--- usr.bin/sort.orig/sort.c 2017-01-10 21:13:45.000000000 +0000
+++ usr.bin/sort/sort.c 2022-03-31 09:17:25.991550462 +0000
@@ -329,7 +329,7 @@
errno = 0;
if (access(outpath, W_OK))
err(2, "%s", outpath);
- (void)snprintf(toutpath, sizeof(toutpath), "%sXXXXXX",
+ (void)snprintf(toutpath, sizeof(toutpath), "%s.XXXXXX",
outpath);
if ((outfd = mkstemp(toutpath)) == -1)
err(2, "Cannot create temporary file `%s'", toutpath);
@@ -368,11 +368,8 @@
outpath, outfile);
}
- (void)unlink(outpath);
- if (link(outfile, outpath))
- err(2, "cannot link %s: output left in %s",
- outpath, outfile);
- (void)unlink(outfile);
+ if (rename(outfile, outpath) == -1)
+ err(2, "rename failed: %s->%s", outfile, outpath);
toutpath[0] = 0;
}
exit(0);
>Audit-Trail:
From: Thomas Klausner <wiz@NetBSD.org>
To: gnats-bugs@netbsd.org
Cc:
Subject: Re: bin/56775: sort -o foo foo can sometines remove foo
Date: Thu, 31 Mar 2022 21:19:06 +0200
On Thu, Mar 31, 2022 at 07:10:00PM +0000, rvp@SDF.ORG wrote:
> @@ -368,11 +368,8 @@
> outpath, outfile);
> }
>
> - (void)unlink(outpath);
> - if (link(outfile, outpath))
> - err(2, "cannot link %s: output left in %s",
> - outpath, outfile);
> - (void)unlink(outfile);
> + if (rename(outfile, outpath) == -1)
> + err(2, "rename failed: %s->%s", outfile, outpath);
> toutpath[0] = 0;
> }
> exit(0);
>
Shouldn't the there be an
unlink(outfile);
in the error case?
Thomas
From: RVP <rvp@SDF.ORG>
To: gnats-bugs@netbsd.org
Cc:
Subject: Re: bin/56775: sort -o foo foo can sometines remove foo
Date: Thu, 31 Mar 2022 21:31:11 +0000 (UTC)
On Thu, 31 Mar 2022, Thomas Klausner wrote:
> Shouldn't the there be an
>
> unlink(outfile);
>
> in the error case?
>
rename(2) will do that.
-RVP
From: Thomas Klausner <wiz@NetBSD.org>
To: NetBSD bugtracking <gnats-bugs@NetBSD.org>
Cc:
Subject: Re: bin/56775: sort -o foo foo can sometines remove foo
Date: Thu, 31 Mar 2022 23:49:16 +0200
On Thu, Mar 31, 2022 at 09:35:01PM +0000, RVP wrote:
> The following reply was made to PR bin/56775; it has been noted by GNATS.
>
> From: RVP <rvp@SDF.ORG>
> To: gnats-bugs@netbsd.org
> Cc:
> Subject: Re: bin/56775: sort -o foo foo can sometines remove foo
> Date: Thu, 31 Mar 2022 21:31:11 +0000 (UTC)
>
> On Thu, 31 Mar 2022, Thomas Klausner wrote:
>
> > Shouldn't the there be an
> >
> > unlink(outfile);
> >
> > in the error case?
> >
>
> rename(2) will do that.
If the rename fails, it won't. That's why I wrote "in the error case".
Thomas
From: RVP <rvp@SDF.ORG>
To: gnats-bugs@netbsd.org
Cc:
Subject: Re: bin/56775: sort -o foo foo can sometines remove foo
Date: Thu, 31 Mar 2022 21:59:24 +0000 (UTC)
On Thu, 31 Mar 2022, Thomas Klausner wrote:
> If the rename fails, it won't. That's why I wrote "in the error case".
> Thomas
>
cleanup() will remove the temp. file.
-RVP
From: RVP <rvp@SDF.ORG>
To: gnats-bugs@netbsd.org
Cc:
Subject: Re: bin/56775: sort -o foo foo can sometines remove foo
Date: Mon, 4 Apr 2022 21:39:51 +0000 (UTC)
Noticed another problem with sort: the -o flag doesn't preserve
the original file type. FIFOs and symlinks are turned into ordinary
files:
$ >foo
$ ln -s foo bar
$ ls -l bar
lrwx------ 1 rvp wheel 3 Apr 4 21:04 bar -> foo
$ sort -o bar bar
$ ls -l bar
-rwx------ 1 rvp wheel 0 Apr 4 21:05 bar
$
The straight-forward "fix" of changing the test on line 323 to:
&& S_ISREG(st.st_mode)) {
will deadlock on FIFOs because both the input and output files will
be the same (and have to be kept open). I'm copying the temp. file
into the FIFO in this case. (It also doesn't work if the files are
symlinks--and they, in turn, point to other symlinks. I'm also
assuming that following symlinks is the correct behaviour.)
The patch below also includes the previous patch:
---START---
diff -urN usr.bin/sort.orig/sort.c usr.bin/sort/sort.c
--- usr.bin/sort.orig/sort.c 2017-01-10 21:13:45.000000000 +0000
+++ usr.bin/sort/sort.c 2022-04-04 20:47:12.959039129 +0000
@@ -80,6 +80,7 @@
#include <locale.h>
#include <paths.h>
#include <signal.h>
+#include <stdbool.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
@@ -105,12 +106,14 @@
unsigned int debug_flags = 0;
-static char toutpath[MAXPATHLEN];
+static char toutpath[MAXPATHLEN + 8];
+static char truepath[MAXPATHLEN];
const char *tmpdir; /* where temporary files should be put */
static void cleanup(void);
static void onsignal(int);
+static bool doesexist(char *outpath, struct stat *st);
__dead static void usage(const char *);
int
@@ -319,7 +322,7 @@
toutpath[0] = '\0'; /* path not used in this case */
outfile = outpath = toutpath;
outfp = stdout;
- } else if (lstat(outpath, &st) == 0
+ } else if (doesexist(outpath, &st)
&& !S_ISCHR(st.st_mode) && !S_ISBLK(st.st_mode)) {
/* output file exists and isn't character or block device */
struct sigaction act;
@@ -327,9 +330,10 @@
SIGXCPU, SIGXFSZ, SIGVTALRM, SIGPROF, 0};
int outfd;
errno = 0;
+ outpath = truepath;
if (access(outpath, W_OK))
err(2, "%s", outpath);
- (void)snprintf(toutpath, sizeof(toutpath), "%sXXXXXX",
+ (void)snprintf(toutpath, sizeof(toutpath), "%s.XXXXXX",
outpath);
if ((outfd = mkstemp(toutpath)) == -1)
err(2, "Cannot create temporary file `%s'", toutpath);
@@ -363,21 +367,52 @@
* st is initialized above, when we create the
* temporary spool file.
*/
- if (lchmod(outfile, st.st_mode & ALLPERMS) != 0) {
+ if (chmod(outfile, st.st_mode & ALLPERMS) != 0) {
err(2, "cannot chmod %s: output left in %s",
outpath, outfile);
}
- (void)unlink(outpath);
- if (link(outfile, outpath))
- err(2, "cannot link %s: output left in %s",
- outpath, outfile);
- (void)unlink(outfile);
- toutpath[0] = 0;
+ if (S_ISREG(st.st_mode)) {
+ if (rename(outfile, outpath) == -1)
+ err(2, "rename failed: %s->%s", outfile, outpath);
+ toutpath[0] = 0;
+ } else { /* FIFO: copy sorted */
+ char buf[BUFSIZ];
+ FILE *ifp, *ofp;
+ size_t n;
+
+ if (fclose(outfp))
+ err(2, "IO error");
+ if ((ifp = fopen(outfile, "r")) == NULL)
+ err(2, "cannot read %s", outfile);
+ if ((ofp = fopen(outpath, "w")) == NULL)
+ err(2, "cannot write %s", outpath);
+ while ((n = fread(buf, 1, sizeof buf, ifp)) > 0)
+ if (fwrite(buf, 1, n, ofp) != n)
+ break;
+ if (fclose(ofp) || ferror(ifp))
+ err(2, "IO error");
+ }
}
exit(0);
}
+static bool
+doesexist(char *outpath, struct stat *st)
+{
+ if (lstat(outpath, st) == -1)
+ return false; /* assume no file */
+ if (!S_ISLNK(st->st_mode)) {
+ strlcpy(truepath, outpath, sizeof truepath);
+ return true; /* skip a func. call */
+ }
+ if (realpath(outpath, truepath) == NULL)
+ err(2, "%s: realpath failed", outpath); /* hopeless */
+ if (stat(truepath, st) == -1)
+ err(2, "%s: stat failed", outpath); /* " */
+ return true;
+}
+
static void
onsignal(int sig)
{
---END---
Thanks,
-RVP
(Contact us)
$NetBSD: query-full-pr,v 1.46 2020/01/03 16:35:01 leot Exp $
$NetBSD: gnats_config.sh,v 1.9 2014/08/02 14:16:04 spz Exp $
Copyright © 1994-2020
The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.