Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cygwin::win_to_posix_path() fails, possible memory corruption #16244

Closed
p5pRT opened this issue Nov 14, 2017 · 9 comments
Closed

Cygwin::win_to_posix_path() fails, possible memory corruption #16244

p5pRT opened this issue Nov 14, 2017 · 9 comments

Comments

@p5pRT
Copy link

p5pRT commented Nov 14, 2017

Migrated from rt.perl.org#132443 (status was 'rejected')

Searchable as RT132443$

@p5pRT
Copy link
Author

p5pRT commented Nov 14, 2017

From ln@h0x.de

This is a bug report for perl from ln@​h0x.de,
generated with the help of perlbug 1.40 running under perl 5.26.1.

The Perl-integrated conversion function for Cygwin paths Cygwin​::win_to_posix_path() is broken.
Tested with a path read from a XML file using XML​::DOM, its output is faulty.
Depending on the setting, its output is empty, garbage or aborts with 'Out of memory'.
Most likely, there are some memory allocation issues.

To reproduce, please call script 'test.pl' with attached XML file 'test.xml' as argument.
Depending on used path to file (relative or absolute etc.), different types of failure appear.

+++ test.pl +++
#!/usr/bin/env perl

use XML​::DOM;

print 'XML FILE (argument)​:>' . $ARGV[0] . "<\n";
my $xml_path = Cygwin​::win_to_posix_path($ARGV[0]);
print 'XML FILE (converted)​:>' . $xml_path . "<\n";

my $parser = new XML​::DOM​::Parser();
my $dom_tree = $parser->parsefile($xml_path);
my $attr_path = $dom_tree->getDocumentElement()->getAttribute('attr');
print 'PATH FROM ATTRIBUTE​:>' . $attr_path . "<\n";

my $win_attr_path = Cygwin​::win_to_posix_path($attr_path);
print 'PATH FROM ATTRIBUTE (converted)​:>' . $win_attr_path . "<\n";

+++ test.xml +++
<?xml version="1.0" ?>
<Test attr="/cygdrive/C/Cygwin/usr/bin/ls.exe"></Test>

Please note that in my tests, only the path read from the XML is affected. Direct output of this
file before (broken) conversion shows no abnormality though. Therefore I am not sure if this bug
originates in Perl core or XML​::DOM module.---
Flags​:
  category=core
  severity=high


Site configuration information for perl 5.26.1​:

Configured by ASSI at Tue Sep 26 18​:57​:14 CEST 2017.

Summary of my perl5 (revision 5 version 26 subversion 1) configuration​:
 
  Platform​:
  osname=cygwin
  osvers=2.9.0(0.31853)
  archname=i686-cygwin-threads-64int-multi
  uname='cygwin_nt-6.3-wow cygwin 2.9.0(0.31853) 2017-09-12 10​:41 i686 cygwin '
  config_args='-des -Dprefix=/usr -Dmksymlinks -Darchname=i686-cygwin-threads-64int -Dlibperl=cygperl5_26.dll -Dcc=gcc -Dld=g++ -Accflags=-ggdb -O2 -pipe -Wimplicit-function-declaration -fdebug-prefix-map=/mnt/share/maint/perl.x86/build=/usr/src/debug/perl-5.26.1-1 -fdebug-prefix-map=/mnt/share/maint/perl.x86/src/perl-5.26.1=/usr/src/debug/perl-5.26.1-1 -fwrapv'
  hint=recommended
  useposix=true
  d_sigaction=define
  useithreads=define
  usemultiplicity=define
  use64bitint=define
  use64bitall=undef
  uselongdouble=undef
  usemymalloc=n
  default_inc_excludes_dot=define
  bincompat5005=undef
  Compiler​:
  cc='gcc'
  ccflags ='-DPERL_USE_SAFE_PUTENV -U__STRICT_ANSI__ -D_GNU_SOURCE -ggdb -O2 -pipe -Wimplicit-function-declaration -fdebug-prefix-map=/mnt/share/maint/perl.x86/build=/usr/src/debug/perl-5.26.1-1 -fdebug-prefix-map=/mnt/share/maint/perl.x86/src/perl-5.26.1=/usr/src/debug/perl-5.26.1-1 -fwrapv -fno-strict-aliasing -fstack-protector-strong -D_FORTIFY_SOURCE=2'
  optimize='-O3'
  cppflags='-DPERL_USE_SAFE_PUTENV -U__STRICT_ANSI__ -D_GNU_SOURCE -ggdb -O2 -pipe -Wimplicit-function-declaration -fdebug-prefix-map=/mnt/share/maint/perl.x86/build=/usr/src/debug/perl-5.26.1-1 -fdebug-prefix-map=/mnt/share/maint/perl.x86/src/perl-5.26.1=/usr/src/debug/perl-5.26.1-1 -fwrapv -fno-strict-aliasing -fstack-protector-strong'
  ccversion=''
  gccversion='6.4.0'
  gccosandvers=''
  intsize=4
  longsize=4
  ptrsize=4
  doublesize=8
  byteorder=12345678
  doublekind=3
  d_longlong=define
  longlongsize=8
  d_longdbl=define
  longdblsize=12
  longdblkind=3
  ivtype='long 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 -lgdbm -ldb -ldl -lcrypt -lgdbm_compat
  perllibs=-lpthread -ldl -lcrypt
  libc=/usr/lib/libcygwin.a
  so=dll
  useshrplib=true
  libperl=cygperl5_26.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​: Win32 correct UTF8 handling
  Perl​: File-Path-2.14 (fixes CVE2017-6512)


