Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.92k stars 551 forks source link

wrongly-cased module import appears to succeed, but does not import any symbols (cygwin/ntfs) #19633

Open jeremyhetzler opened 2 years ago

jeremyhetzler commented 2 years ago

This is a bug report for perl from generated with the help of perlbug 1.42 running under perl 5.32.1.


[Please describe your issue here]

On Cygwin, on NTFS, a wrongly-cased "use Module" import will appear to succeed, but will not actually import any symbols.

Example:

$ /bin/perl -we'use data::dumper; print Dumper(5);' Undefined subroutine &main::Dumper called at -e line 1.

What should happen is that the import should fail during BEGIN, as in any other scenario where the module name is misspelled.

[Please do not change anything below this line]


Flags: category=core severity=low

Site configuration information for perl 5.32.1:

Configured by ASSI at Sat Aug 14 09:03:17 CEST 2021.

Summary of my perl5 (revision 5 version 32 subversion 1) configuration:

Platform: osname=cygwin osvers=3.2.0(0.34053) archname=x86_64-cygwin-threads-multi uname='cygwin_nt-10.0 cygwinpro 3.2.0(0.34053) 2021-03-29 08:42 x86_64 cygwin ' config_args='-des -Dprefix=/usr -Dmksymlinks -Darchname=x86_64-cygwin-threads -Dlibperl=cygperl5_32.dll -Dcc=gcc -Dld=g++ -Accflags=-ggdb -O2 -pipe -Wall -Werror=format-security -D_FORTIFY_SOURCE=2 -fstack-protector-strong --param=ssp-buffer-size=4 -fdebug-prefix-map=/mnt/share/cygpkgs/perl/perl.x86_64/build=/usr/src/debug/perl-5.32.1-2 -fdebug-prefix-map=/mnt/share/cygpkgs/perl/perl.x86_64/src/perl-5.32.1=/usr/src/debug/perl-5.32.1-2 -fwrapv' hint=recommended useposix=true d_sigaction=define useithreads=define usemultiplicity=define use64bitint=define use64bitall=define uselongdouble=undef usemymalloc=n default_inc_excludes_dot=define bincompat5005=undef Compiler: cc='gcc' ccflags ='-DPERL_USE_SAFE_PUTENV -USTRICT_ANSI__ -D_GNU_SOURCE -ggdb -O2 -pipe -Wall -Werror=format-security -D_FORTIFY_SOURCE=2 -fstack-protector-strong --param=ssp-buffer-size=4 -fdebug-prefix-map=/mnt/share/cygpkgs/perl/perl.x86_64/build=/usr/src/debug/perl-5.32.1-2 -fdebug-prefix-map=/mnt/share/cygpkgs/perl/perl.x86_64/src/perl-5.32.1=/usr/src/debug/perl-5.32.1-2 -fwrapv -fno-strict-aliasing' optimize='-O3' cppflags='-DPERL_USE_SAFE_PUTENV -USTRICT_ANSI__ -D_GNU_SOURCE -ggdb -O2 -pipe -Wall -Werror=format-security -D_FORTIFY_SOURCE=2 -fstack-protector-strong --param=ssp-buffer-size=4 -fdebug-prefix-map=/mnt/share/cygpkgs/perl/perl.x86_64/build=/usr/src/debug/perl-5.32.1-2 -fdebug-prefix-map=/mnt/share/cygpkgs/perl/perl.x86_64/src/perl-5.32.1=/usr/src/debug/perl-5.32.1-2 -fwrapv -fno-strict-aliasing' ccversion='' gccversion='10.2.0' gccosandvers='' intsize=4 longsize=8 ptrsize=8 doublesize=8 byteorder=12345678 doublekind=3 d_longlong=define longlongsize=8 d_longdbl=define longdblsize=16 longdblkind=3 ivtype='long' ivsize=8 nvtype='double' nvsize=8 Off_t='off_t' lseeksize=8 alignbytes=8 prototype=define Linker and Libraries: ld='g++' ldflags =' -Wl,--enable-auto-import -Wl,--export-all-symbols -Wl,--enable-auto-image-base -fstack-protector-strong' libpth=/usr/lib libs=-lpthread -lnsl -lgdbm -ldb -ldl -lcrypt -lgdbm_compat perllibs=-lpthread -lnsl -ldl -lcrypt libc=/usr/lib/libcygwin.a so=dll useshrplib=true libperl=cygperl5_32.dll gnulibc_version='' Dynamic Linking: dlsrc=dl_dlopen.xs dlext=dll d_dlsymun=undef ccdlflags=' ' cccdlflags=' ' lddlflags=' --shared -Wl,--enable-auto-import -Wl,--export-all-symbols -Wl,--enable-auto-image-base -fstack-protector-strong'

