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

Segfault in S_regmatch from bad backreference #13210

Closed
p5pRT opened this issue Aug 28, 2013 · 5 comments
Closed

Segfault in S_regmatch from bad backreference #13210

p5pRT opened this issue Aug 28, 2013 · 5 comments

Comments

@p5pRT
Copy link

p5pRT commented Aug 28, 2013

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

Searchable as RT119505$

@p5pRT
Copy link
Author

p5pRT commented Aug 28, 2013

From andrewn@locus.net

Created by andrewn@locus.net

$ ./perl -e '/\7777777777/'
Segmentation fault

This is caused by a negative backreference in the compiled regex when the
following code returns a negative number​:

regcomp.c​:10690​: num = atoi(RExC_parse);

This bug was discovered in our production system running perl-5.8.8-40.el5_9
(CentOS5), confirmed on a developer's ActivePerl 5.16.2 (Windows7), and
debugged/reported on the current git.

The following patch does not properly correct the parsing of large integers,
but it should at least die cleanly instead of segfaulting​:

Inline Patch
diff --git a/regcomp.c b/regcomp.c
index 5a1e234..3854569 100644
--- a/regcomp.c
+++ b/regcomp.c
@@ -10688,6 +10688,9 @@ tryagain:
 		        goto parse_named_seq;
 		}   }
 		num = atoi(RExC_parse);
