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

Bleadperl v5.27.4-48-g0cbfaef69b breaks ZEFRAM/Module-Runtime-0.015.tar.gz #16187

Closed
p5pRT opened this issue Oct 6, 2017 · 10 comments
Closed

Bleadperl v5.27.4-48-g0cbfaef69b breaks ZEFRAM/Module-Runtime-0.015.tar.gz #16187

p5pRT opened this issue Oct 6, 2017 · 10 comments
Labels
BBC

Comments

@p5pRT
Copy link

@p5pRT p5pRT commented Oct 6, 2017

Migrated from rt.perl.org#132235 (status was 'resolved')

Searchable as RT132235$

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Oct 6, 2017

From @andk

bisect


commit 0cbfaef
Author​: Nicolas R <atoomic@​cpan.org>
Date​: Tue Sep 26 18​:07​:47 2017 -0500

  pp_require​: return earlier when module is already loaded

diagnostics


http​://www.cpantesters.org/cpan/report/992841a8-a9f3-11e7-bbbb-db5a96210b79

perl -V


Summary of my perl5 (revision 5 version 27 subversion 5) configuration​:
  Commit id​: 93309f6
  Platform​:
  osname=linux
  osvers=4.12.0-2-amd64
  archname=x86_64-linux-ld
  uname='linux k93msid 4.12.0-2-amd64 #1 smp debian 4.12.13-1 (2017-09-19) x86_64 gnulinux '
  config_args='-Dprefix=/home/sand/src/perl/repoperls/installed-perls/host/k93msid/v5.27.4-49-g93309f6b79/ea6b -Dmyhostname=k93msid -Dinstallusrbinperl=n -Uversiononly -Dusedevel -des -Ui_db -Dlibswanted=cl pthread socket inet nsl gdbm dbm malloc dl ld sun m crypt sec util c cposix posix ucb BSD gdbm_compat -Uuseithreads -Duselongdouble -DEBUGGING=none'
  hint=recommended
  useposix=true
  d_sigaction=define
  useithreads=undef
  usemultiplicity=undef
  use64bitint=define
  use64bitall=define
  uselongdouble=define
  usemymalloc=n
  default_inc_excludes_dot=define
  bincompat5005=undef
  Compiler​:
  cc='cc'
  ccflags ='-fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_FORTIFY_SOURCE=2'
  optimize='-O2'
  cppflags='-fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include'
  ccversion=''
  gccversion='7.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='long double'
  nvsize=16
  Off_t='off_t'
  lseeksize=8
  alignbytes=16
  prototype=define
  Linker and Libraries​:
  ld='cc'
  ldflags =' -fstack-protector-strong -L/usr/local/lib'
  libpth=/usr/local/lib /usr/lib/gcc/x86_64-linux-gnu/7/include-fixed /usr/include/x86_64-linux-gnu /usr/lib /lib/x86_64-linux-gnu /lib/../lib /usr/lib/x86_64-linux-gnu /usr/lib/../lib /lib
  libs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
  perllibs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
  libc=libc-2.24.so
  so=so
  useshrplib=false
  libperl=libperl.a
  gnulibc_version='2.24'
  Dynamic Linking​:
  dlsrc=dl_dlopen.xs
  dlext=so
  d_dlsymun=undef
  ccdlflags='-Wl,-E'
  cccdlflags='-fPIC'
  lddlflags='-shared -O2 -L/usr/local/lib -fstack-protector-strong'

Characteristics of this binary (from libperl)​:
  Compile-time options​:
  HAS_TIMES
  PERLIO_LAYERS
  PERL_COPY_ON_WRITE
  PERL_DONT_CREATE_GVSV
  PERL_MALLOC_WRAP
  PERL_OP_PARENT
  PERL_PRESERVE_IVUV
  PERL_USE_DEVEL
  USE_64_BIT_ALL
  USE_64_BIT_INT
  USE_LARGE_FILES
  USE_LOCALE
  USE_LOCALE_COLLATE
  USE_LOCALE_CTYPE
  USE_LOCALE_NUMERIC
  USE_LOCALE_TIME
  USE_LONG_DOUBLE
  USE_PERLIO
  USE_PERL_ATOF
  Built under linux
  Compiled at Oct 5 2017 06​:45​:11
  %ENV​:
  PERL5LIB=""
  PERL5OPT=""
  PERL5_CPANPLUS_IS_RUNNING="24716"
  PERL5_CPAN_IS_RUNNING="24716"
  PERL_CANARY_STABILITY_NOPROMPT="1"
  PERL_MM_USE_DEFAULT="1"
  PERL_USE_UNSAFE_INC="1"
  @​INC​:
  /home/sand/src/perl/repoperls/installed-perls/host/k93msid/v5.27.4-49-g93309f6b79/ea6b/lib/site_perl/5.27.5/x86_64-linux-ld
  /home/sand/src/perl/repoperls/installed-perls/host/k93msid/v5.27.4-49-g93309f6b79/ea6b/lib/site_perl/5.27.5
  /home/sand/src/perl/repoperls/installed-perls/host/k93msid/v5.27.4-49-g93309f6b79/ea6b/lib/5.27.5/x86_64-linux-ld
  /home/sand/src/perl/repoperls/installed-perls/host/k93msid/v5.27.4-49-g93309f6b79/ea6b/lib/5.27.5
  .