Locally applied patches: Cygwin: README Cygwin: use auto-image-base instead of fixed DLL base address Cygwin: modify hints Cygwin: Configure correct libsearch Cygwin: Configure correct libpth Cygwin: Encode fix for CVE-2021-36770 Cygwin: Win32 correct UTF8 handling


@INC for perl 5.32.1: /home/jhetzler/lib/perl /usr/local/libperl /usr/local/lib/perl5/site_perl/5.32/x86_64-cygwin-threads /usr/local/share/perl5/site_perl/5.32 /usr/lib/perl5/vendor_perl/5.32/x86_64-cygwin-threads /usr/share/perl5/vendor_perl/5.32 /usr/lib/perl5/5.32/x86_64-cygwin-threads /usr/share/perl5/5.32


Environment for perl 5.32.1: HOME=/home/jhetzler LANG=C.UTF-8 LANGUAGE (unset) LC_COLLATE=C LC_CTYPE=C.UTF-8 LD_LIBRARY_PATH (unset) LOGDIR=/home/jhetzler/log PATH=/usr/local/bin:/usr/bin:/cygdrive/c/Windows/system32:/cygdrive/c/Windows:/cygdrive/c/Windows/System32/Wbem:/cygdrive/c/Windows/System32/WindowsPowerShell/v1.0:/cygdrive/c/Windows/System32/OpenSSH:/cygdrive/c/Program Files/dotnet:/cygdrive/c/Users/jhetzler/AppData/Local/Microsoft/WindowsApps:/cygdrive/c/users/jhetzler/cyghome/scripts:/usr/games:/cygdrive/c/google_appengine:/cygdrive/c/Windows/System32:/cygdrive/c/Program Files/PSTools PERL5LIB=/home/jhetzler/lib/perl:/usr/local/libperl PERLDOC=-otext PERLIO=perlio PERL_BADLANG (unset) SHELL=/usr/bin/bash

jeremyhetzler commented 2 years ago

Here is a patch against 5.32.1 that fixes most of the problem.

diff --git a/hints/cygwin.sh b/hints/cygwin.sh
index 20e0e58821..e69a6daebd 100644
--- a/hints/cygwin.sh
+++ b/hints/cygwin.sh
@@ -80,3 +80,7 @@ lddlflags="$lddlflags $ldflags"
 #ldflags="$ldflags -s"
 #ccdlflags="$ccdlflags -s"
 #lddlflags="$lddlflags -s"
+
+# add REQUIRE_STRICT_CASE
+ccflags="$ccflags -DREQUIRE_STRICT_CASE"
+
diff --git a/pp_ctl.c b/pp_ctl.c
index 3797ec434e..208b005598 100644
--- a/pp_ctl.c
+++ b/pp_ctl.c
@@ -4212,7 +4212,38 @@ S_require_file(pTHX_ SV *sv)
    }
     }

-    /* at this point we've ether opened a file (tryrsfp) or set errno */
+#ifdef REQUIRE_STRICT_CASE
+#ifdef __CYGWIN__
+
+    if (tryrsfp) {
+        char* last_sep = strrchr(tryname, '/');
+        if (last_sep) {
+            char dirname[MAXPATHLEN];
+            char* basename = last_sep + 1;
+            DIR *dir;
+            Direntry_t *dp;
+            int fl_match=0;
+            my_strlcpy(dirname, tryname, (last_sep-tryname)+1);
+            if (!(dir = PerlDir_open(dirname))) {
+                Perl_croak(aTHX_ "opendir failed: %s", dirname);
+            };
+            while ((dp = PerlDir_read(dir)) != NULL) {
+                if (strEQ(basename, dp->d_name)) {
+                    fl_match=1;
+                    break;
+                };
+            };
+            if (!fl_match) {
+                tryrsfp = NULL;
+                errno = ENOENT;
+            };
+        };
+    };
+
+#endif
+#endif
+
+    /* at this point we've either opened a file (tryrsfp) or set errno */

     saved_errno = errno; /* sv_2mortal can realloc things */
     sv_2mortal(namesv);
