[Patch] No warnings for `rm /*`

[Patch] No warnings for `rm /*`

[Glenn Smith]

[-- Attachment #1.1: Type: text/plain, Size: 726 bytes --]

Hi all,
I was reading a reddit thread
<https://www.reddit.com/r/ProgrammerHumor/comments/4ghiac/saw_this_on_my_way_to_uni_command_line_russian/d2hp52s>
today about shells protecting you from `rm -rf /` but not `rm -rf /*` and
remembered how zsh has a feature that warns you before executing `rm *` or
`rm /dir/*`. After a short investigation (and the loss of a few files, but
that's beside the point), I discovered that zsh warns in every case except
`rm /*`.

This seemed like a serious oversight, so I cloned the repository and made a
small patch that should fix this. You can find it attached to this message.
I hope you accept this patch, as this is a simple fix that can save many
people a lot of trouble.

Thanks,
Glenn

[-- Attachment #1.2: Type: text/html, Size: 832 bytes --]

[-- Attachment #2: fix-rm-slash-star.diff --]
[-- Type: text/plain, Size: 771 bytes --]

diff --git a/Src/exec.c b/Src/exec.c
index 50eff72..3f0a141 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -2902,13 +2902,23 @@ execcmd(Estate state, int input, int output, int how, int last1)
 	    if (s[0] == Star && !s[1]) {
 		if (!checkrmall(pwd))
 		    uremnode(args, node);
-	    } else if (l > 2 && s[l - 2] == '/' && s[l - 1] == Star) {
-		char t = s[l - 2];
-
-		s[l - 2] = 0;
+	    } else if (l >= 2 && s[l - 2] == '/' && s[l - 1] == Star) {
+		char t;
+		if (l > 2) {
+		    t = s[l - 2];
+		    s[l - 2] = 0;
+		} else {
+		    t = s[l - 1];
+		    s[l - 1] = 0;
+		}
 		if (!checkrmall(s))
 		    uremnode(args, node);
-		s[l - 2] = t;
+
+		if (l > 2) {
+		    s[l - 2] = t;
+		} else {
+		    s[l - 1] = t;
+		}
 	    }
 	}
 	if (!nextnode(firstnode(args)))

Re: [Patch] No warnings for `rm /*`

[Bart Schaefer]

On Apr 26, 10:31am, Glenn Smith wrote:
} Subject: [Patch] No warnings for `rm /*`
}
} I discovered that zsh warns in every case except `rm /*`.

Thanks.  I think the following simpler patch does the same.

diff --git a/Src/exec.c b/Src/exec.c
index 50eff72..2dcd5bc 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -2902,11 +2902,11 @@ execcmd(Estate state, int input, int output, int how, int last1)
 	    if (s[0] == Star && !s[1]) {
 		if (!checkrmall(pwd))
 		    uremnode(args, node);
-	    } else if (l > 2 && s[l - 2] == '/' && s[l - 1] == Star) {
+	    } else if (l >= 2 && s[l - 2] == '/' && s[l - 1] == Star) {
 		char t = s[l - 2];
 
 		s[l - 2] = 0;
-		if (!checkrmall(s))
+		if (!checkrmall(*s ? s : "/"))
 		    uremnode(args, node);
 		s[l - 2] = t;
 	    }


As an aside:

% rm /nonexistentdir/*
zsh: sure you want to delete all the files in /nonexistentdir [yn]? 

Seems like we could detect when opendir() fails with ENOENT and skip
the prompt, but maybe there are cases of race condition (directory is
created after I type "rm ..." but before /bin/rm is executed?) where
the default answer of "y" would be incorrect.

Re: [Patch] No warnings for `rm /*`

[Mikael Magnusson]

On Tue, Apr 26, 2016 at 7:36 PM, Bart Schaefer
<schaefer@brasslantern.com> wrote:
> On Apr 26, 10:31am, Glenn Smith wrote:
> } Subject: [Patch] No warnings for `rm /*`
> }
> } I discovered that zsh warns in every case except `rm /*`.
>
> Thanks.  I think the following simpler patch does the same.
>
> diff --git a/Src/exec.c b/Src/exec.c
> index 50eff72..2dcd5bc 100644
> --- a/Src/exec.c
> +++ b/Src/exec.c
> @@ -2902,11 +2902,11 @@ execcmd(Estate state, int input, int output, int how, int last1)
>             if (s[0] == Star && !s[1]) {
>                 if (!checkrmall(pwd))
>                     uremnode(args, node);
> -           } else if (l > 2 && s[l - 2] == '/' && s[l - 1] == Star) {
> +           } else if (l >= 2 && s[l - 2] == '/' && s[l - 1] == Star) {
>                 char t = s[l - 2];
>
>                 s[l - 2] = 0;
> -               if (!checkrmall(s))
> +               if (!checkrmall(*s ? s : "/"))
>                     uremnode(args, node);
>                 s[l - 2] = t;
>             }
>
>
> As an aside:
>
> % rm /nonexistentdir/*
> zsh: sure you want to delete all the files in /nonexistentdir [yn]?
>
> Seems like we could detect when opendir() fails with ENOENT and skip
> the prompt, but maybe there are cases of race condition (directory is
> created after I type "rm ..." but before /bin/rm is executed?) where
> the default answer of "y" would be incorrect.

This is pretty contrived, but it's not even a race condition.

% mkdir /tmp/r; rm /tmp/r/.(e:'echo hello; touch $REPLY/foo:') /tmp/r/*
zsh: sure you want to delete all the files in /tmp/r [yn]? y
hello
rm: cannot remove '/tmp/r/.': Is a directory
removed '/tmp/r/foo'

-- 
Mikael Magnusson