--
andreas

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Oct 7, 2017

From zefram@fysh.org

The test failures are concerned with tainting. The tests are checking
that requiring a module through a tainted name yields a taint exception.
The tests are written to require a module that's already loaded​: not
for any reason specific to the semantics of already-loaded modules,
but merely because it's a convenient module to refer to. The test
implicitly assumes that the tainting semantics will be the same between
modules that are already loaded and those that are not.

Until now that was the case, and the attempt to reload with a tainted
module name produced an exception. The bisected commit moves require's
check of %INC earlier, such that it now precedes the taint check.
So requiring an already-loaded module now succeeds even if the module
name is tainted, and so M​:R's test fails. Taint checking still works
when requiring a module that is not already loaded.

The question here is whether requiring a module is an insecure action, for
taint purposes, even if the module is already loaded. It's difficult to
frame that question in an unbiased way​: if we express it as "is requiring
a module an insecure action?" then clearly the answer is yes, but if
we express it as "is requiring an already-loaded module an insecure
action?" then the answer is probably no. Which way do we demarcate
actions for tainting purposes? I think we tend to look at the actual
data involved, rather than make categorical judgements, which would lead
to the conclusion that the new taint behaviour is acceptable and that
M​:R's tests need to change.

-zefram

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Oct 7, 2017

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

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Oct 7, 2017

From @iabyn

On Sat, Oct 07, 2017 at 08​:57​:37AM +0100, Zefram wrote​:

The question here is whether requiring a module is an insecure action, for
taint purposes, even if the module is already loaded.

It's a bit of a stretch, but if - due to a code flaw - an attacker can
change the name of a module to be loaded, then they can change the name
to an-already loaded module, and so force an intended module *not* to be
loaded. Where not loading that module decreases the security of the code.

For example, instead of loading, "Foo​::Audit", it could instead force the
(re)require of 'strict' instead, and continue without error, and without
auditing enabled.

--
I don't want to achieve immortality through my work...
I want to achieve it through not dying.
  -- Woody Allen

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Oct 10, 2017

From @xsawyerx

On 10/07/2017 06​:10 PM, Dave Mitchell wrote​:

On Sat, Oct 07, 2017 at 08​:57​:37AM +0100, Zefram wrote​:

The question here is whether requiring a module is an insecure action, for
taint purposes, even if the module is already loaded.
It's a bit of a stretch, but if - due to a code flaw - an attacker can
change the name of a module to be loaded, then they can change the name
to an-already loaded module, and so force an intended module *not* to be
loaded. Where not loading that module decreases the security of the code.

For example, instead of loading, "Foo​::Audit", it could instead force the
(re)require of 'strict' instead, and continue without error, and without
auditing enabled.

I have a few questions and apologies if they all seem uninformed.

* Does it seem reasonable to audit the code to verify (to some guarantee
or level of comfort) that it is not possible (or unlikely) to change the
name of a module to be reloaded?
* Can we try and hack it? Attempt to achieve the example (or one of
similar effect) raised above?
* Otherwise, would it be reasonable to use the location of said module
instead of name for the lookup? I'm not sure this would make a
difference. Maybe worse.
* Adding a TAINT_PROPER(op_name) on the condition for already-loaded
module. Considering it was run every time prior to this, I'm assuming it
wouldn't add a noticeable difference.

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Oct 17, 2017

From zefram@fysh.org

Since there's been no move to revert the require tainting change for
5.27.5, I've released a new version of Module-Runtime that works under
the new tainting regime.

-zefram

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Feb 6, 2018

From @jkeenan

On Tue, 17 Oct 2017 21​:45​:29 GMT, zefram@​fysh.org wrote​:

Since there's been no move to revert the require tainting change for
5.27.5, I've released a new version of Module-Runtime that works under
the new tainting regime.

-zefram

It installs on blead. Resolving ticket.

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

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Feb 6, 2018

@jkeenan - Status changed from 'open' to 'pending release'

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Jun 23, 2018

From @khwilliamson

Thank you for filing this report. You have helped make Perl better.

With the release yesterday of Perl 5.28.0, this and 185 other issues have been
resolved.

Perl 5.28.0 may be downloaded via​:
https://metacpan.org/release/XSAWYERX/perl-5.28.0

If you find that the problem persists, feel free to reopen this ticket.

@p5pRT p5pRT closed this as completed Jun 23, 2018
@p5pRT
Copy link
Author

@p5pRT p5pRT commented Jun 23, 2018

@khwilliamson - Status changed from 'pending release' to 'resolved'

@p5pRT p5pRT added BBC Severity Low labels Oct 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BBC
Projects
None yet
Development

No branches or pull requests

1 participant