demerphq commented 2 years ago

On Mon, 25 Jul 2022 at 06:13, jeremyhetzler @.***> wrote:

Here is a patch against 5.32.1 that fixes most of the problem.

This is one way, perhaps even a good way for your specific case but IMO it isnt a generic solution. I proposed a more generic alternative in a different PR in the last dev cycle. Basically UNIVERSAL::import() should exist and trap these cases so we get a proper error message. Currently WHATEVER::import() is special cased to not throw an error. No matter whether it exists.

Imagine the opposite case on a case sensitive file system. You do:

use Foo;

and in Foo.pm we have:

package foo;

your patch isn't going to throw an error, but the same bug will be at play. If we apply the patch to make UNIVERSAL::import() exist, then we do not have this issue, anything that reaches UNIVERSAL::import() and expects to export something will throw an exception.

cheers, yves

-- perl -Mre=debug -e "/just|another|perl|hacker/"

jeremyhetzler commented 2 years ago

On Wed, Jul 27, 2022 at 12:29 PM Yves Orton @.***> wrote:

On Mon, 25 Jul 2022 at 06:13, jeremyhetzler @.***> wrote:

Here is a patch against 5.32.1 that fixes most of the problem.

This is one way, perhaps even a good way for your specific case but IMO it isnt a generic solution. I proposed a more generic alternative in a different PR in the last dev cycle. Basically UNIVERSAL::import() should exist and trap these cases so we get a proper error message. Currently WHATEVER::import() is special cased to not throw an error. No matter whether it exists.

Imagine the opposite case on a case sensitive file system. You do:

use Foo;

and in Foo.pm we have:

package foo;

your patch isn't going to throw an error, but the same bug will be at play. If we apply the patch to make UNIVERSAL::import() exist, then we do not have this issue, anything that reaches UNIVERSAL::import() and expects to export something will throw an exception.

cheers, yves

Yves,

That sounds like a good solution.

Any hopes of getting your patch accepted?

Thanks, Jeremy

demerphq commented 2 years ago

On Wed, 27 Jul 2022 at 23:16, jeremyhetzler @.***> wrote:

On Wed, Jul 27, 2022 at 12:29 PM Yves Orton @.***> wrote:

On Mon, 25 Jul 2022 at 06:13, jeremyhetzler @.***> wrote:

Here is a patch against 5.32.1 that fixes most of the problem.

This is one way, perhaps even a good way for your specific case but IMO it isnt a generic solution. I proposed a more generic alternative in a different PR in the last dev cycle. Basically UNIVERSAL::import() should exist and trap these cases so we get a proper error message. Currently WHATEVER::import() is special cased to not throw an error. No matter whether it exists.

Imagine the opposite case on a case sensitive file system. You do:

use Foo;

and in Foo.pm we have:

package foo;

your patch isn't going to throw an error, but the same bug will be at play. If we apply the patch to make UNIVERSAL::import() exist, then we do not have this issue, anything that reaches UNIVERSAL::import() and expects to export something will throw an exception.

cheers, yves

Yves,

That sounds like a good solution.

Any hopes of getting your patch accepted

There seemed to be some appetite for it, yes. It got caught up in some other questions and was big enough a change that it was parked until this dev cycle. Ill try to find time to bring it back to life.

Possibly we should do your patch AND my patch. Have you looked into your patches performance implications? Whenever the idea of doing something like you are proposing someone brings up performance implications. It would be nice to get some data to use in that discussion.

cheers, Yves

-- perl -Mre=debug -e "/just|another|perl|hacker/"