+                if(num < 0) {
+                    vFAIL("Integer wrapped?");
+                }
 		if (isg && num == 0) {
 	            if (*RExC_parse == '0') {
                         vFAIL("Reference to invalid group 0");
Perl Info

Flags:
    category=core
    severity=medium

Site configuration information for perl 5.19.4:

Configured by andrewn at Wed Aug 28 15:05:31 EDT 2013.

Summary of my perl5 (revision 5 version 19 subversion 4) configuration:
  Commit id: 5b903226e771166eeb3a78d139181139a3759c2d
  Platform:
    osname=linux, osvers=3.7.10-gentoo-r1, archname=x86_64-linux
    uname='linux andrewn 3.7.10-gentoo-r1 #1 smp preempt thu may 16 09:49:20 edt 2013 x86_64 intel(r) core(tm) i7-2600 cpu @ 3.40ghz genuineintel gnulinux '
    config_args='-de -Dusedevel -Dstartperl=#!/home/andrewn/src/git/perl/perl -Doptimize=none -DEBUGGING=both'
    hint=recommended, useposix=true, d_sigaction=define
    useithreads=undef, usemultiplicity=undef
    useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
    use64bitint=define, use64bitall=define, uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='cc', ccflags ='-DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
    optimize='  -g',
    cppflags='-DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector'
    ccversion='', gccversion='4.6.3', gccosandvers=''
    intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678
    d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16
    ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
    alignbytes=8, prototype=define
  Linker and Libraries:
    ld='cc', ldflags =' -fstack-protector -L/usr/local/lib'
    libpth=/usr/local/lib /lib/../lib64 /usr/lib/../lib64 /lib /usr/lib /lib64 /usr/lib64 /usr/local/lib64
    libs=-lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lc -lgdbm_compat
    perllibs=-lnsl -ldl -lm -lcrypt -lutil -lc
    libc=/lib/libc-2.15.so, so=so, useshrplib=false, libperl=libperl.a
    gnulibc_version='2.15'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
    cccdlflags='-fPIC', lddlflags='-shared   -g -L/usr/local/lib -fstack-protector'



@INC for perl 5.19.4:
    ./lib
    /home/andrewn/perl5lib/lib/x86_64-linux
    /home/andrewn/perl5lib/lib
    /home/andrewn/perl5lib/lib/x86_64-linux
    /home/andrewn/svn/puppet/trunk/files/package/perl
    /home/andrewn/perl5lib/lib/x86_64-linux
    /home/andrewn/perl5lib/lib
    /home/andrewn/perl5lib/lib/x86_64-linux
    /home/andrewn/svn/puppet/trunk/files/package/perl
    /home/andrewn/perl5lib/lib/x86_64-linux
    /home/andrewn/perl5lib/lib
    /home/andrewn/perl5lib/lib/x86_64-linux
    /home/andrewn/svn/puppet/trunk/files/package/perl
    /usr/local/lib/perl5/site_perl/5.19.4/x86_64-linux
    /usr/local/lib/perl5/site_perl/5.19.4
    /usr/local/lib/perl5/5.19.4/x86_64-linux
    /usr/local/lib/perl5/5.19.4
    .


Environment for perl 5.19.4:
    HOME=/home/andrewn
    LANG=en_US.UTF-8
    LANGUAGE (unset)
    LC_COLLATE=C
    LD_LIBRARY_PATH=:/home/andrewn/lib
    LOGDIR (unset)
    PATH=/home/andrewn/bin:/usr/local/bin:/usr/bin:/bin:/opt/bin:/usr/x86_64-pc-linux-gnu/gcc-bin/4.6.3:/usr/games/bin
    PERL5LIB=./lib:/home/andrewn/perl5lib/lib:/home/andrewn/perl5lib/lib/x86_64-linux:/home/andrewn/svn/puppet/trunk/files/package/perl:/home/andrewn/perl5lib/lib:/home/andrewn/perl5lib/lib/x86_64-linux:/home/andrewn/svn/puppet/trunk/files/package/perl:/home/andrewn/perl5lib/lib:/home/andrewn/perl5lib/lib/x86_64-linux:/home/andrewn/svn/puppet/trunk/files/package/perl
    PERL_BADLANG (unset)
    SHELL=/bin/bash

@p5pRT
Copy link
Author

p5pRT commented Aug 28, 2013

From @jkeenan

On Wed Aug 28 14​:09​:51 2013, andrewn@​locus.net wrote​:

This is a bug report for perl from andrewn@​locus.net,
generated with the help of perlbug 1.39 running under perl 5.19.4.

-----------------------------------------------------------------
[Please describe your issue here]

$ ./perl -e '/\7777777777/'
Segmentation fault

This is caused by a negative backreference in the compiled regex when
the
following code returns a negative number​:

regcomp.c​:10690​: num = atoi(RExC_parse);

This bug was discovered in our production system running perl-5.8.8-
40.el5_9
(CentOS5), confirmed on a developer's ActivePerl 5.16.2 (Windows7),
and
debugged/reported on the current git.

The following patch does not properly correct the parsing of large
integers,
but it should at least die cleanly instead of segfaulting​:

diff --git a/regcomp.c b/regcomp.c
index 5a1e234..3854569 100644
--- a/regcomp.c
+++ b/regcomp.c
@​@​ -10688,6 +10688,9 @​@​ tryagain​:
goto parse_named_seq;
} }
num = atoi(RExC_parse);
+ if(num < 0) {
+ vFAIL("Integer wrapped?");
+ }
if (isg && num == 0) {
if (*RExC_parse == '0') {
vFAIL("Reference to invalid group 0");

Confirmed in blead.

#####
$ ./perl -Ilib -v | head -2 | tail -1
This is perl 5, version 19, subversion 4 (v5.19.4
(v5.19.3-148-g5b90322)) built for x86_64-linux
$ ./perl -e '/\7777777777/'
Segmentation fault
#####

At the very least, we would need to add a regression test before
applying this patch.

Thank you very much.
Jim Keenan

@p5pRT
Copy link
Author

p5pRT commented Aug 28, 2013

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

@p5pRT
Copy link
Author

p5pRT commented Oct 17, 2013

From @iabyn

On Wed, Aug 28, 2013 at 02​:09​:51PM -0700, andrewn@​locus.net wrote​:

$ ./perl -e '/\7777777777/'
Segmentation fault

This is caused by a negative backreference in the compiled regex when the
following code returns a negative number​:

regcomp.c​:10690​: num = atoi(RExC_parse);

This bug was discovered in our production system running perl-5.8.8-40.el5_9
(CentOS5), confirmed on a developer's ActivePerl 5.16.2 (Windows7), and
debugged/reported on the current git.

The following patch does not properly correct the parsing of large integers,
but it should at least die cleanly instead of segfaulting​:

Thanks for the report and patch. I've actually applied this more general
fix to bleed​:

commit 0c2990d
Author​: David Mitchell <davem@​iabyn.com>
AuthorDate​: Wed Oct 16 13​:59​:12 2013 +0100
Commit​: David Mitchell <davem@​iabyn.com>
CommitDate​: Thu Oct 17 10​:57​:35 2013 +0100

  [perl #119505] Segfault from bad backreference
 
  The code that parses regex backrefs (or ambiguous backref/octal) such as
  \123, did a simple atoi(), which could wrap round to negative values on
  long digit strings and cause seg faults.
 
  Include a check on the length of the digit string, and if greater than 9
  digits, assume it can never be a valid backref (obviating the need for the
  atoi() call).
 
  I've also simplified the code a bit, putting most of the \g handling code
  into a single block, rather than doing multiple "if (isg) {...}".

M regcomp.c
M t/re/re_tests

--
Lear​: Dost thou call me fool, boy?
Fool​: All thy other titles thou hast given away; that thou wast born with.

@p5pRT
Copy link
Author

p5pRT commented Oct 17, 2013

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

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