Re: Patch to add -f flag to cat(1)
On 18/07/2015 07:40, Philip Guenther wrote: You have in mind a place where this would be used? Where are there bugs that this would resolve? Hi Philip, I originally thought it was meant to be a performance thing in busy environments but that's because I'd misinterpreted things due to O_NONBLOCK flag. The feature was actually added to ensure whatever cat was meant to be reading from was indeed a plain file and not another which could block a process. Use cat -f to avoid denial of service attacks by people who make .rhosts files fifos. http://mail-index.netbsd.org/source-changes/2000/01/14/0069.html This diff as it stands has 2 flaws: 1) The new flag doesn't actually work as intended because the OpenBSD fopen(3) doesn't support the f mode. 2) It passes the f flag to fopen by default, I need integrate the changes from r1.24 which makes it so that the flag is only passed if it has been specified by the use. I'm happy to follow up with a new diff, the question is, is the functionality it provides of interest? Sevan
Re: Patch to add -f flag to cat(1)
The place to solve this is in whatever is using cat for this purpose. check for the file type before blindly cat'ing. this solution is like soaking your clothing with antiseptic every morning because you are prone to stabbing yourself. On Sun, Jul 19, 2015 at 8:26 AM, Ted Unangst t...@tedunangst.com wrote: Sevan Janiyan wrote: The feature was actually added to ensure whatever cat was meant to be reading from was indeed a plain file and not another which could block a process. Use cat -f to avoid denial of service attacks by people who make .rhosts files fifos. http://mail-index.netbsd.org/source-changes/2000/01/14/0069.html hmm, well, security(8) in openbsd is a perl script that doesn't exec cat, so this wouldn't help solve that problem. now, looking at security, it seems there may be an issue if it tries to open a blocking file, but that will need solving there, not in cat.
Re: Patch to add -f flag to cat(1)
Hi Philip,
Philip Guenther wrote on Sun, Jul 19, 2015 at 11:19:53AM -0700:
On Sun, Jul 19, 2015 at 11:04 AM, Ingo Schwarze schwa...@usta.de wrote:
Philip Guenther wrote on Sun, Jul 19, 2015 at 10:28:57AM -0700:
On Sun, Jul 19, 2015 at 10:24 AM, Ingo Schwarze schwa...@usta.de wrote:
I don't think we are vulnerable.
If my analysis is accurate, the only user-controlled files
we open in security(8) are ~/.rhosts and ~/.shosts
in check_rhosts_content(). However, there is
next unless -s $filename;
right before the open(), and for fifos, -s returns false:
TOCTOU race there. If they can hit the gap and move a fifo
over a normal file between the test and the open, the open
will hang. Should switch to sysopen() with O_NONBLOCK.
Oops, indeed.
Index: security
===
RCS file: /cvs/src/libexec/security/security,v
retrieving revision 1.35
diff -u -p -r1.35 security
--- security21 Apr 2015 10:24:22 - 1.35
+++ security19 Jul 2015 18:02:38 -
@@ -22,7 +22,7 @@ use strict;
use Digest::SHA qw(sha256_hex);
use Errno qw(ENOENT);
-use Fcntl qw(:mode);
+use Fcntl qw(O_RDONLY O_NONBLOCK :mode);
use File::Basename qw(basename);
use File::Compare qw(compare);
use File::Copy qw(copy);
@@ -371,7 +371,7 @@ sub check_rhosts_content {
foreach my $base (qw(rhosts shosts)) {
my $filename = $home/.$base;
next unless -s $filename;
- nag !open(my $fh, '', $filename),
+ nag !sysopen(my $fh, $filename, O_RDONLY | O_NONBLOCK),
open: $filename: $!
and next;
local $_;
You need to then test the resulting file handle to verify it was a
normal file. I think just
nag !-f $fh
?
I don't think that's very important. If users make their ~/.rhosts
file a fifo, the script won't warn because the -s above the open()
bails. If they go to the length of racing the -s to prevent that,
the above code will do about the same, in particular not warn either,
because the subsequent read won't return any data. So why should
we warn in that exotic case?
Then again, if you insist, nothing much is wrong with the patch
below, even though i'd probably mildly prefer the simpler one above.
Yours,
Ingo
Index: security
===
RCS file: /cvs/src/libexec/security/security,v
retrieving revision 1.35
diff -u -p -r1.35 security
--- security21 Apr 2015 10:24:22 - 1.35
+++ security20 Jul 2015 00:48:28 -
@@ -22,7 +22,7 @@ use strict;
use Digest::SHA qw(sha256_hex);
use Errno qw(ENOENT);
-use Fcntl qw(:mode);
+use Fcntl qw(O_RDONLY O_NONBLOCK :mode);
use File::Basename qw(basename);
use File::Compare qw(compare);
use File::Copy qw(copy);
@@ -371,8 +371,11 @@ sub check_rhosts_content {
foreach my $base (qw(rhosts shosts)) {
my $filename = $home/.$base;
next unless -s $filename;
- nag !open(my $fh, '', $filename),
+ nag !sysopen(my $fh, $filename, O_RDONLY | O_NONBLOCK),
open: $filename: $!
+ and next;
+ nag !(-f $fh),
+ $filename is not a regular file
and next;
local $_;
nag /^\+\s*$/,
Re: Patch to add -f flag to cat(1)
On Sun, Jul 19, 2015 at 10:24 AM, Ingo Schwarze schwa...@usta.de wrote: ... I don't think we are vulnerable. If my analysis is accurate, the only user-controlled files we open in security(8) are ~/.rhosts and ~/.shosts in check_rhosts_content(). However, there is next unless -s $filename; right before the open(), and for fifos, -s returns false: TOCTOU race there. If they can hit the gap and move a fifo over a normal file between the test and the open, the open will hang. Should switch to sysopen() with O_NONBLOCK. Philip
Re: Patch to add -f flag to cat(1)
Hi Philip,
Philip Guenther wrote on Sun, Jul 19, 2015 at 10:28:57AM -0700:
On Sun, Jul 19, 2015 at 10:24 AM, Ingo Schwarze schwa...@usta.de wrote:
I don't think we are vulnerable.
If my analysis is accurate, the only user-controlled files
we open in security(8) are ~/.rhosts and ~/.shosts
in check_rhosts_content(). However, there is
next unless -s $filename;
right before the open(), and for fifos, -s returns false:
TOCTOU race there. If they can hit the gap and move a fifo
over a normal file between the test and the open, the open
will hang. Should switch to sysopen() with O_NONBLOCK.
Oops, indeed.
OK?
Ingo
Index: security
===
RCS file: /cvs/src/libexec/security/security,v
retrieving revision 1.35
diff -u -p -r1.35 security
--- security21 Apr 2015 10:24:22 - 1.35
+++ security19 Jul 2015 18:02:38 -
@@ -22,7 +22,7 @@ use strict;
use Digest::SHA qw(sha256_hex);
use Errno qw(ENOENT);
-use Fcntl qw(:mode);
+use Fcntl qw(O_RDONLY O_NONBLOCK :mode);
use File::Basename qw(basename);
use File::Compare qw(compare);
use File::Copy qw(copy);
@@ -371,7 +371,7 @@ sub check_rhosts_content {
foreach my $base (qw(rhosts shosts)) {
my $filename = $home/.$base;
next unless -s $filename;
- nag !open(my $fh, '', $filename),
+ nag !sysopen(my $fh, $filename, O_RDONLY | O_NONBLOCK),
open: $filename: $!
and next;
local $_;
Re: Patch to add -f flag to cat(1)
On 19/07/2015 16:13, Ted Unangst wrote: I could maybe be convinced. However, fopen is the C standard stdio function. One reason you may be using stdio is because you want portability, so adding nonportable extensions to it seems counter productive. Understood, I'll leave it as it's not required. If you need to know about all sorts of fifos vs sockets vs files, there are a variety of posix APIs for that, portable to all posix systems. Noted :) Thanks. Sevan
Re: Patch to add -f flag to cat(1)
On Sun, Jul 19, 2015 at 11:04 AM, Ingo Schwarze schwa...@usta.de wrote:
Philip Guenther wrote on Sun, Jul 19, 2015 at 10:28:57AM -0700:
On Sun, Jul 19, 2015 at 10:24 AM, Ingo Schwarze schwa...@usta.de wrote:
I don't think we are vulnerable.
If my analysis is accurate, the only user-controlled files
we open in security(8) are ~/.rhosts and ~/.shosts
in check_rhosts_content(). However, there is
next unless -s $filename;
right before the open(), and for fifos, -s returns false:
TOCTOU race there. If they can hit the gap and move a fifo
over a normal file between the test and the open, the open
will hang. Should switch to sysopen() with O_NONBLOCK.
Oops, indeed.
OK?
Ingo
Index: security
===
RCS file: /cvs/src/libexec/security/security,v
retrieving revision 1.35
diff -u -p -r1.35 security
--- security21 Apr 2015 10:24:22 - 1.35
+++ security19 Jul 2015 18:02:38 -
@@ -22,7 +22,7 @@ use strict;
use Digest::SHA qw(sha256_hex);
use Errno qw(ENOENT);
-use Fcntl qw(:mode);
+use Fcntl qw(O_RDONLY O_NONBLOCK :mode);
use File::Basename qw(basename);
use File::Compare qw(compare);
use File::Copy qw(copy);
@@ -371,7 +371,7 @@ sub check_rhosts_content {
foreach my $base (qw(rhosts shosts)) {
my $filename = $home/.$base;
next unless -s $filename;
- nag !open(my $fh, '', $filename),
+ nag !sysopen(my $fh, $filename, O_RDONLY | O_NONBLOCK),
open: $filename: $!
and next;
local $_;
You need to then test the resulting file handle to verify it was a
normal file. I think just
nag !-f $fh
?
Philip
Re: Patch to add -f flag to cat(1)
Sevan Janiyan wrote: The feature was actually added to ensure whatever cat was meant to be reading from was indeed a plain file and not another which could block a process. Use cat -f to avoid denial of service attacks by people who make .rhosts files fifos. http://mail-index.netbsd.org/source-changes/2000/01/14/0069.html hmm, well, security(8) in openbsd is a perl script that doesn't exec cat, so this wouldn't help solve that problem. now, looking at security, it seems there may be an issue if it tries to open a blocking file, but that will need solving there, not in cat.
Re: Patch to add -f flag to cat(1)
Sevan Janiyan wrote: On 19/07/2015 15:35, Bob Beck wrote: The place to solve this is in whatever is using cat for this purpose. check for the file type before blindly cat'ing. Understood both your Ted's explanation regarding cat. Just so it's crisp clear, ignoring cat(1), having such a flag in fopen(2) is not of interest for use elsewhere either? I could maybe be convinced. However, fopen is the C standard stdio function. One reason you may be using stdio is because you want portability, so adding nonportable extensions to it seems counter productive. If you need to know about all sorts of fifos vs sockets vs files, there are a variety of posix APIs for that, portable to all posix systems.
Re: Patch to add -f flag to cat(1)
Hi, Ted Unangst wrote on Sun, Jul 19, 2015 at 10:26:19AM -0400: Sevan Janiyan wrote: The feature was actually added to ensure whatever cat was meant to be reading from was indeed a plain file and not another which could block a process. Use cat -f to avoid denial of service attacks by people who make .rhosts files fifos. http://mail-index.netbsd.org/source-changes/2000/01/14/0069.html hmm, well, security(8) in openbsd is a perl script that doesn't exec cat, so this wouldn't help solve that problem. now, looking at security, it seems there may be an issue if it tries to open a blocking file, but that will need solving there, not in cat. I don't think we are vulnerable. If my analysis is accurate, the only user-controlled files we open in security(8) are ~/.rhosts and ~/.shosts in check_rhosts_content(). However, there is next unless -s $filename; right before the open(), and for fifos, -s returns false: Both test(1) and perl(1) consider fifos zero-length, so security(8) won't attempt to open such fifos. I confirmed that by creating such a fifo in my home directory, and security(8) did not hang, but it did complain about some permissions on the fifo. So, i don't see any need for action at this point. (Andrew, do you agree?) Yours, Ingo
Re: Patch to add -f flag to cat(1)
On Fri, Jul 17, 2015 at 8:07 PM, Sevan Janiyan ventur...@geeklan.co.uk wrote: Attached is a patch to add the -f flag to cat(1). -f ensures that cat is opening a regular file in non blocking mode aborts otherwise. Obtained from NetBSD src/bin/cat/cat.c r1.22 r1.34 You have in mind a place where this would be used? Where are there bugs that this would resolve? Philip Guenther
Patch to add -f flag to cat(1)
Hi,
Attached is a patch to add the -f flag to cat(1).
-f ensures that cat is opening a regular file in non blocking mode
aborts otherwise.
Obtained from NetBSD src/bin/cat/cat.c r1.22 r1.34
Sevan Janiyan
From NetBSD
cat.c r1.22, r1.34
cat.1 r1.18, r1.25
Index: bin/cat/cat.c
===
RCS file: /cvs/src/bin/cat/cat.c,v
retrieving revision 1.21
diff -u -r1.21 cat.c
--- bin/cat/cat.c 16 Jan 2015 06:39:28 - 1.21
+++ bin/cat/cat.c 18 Jul 2015 01:45:14 -
@@ -50,7 +50,7 @@
extern char *__progname;
-int bflag, eflag, nflag, sflag, tflag, vflag;
+int bflag, eflag, fflag, nflag, sflag, tflag, vflag;
int rval;
char *filename;
@@ -66,7 +66,7 @@
setlocale(LC_ALL, );
- while ((ch = getopt(argc, argv, benstuv)) != -1)
+ while ((ch = getopt(argc, argv, befnstuv)) != -1)
switch (ch) {
case 'b':
bflag = nflag = 1; /* -b implies -n */
@@ -74,6 +74,9 @@
case 'e':
eflag = vflag = 1; /* -e implies -v */
break;
+ case 'f':
+ fflag = 1;
+ break;
case 'n':
nflag = 1;
break;
@@ -91,7 +94,7 @@
break;
default:
(void)fprintf(stderr,
- usage: %s [-benstuv] [file ...]\n, __progname);
+ usage: %s [-befnstuv] [file ...]\n, __progname);
exit(1);
/* NOTREACHED */
}
@@ -118,7 +121,7 @@
if (*argv) {
if (!strcmp(*argv, -))
fp = stdin;
- else if ((fp = fopen(*argv, r)) == NULL) {
+ else if ((fp = fopen(*argv, rf)) == NULL) {
warn(%s, *argv);
rval = 1;
++argv;
@@ -202,8 +205,26 @@
if (*argv) {
if (!strcmp(*argv, -))
fd = fileno(stdin);
+ else if (fflag) {
+ struct stat st;
+ fd = open(*argv, O_RDONLY|O_NONBLOCK, 0);
+ if (fd 0)
+ goto skip;
+
+ if (fstat(fd, st) == -1) {
+ close(fd);
+ goto skip;
+ }
+ if (!S_ISREG(st.st_mode)) {
+ close(fd);
+ warnx(%s: not a regular file, *argv);
+ goto skipnomsg;
+ }
+ }
else if ((fd = open(*argv, O_RDONLY, 0)) 0) {
+skip:
warn(%s, *argv);
+skipnomsg:
rval = 1;
++argv;
continue;
Index: bin/cat/cat.1
===
RCS file: /cvs/src/bin/cat/cat.1,v
retrieving revision 1.34
diff -u -r1.34 cat.1
--- bin/cat/cat.1 15 Jan 2015 19:06:31 - 1.34
+++ bin/cat/cat.1 18 Jul 2015 02:02:23 -
@@ -33,7 +33,7 @@
.\
.\ @(#)cat.1 8.3 (Berkeley) 5/2/95
.\
-.Dd $Mdocdate: January 15 2015 $
+.Dd $Mdocdate: July 18 2015 $
.Dt CAT 1
.Os
.Sh NAME
@@ -41,7 +41,7 @@
.Nd concatenate and print files
.Sh SYNOPSIS
.Nm cat
-.Op Fl benstuv
+.Op Fl befnstuv
.Op Ar
.Sh DESCRIPTION
The
@@ -70,6 +70,8 @@
option and also prints a dollar sign
.Pq Ql \$
at the end of each line.
+.It Fl f
+Only attempt to display regular files.
.It Fl n
Number the output lines, starting at 1.
.It Fl s