@​INC for perl 5.26.1​:
  /usr/local/lib/perl5/site_perl/5.26/i686-cygwin-threads-64int
  /usr/local/share/perl5/site_perl/5.26
  /usr/lib/perl5/vendor_perl/5.26/i686-cygwin-threads-64int
  /usr/share/perl5/vendor_perl/5.26
  /usr/lib/perl5/5.26/i686-cygwin-threads-64int
  /usr/share/perl5/5.26


Environment for perl 5.26.1​:
  HOME=/home/ln
  LANG=en_US.UTF-8
  LANGUAGE (unset)
  LD_LIBRARY_PATH (unset)
  LOGDIR (unset)
  PATH=/usr/local/bin​:/usr/bin​:/cygdrive/c/ProgramData/Oracle/Java/javapath​:/cygdrive/c/Windows/system32​:/cygdrive/c/Windows​:/cygdrive/c/Windows/System32/Wbem​:/cygdrive/c/Windows/System32/WindowsPowerShell/v1.0​:/cygdrive/c/Program Files/Git/cmd​:/cygdrive/c/Program Files/TortoiseGit/bin
  PERL_BADLANG (unset)
  SHELL=/bin/bash

@p5pRT
Copy link
Author

p5pRT commented Nov 15, 2017

From @jkeenan

On Tue, 14 Nov 2017 12​:52​:03 GMT, ln@​h0x.de wrote​:

This is a bug report for perl from ln@​h0x.de,
generated with the help of perlbug 1.40 running under perl 5.26.1.

The Perl-integrated conversion function for Cygwin paths
Cygwin​::win_to_posix_path() is broken.
Tested with a path read from a XML file using XML​::DOM, its output is
faulty.
Depending on the setting, its output is empty, garbage or aborts with
'Out of memory'.
Most likely, there are some memory allocation issues.

To reproduce, please call script 'test.pl' with attached XML file
'test.xml' as argument.
Depending on used path to file (relative or absolute etc.), different
types of failure appear.

+++ test.pl +++
#!/usr/bin/env perl

use XML​::DOM;

print 'XML FILE (argument)​:>' . $ARGV[0] . "<\n";
my $xml_path = Cygwin​::win_to_posix_path($ARGV[0]);
print 'XML FILE (converted)​:>' . $xml_path . "<\n";

my $parser = new XML​::DOM​::Parser();
my $dom_tree = $parser->parsefile($xml_path);
my $attr_path = $dom_tree->getDocumentElement()->getAttribute('attr');
print 'PATH FROM ATTRIBUTE​:>' . $attr_path . "<\n";

my $win_attr_path = Cygwin​::win_to_posix_path($attr_path);
print 'PATH FROM ATTRIBUTE (converted)​:>' . $win_attr_path . "<\n";

+++ test.xml +++
<?xml version="1.0" ?>
<Test attr="/cygdrive/C/Cygwin/usr/bin/ls.exe"></Test>

Please note that in my tests, only the path read from the XML is
affected. Direct output of this
file before (broken) conversion shows no abnormality though. Therefore
I am not sure if this bug
originates in Perl core or XML​::DOM module.---

I recommend that you first discuss this problem with the maintainer of XML​::DOM. You can file a bug report at https://rt.cpan.org/Dist/Display.html?Status=Active;Queue=XML-DOM.

Thank you very much.

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Nov 15, 2017

The RT System itself - Status changed from 'new' to 'open'

@p5pRT
Copy link
Author

p5pRT commented Nov 16, 2017

