Deleting a directory with the same name as a shortcut deletes everything in CWD

Deleting a directory with the same name as a shortcut deletes everything in CWD

[Oskar Skog]

[-- Attachment #1.1.1: Type: text/plain, Size: 676 bytes --]

Deleting a directory with the same name as a shortcut deletes everything
in the current working directory.

Tested on:
CYGWIN_NT-10.0-19044 3.3.5-341.x86_64 2022-05-13 12:27 UTC x86_64 Cygwin
CYGWIN_NT-10.0-22000 3.3.5-341.x86_64 2022-05-13 12:27 UTC x86_64 Cygwin

Script to reproduce the bug (also in the tar):

#!/bin/bash
echo 'EVERY FILE IN THE CURRENT WORKING DIRECTORY WILL BE DELETED!'
read -p "Enter 'sure' to continue: " var
echo $var | grep -q sure || exit 1

do_stuff ()
{
     mkdir foo
     touch Foo.lnk
     if [ -d foo ]; then
         rm -rf foo
     fi
}
do_stuff
do_stuff
# All files in the current working directory are now GONE!

[-- Attachment #1.1.2: delete-all-files.tar --]
[-- Type: application/x-tar, Size: 10240 bytes --]

[-- Attachment #1.1.3: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 2485 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 665 bytes --]

Re: Deleting a directory with the same name as a shortcut deletes everything in CWD

[Christian Franke]

Oskar Skog wrote:
> Deleting a directory with the same name as a shortcut deletes everything
> in the current working directory.
>
> Tested on:
> CYGWIN_NT-10.0-19044 3.3.5-341.x86_64 2022-05-13 12:27 UTC x86_64 Cygwin
> CYGWIN_NT-10.0-22000 3.3.5-341.x86_64 2022-05-13 12:27 UTC x86_64 Cygwin
>
> Script to reproduce the bug (also in the tar):
>
> #!/bin/bash
> echo 'EVERY FILE IN THE CURRENT WORKING DIRECTORY WILL BE DELETED!'
> read -p "Enter 'sure' to continue: " var
> echo $var | grep -q sure || exit 1
>
> do_stuff ()
> {
>     mkdir foo
>     touch Foo.lnk
>     if [ -d foo ]; then
>         rm -rf foo
>     fi
> }
> do_stuff
> do_stuff
> # All files in the current working directory are now GONE!

The first "do_stuff" removes "foo" as expected. The second call 
accidentally detects "foo" as a directory because stat("foo", .) returns 
"directory" and opendir("foo") succeeds unexpectedly. This behavior 
recurses.

Testcase:

$ ls -a
.  ..

$ touch link.lnk file.txt

$ ls -a
.  ..  file.txt  link.lnk

$ stat -c %F link.lnk
regular empty file

$ stat -c %F link
directory

$ ls -a link
.  ..  file.txt  link.lnk

$ stat -c %F link/link/link/link
directory

$ ls link/link/link/link
.  ..  file.txt  link.lnk

$ rm link/link/link/link
rm: cannot remove 'link/link/link/link': Is a directory

$ rm -rfv link/link/link/link
removed 'link/link/link/link/file.txt'
removed 'link/link/link/link/link.lnk'

$ ls -a
.  ..


-- 
Regards,
Christian

Re: Deleting a directory with the same name as a shortcut deletes everything in CWD

[Ken Brown]

On 8/8/2022 7:56 AM, Christian Franke wrote:
> Testcase:
> 
> $ ls -a
> .  ..
> 
> $ touch link.lnk file.txt
> 
> $ ls -a
> .  ..  file.txt  link.lnk
> 
> $ stat -c %F link.lnk
> regular empty file
> 
> $ stat -c %F link
> directory

This happens because symlink_info::check returns -1 instead of 0 when called on 
"link".  The main loop over suffixes finds and rejects link.lnk but then leaves 
"res" as -1 when it continues looping.  The following patch fixes the problem:

--- a/winsup/cygwin/path.cc
+++ b/winsup/cygwin/path.cc
@@ -3422,6 +3422,7 @@ restart:
         {
           fileattr = INVALID_FILE_ATTRIBUTES;
           set_error (ENOENT);
+         res = 0;
           continue;
         }

It's possible that a better fix would be to set res = 0 at the beginning of each 
iteration of the loop, but I'll have to study the code more carefully before I'm 
sure of that.  Also, the comment preceding symlink_info::check needs to be fixed 
to correctly indicate what a negative return value means.  I'll look at all this 
more carefully tomorrow if no one beats me to it.

Ken

Re: Deleting a directory with the same name as a shortcut deletes everything in CWD

[Ken Brown]

On 8/8/2022 7:29 PM, Ken Brown wrote:
> On 8/8/2022 7:56 AM, Christian Franke wrote:
>> Testcase:
>>
>> $ ls -a
>> .  ..
>>
>> $ touch link.lnk file.txt
>>
>> $ ls -a
>> .  ..  file.txt  link.lnk
>>
>> $ stat -c %F link.lnk
>> regular empty file
>>
>> $ stat -c %F link
>> directory
> 
> This happens because symlink_info::check returns -1 instead of 0 when called on 
> "link".  The main loop over suffixes finds and rejects link.lnk but then leaves 
> "res" as -1 when it continues looping.  The following patch fixes the problem:
> 
> --- a/winsup/cygwin/path.cc
> +++ b/winsup/cygwin/path.cc
> @@ -3422,6 +3422,7 @@ restart:
>          {
>            fileattr = INVALID_FILE_ATTRIBUTES;
>            set_error (ENOENT);
> +         res = 0;
>            continue;
>          }
> 
> It's possible that a better fix would be to set res = 0 at the beginning of each 
> iteration of the loop, but I'll have to study the code more carefully before I'm 
> sure of that.  Also, the comment preceding symlink_info::check needs to be fixed 
> to correctly indicate what a negative return value means.  I'll look at all this 
> more carefully tomorrow if no one beats me to it.

I think the best fix is to set res = 0 at the beginning of the loop and to 
remove a "res = -1" that occurs later.  I've sent a patch to cygwin-patches.

Ken

Re: Deleting a directory with the same name as a shortcut deletes everything in CWD

[Oskar Skog]

[-- Attachment #1.1.1: Type: text/plain, Size: 651 bytes --]

On 2022-08-09 22:52, Ken Brown wrote:
> I think the best fix is to set res = 0 at the beginning of the loop and 
> to remove a "res = -1" that occurs later.  I've sent a patch to 
> cygwin-patches.
> 
> Ken
> 

 From https://cygwin.com/pipermail/cygwin-patches/2022q3/011994.html
 > Currently it is possible for symlink_info::check to return -1 in case
 > we're searching for foo and find foo.lnk that is not a shortcut.

Does it matter that it is *not* a shortcut for this patch to work?

I first experienced this issue using an actual shortcut, but simplified
the reproduction code.

mkshortcut -n Foo.lnk ...   ->  touch Foo.lnk

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 2485 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 665 bytes --]

Re: Deleting a directory with the same name as a shortcut deletes everything in CWD

[Ken Brown]

On 8/9/2022 4:01 PM, Oskar Skog wrote:
> On 2022-08-09 22:52, Ken Brown wrote:
>> I think the best fix is to set res = 0 at the beginning of the loop and to 
>> remove a "res = -1" that occurs later.  I've sent a patch to cygwin-patches.
>>
>> Ken
>>
> 
>  From https://cygwin.com/pipermail/cygwin-patches/2022q3/011994.html
>  > Currently it is possible for symlink_info::check to return -1 in case
>  > we're searching for foo and find foo.lnk that is not a shortcut.
> 
> Does it matter that it is *not* a shortcut for this patch to work?
> 
> I first experienced this issue using an actual shortcut, but simplified
> the reproduction code.
> 
> mkshortcut -n Foo.lnk ...   ->  touch Foo.lnk

It still works.  I'll fix the commit message.  [The code in question is actually 
checking whether foo.lnk is a shortcut that Cygwin treats as a symlink.]

Ken