On 04/10/16 12:38, Pádraig Brady wrote: > On 04/10/16 03:21, Mohammed Sadiq wrote: >> '--no-preserve-root' that can be used to ignore if the path is root when >> using >> the 'rm' command. >> >> But as the most of the GNU commands accepts shortened flag as long as >> there is no ambiguity, this can be an issue too. So, 'rm --n' may have the >> same effect as 'rm --no-preserve-root'. There may be several users unaware >> of this feature which can cause several issues. >> >> 1. A cracker may be able to trick a user to bring a system down using >> '--n' flag. >> 2. A folder/file name like '--n' as an argument to 'rm' command may >> try to delete >> the whole files (in case a '/' too appears as an argument), and >> the user won't >> find a reason why it happened. >> >> One way to overcome this is set '--no-preserve-roots' too an alias for >> '--no-preserve-root'. This means that the user will have include the whole >> flag >> to ignore root check (shortening will create an ambiguity). > > An interesting idea. > The main focus of the --no-preserve-root option is to protect against > accidental insertion of a space with `rm -rf blah /` or `rm -rf $blah/`. > With malicious arguments though one can obfuscate using shell quoting, > and the recent ls quoting changes are more general protection against that. > In saying that I don't see any issue with this, and there is a slight > increase in protection, so I'd be 60:40 for making this change.
This would break scripts that used shortened --no-preserve for example, though that's quite unlikely to be used. Implementation is attached. Pádraig
>From 338ab457344588b4ca305ae6c658e30afee77162 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A1draig=20Brady?= <p...@draigbrady.com> Date: Tue, 4 Oct 2016 13:43:32 +0100 Subject: [PATCH] rm: disallow --n alias for --no-preserve-root * src/rm.c (LONG_OPTS): Add --no-preserve-root- to make shortened options ambiguous. * tests/rm/r-root.sh: Add a test case. * NEWS: Mention the change in behavior. Fixes http://bugs.gnu.org/24604 --- NEWS | 6 ++++-- src/rm.c | 4 ++++ tests/rm/r-root.sh | 6 ++++++ 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index c3554d0..e68de05 100644 --- a/NEWS +++ b/NEWS @@ -52,6 +52,8 @@ GNU coreutils NEWS -*- outline -*- ** Changes in behavior + rm no longer accepts shortened variants of the --no-preserve-root option. + seq no longer accepts 0 value as increment, and now also rejects NaN values for any argument. @@ -81,8 +83,8 @@ GNU coreutils NEWS -*- outline -*- ** New Features - date now accepts the --debug option, to annotate the parsed date string, - display timezone information, and warn about potential misuse. + date now accepts the --debug option, to annotate the parsed date string, + display timezone information, and warn about potential misuse. * Noteworthy changes in release 8.25 (2016-01-20) [stable] diff --git a/src/rm.c b/src/rm.c index 13a5714..11d1a49 100644 --- a/src/rm.c +++ b/src/rm.c @@ -48,6 +48,7 @@ enum INTERACTIVE_OPTION = CHAR_MAX + 1, ONE_FILE_SYSTEM, NO_PRESERVE_ROOT, + NO_PRESERVE_ROOT_, PRESERVE_ROOT, PRESUME_INPUT_TTY_OPTION }; @@ -66,6 +67,9 @@ static struct option const long_opts[] = {"one-file-system", no_argument, NULL, ONE_FILE_SYSTEM}, {"no-preserve-root", no_argument, NULL, NO_PRESERVE_ROOT}, + /* The following enforces full specification of --no-preserve-root + to reduce the chance of users being tricked into specifying --n. */ + {"no-preserve-root-", no_argument, NULL, NO_PRESERVE_ROOT_}, {"preserve-root", no_argument, NULL, PRESERVE_ROOT}, /* This is solely for testing. Do not document. */ diff --git a/tests/rm/r-root.sh b/tests/rm/r-root.sh index b98db14..c5f0ef4 100755 --- a/tests/rm/r-root.sh +++ b/tests/rm/r-root.sh @@ -211,6 +211,12 @@ for opts in \ done #------------------------------------------------------------------------------- +# Exercise with --no-preserve to ensure shortened equivalent is not allowed. +returns_ 1 exercise_rm_r_root --no-preserve / || fail=1 +grep -F "option '--no-preserve' is ambiguous" err || fail=1 +test -f x && fail=1 + +#------------------------------------------------------------------------------- # Exercise "rm -r file1 / file2". # Expect a non-Zero exit status representing failure to remove "/", # yet 'file1' and 'file2' should be removed. -- 2.5.5