From zefram@fysh.org

Lando Nachtmann wrote​:

The Perl-integrated conversion function for Cygwin paths Cygwin​::win_to_posix_path() is broken.
Tested with a path read from a XML file using XML​::DOM, its output is faulty.

You should narrow this problem down to one of those modules or the other.
If the Cygwin module is faulty, you can demonstrate it without using
XML​::DOM. But neither of those modules is maintained as part of the
Perl core, so this is the wrong place to report a problem. You should
report the problem to the bugtracker of whichever module is faulty,
and without referring to the other module.

-zefram

@p5pRT
Copy link
Author

p5pRT commented Nov 16, 2017

@iabyn - Status changed from 'open' to 'rejected'

@p5pRT p5pRT closed this as completed Nov 16, 2017
@p5pRT
Copy link
Author

p5pRT commented Nov 16, 2017

From Stromeko@nexgo.de

Lando Nachtmann writes​:

The Perl-integrated conversion function for Cygwin paths
Cygwin​::win_to_posix_path() is broken.

I'd say it behaves exactly as documented, though not as you expected.
Its documentation says it won't convert "double byte characters" (due to
the Win32 interface it uses behind the scenes) and XML​::DOM seems to
feed it UTF-8, which Perl dutifully hands in as argument. Your example
works if you downgrade the string before feeding it to the conversion
function since it doesn't use any code point above 255. That conversion
routine goes back several years and Cygwin versiosn and should probably
be upgraded to use newer interfaces that can properly deal with UTF in
path names plus it also seems to need an overhaul in recognizing what
kind of string it gets fed and warn as needed instead of producing
random errors or no result.

Regards,
Achim.
--
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Factory and User Sound Singles for Waldorf Blofeld​:
http​://Synth.Stromeko.net/Downloads.html#WaldorfSounds

@p5pRT
Copy link
Author

p5pRT commented Nov 16, 2017

From ln@h0x.de

"Zefram via RT" wrote​:

But neither of those modules is maintained as part of the
Perl core, so this is the wrong place to report a problem. You should
report the problem to the bugtracker of whichever module is faulty,
and without referring to the other module.

Ok, thank you, I will do that.
If you do not consider the Cygwin helper methods to be 'Core' despite being in the Perl sources, I suggest you should state so somewhere. E.g. in the respective Perl documentation (https://perldoc.perl.org/perlcygwin.html) or at least in the Perl source code itself (cygwin/cygwin.c).

@p5pRT
Copy link
Author

p5pRT commented Nov 17, 2017

From @khwilliamson

On 11/16/2017 09​:23 AM, Lando Nachtmann via perl5-porters wrote​:

"Zefram via RT" wrote​:

But neither of those modules is maintained as part of the
Perl core, so this is the wrong place to report a problem. You should
report the problem to the bugtracker of whichever module is faulty,
and without referring to the other module.

Ok, thank you, I will do that.
If you do not consider the Cygwin helper methods to be 'Core' despite being in the Perl sources, I suggest you should state so somewhere. E.g. in the respective Perl documentation (https://perldoc.perl.org/perlcygwin.html) or at least in the Perl source code itself (cygwin/cygwin.c).

The cygwin/cygwin.c file is Core.

@p5pRT
Copy link
Author

p5pRT commented Nov 17, 2017

From ln@h0x.de

Ah! Thank you.
I was trying out different encodings like ASCII or windows-1252 on the XML input side, but XML​::DOM seems to output wide characters anyhow. Down-conversion to one byte is fine for our purposes.

16. November 2017 20​:22, "Achim Gratz via RT" <perlbug-followup@​perl.org> schrieb​:

Lando Nachtmann writes​:

The Perl-integrated conversion function for Cygwin paths
Cygwin​::win_to_posix_path() is broken.

I'd say it behaves exactly as documented, though not as you expected.
Its documentation says it won't convert "double byte characters" (due to
the Win32 interface it uses behind the scenes) and XML​::DOM seems to
feed it UTF-8, which Perl dutifully hands in as argument. Your example
works if you downgrade the string before feeding it to the conversion
function since it doesn't use any code point above 255. That conversion
routine goes back several years and Cygwin versiosn and should probably
be upgraded to use newer interfaces that can properly deal with UTF in
path names plus it also seems to need an overhaul in recognizing what
kind of string it gets fed and warn as needed instead of producing
random errors or no result.

Regards,
Achim.
--
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Factory and User Sound Singles for Waldorf Blofeld​:
http​://Synth.Stromeko.net/Downloads.html#WaldorfSounds

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant