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

null ptr deref -> S_pad_findlex (pad.c:1141 #14621

Closed
p5pRT opened this issue Mar 27, 2015 · 16 comments
Closed

null ptr deref -> S_pad_findlex (pad.c:1141 #14621

p5pRT opened this issue Mar 27, 2015 · 16 comments

Comments

@p5pRT
Copy link

p5pRT commented Mar 27, 2015

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

Searchable as RT124187$

@p5pRT
Copy link
Author

p5pRT commented Mar 27, 2015

From @geeknik

Built v5.21.11 (v5.21.10-23-g21639bf) with the following command line​:

./Configure -des -Dusedevel -DDEBUGGING -Dcc=afl-gcc -Doptimize=-O2\ -g && AFL_HARDEN=1 make -j4 test-prep

Bug found with AFL (http​://lcamtuf.coredump.cx/afl)

Valgrind​:
==14673== Invalid read of size 8
==14673== at 0x682C6C​: S_pad_findlex (pad.c​:1141)
==14673== by 0x6861FA​: Perl_pad_findmy_pvn (pad.c​:962)
==14673== by 0x5F6C64​: Perl_yylex (toke.c​:6482)
==14673== by 0x65A584​: Perl_yyparse (perly.c​:322)
==14673== by 0x531574​: S_parse_body (perl.c​:2296)
==14673== by 0x5392E2​: perl_parse (perl.c​:1626)
==14673== by 0x42AC67​: main (perlmain.c​:114)
==14673== Address 0x0 is not stack'd, malloc'd or (recently) free'd
==14673==
==14673==
==14673== Process terminating with default action of signal 11 (SIGSEGV)
==14673== Access not within mapped region at address 0x0
==14673== at 0x682C6C​: S_pad_findlex (pad.c​:1141)
==14673== by 0x6861FA​: Perl_pad_findmy_pvn (pad.c​:962)
==14673== by 0x5F6C64​: Perl_yylex (toke.c​:6482)
==14673== by 0x65A584​: Perl_yyparse (perly.c​:322)
==14673== by 0x531574​: S_parse_body (perl.c​:2296)
==14673== by 0x5392E2​: perl_parse (perl.c​:1626)
==14673== by 0x42AC67​: main (perlmain.c​:114)
==14673== If you believe this happened as a result of a stack
==14673== overflow in your program's main thread (unlikely but
==14673== possible), you can try to increase the size of the
==14673== main thread stack using the --main-stacksize= flag.
==14673== The main thread stack size used in this run was 8388608.
Segmentation fault

GDB​:
Program received signal SIGSEGV, Segmentation fault.
[----------------------------------registers-----------------------------------]
RAX​: 0x0
RBX​: 0x11eb215 --> 0x21000000 ('')
RCX​: 0x0
RDX​: 0x0
RSI​: 0x2
RDI​: 0x7fffffffdb40 --> 0x9d7526 (<Perl_sv_catpv+2918>​: add BYTE PTR [rax],al)
RBP​: 0x11eb215 --> 0x21000000 ('')
RSP​: 0x7fffffffd8a0 --> 0x11cfd10 --> 0x1
RIP​: 0x682c6c (<S_pad_findlex+76>​: mov rbx,QWORD PTR [rax])
R8 : 0xffffffd2
R9 : 0x1
R10​: 0x11ec198 --> 0x620075 (<Perl_yylex+313365>​: add BYTE PTR [rax-0x75],cl)
R11​: 0x7ffff6eb3730 --> 0xfffd5830fffd5070
R12​: 0x2
R13​: 0x0
R14​: 0x2
R15​: 0x1
EFLAGS​: 0x10206 (carry PARITY adjust zero sign trap INTERRUPT direction overflow)
[-------------------------------------code-------------------------------------]
  0x682c66 <S_pad_findlex+70>​: push rbp
  0x682c67 <S_pad_findlex+71>​: push rbx
  0x682c68 <S_pad_findlex+72>​: sub rsp,0x78
=> 0x682c6c <S_pad_findlex+76>​: mov rbx,QWORD PTR [rax]
  0x682c6f <S_pad_findlex+79>​: mov rsi,QWORD PTR [rsp+0xb8]
  0x682c77 <S_pad_findlex+87>​: mov QWORD PTR [rsp],rdi
  0x682c7b <S_pad_findlex+91>​: mov DWORD PTR [rsp+0x50],edx
  0x682c7f <S_pad_findlex+95>​: mov QWORD PTR [rsp+0x30],rcx
[------------------------------------stack-------------------------------------]
0000| 0x7fffffffd8a0 --> 0x11cfd10 --> 0x1
0008| 0x7fffffffd8a8 --> 0x682c42 (<S_pad_findlex+34>​: mov rax,QWORD PTR [rsp+0x10])
0016| 0x7fffffffd8b0 --> 0x0
0024| 0x7fffffffd8b8 --> 0x0
0032| 0x7fffffffd8c0 --> 0x7fffffffd98c --> 0x11efbde00000000
0040| 0x7fffffffd8c8 --> 0x4000 ('')
0048| 0x7fffffffd8d0 --> 0x6861b2 (<Perl_pad_findmy_pvn+130>​: mov rax,QWORD PTR [rsp+0x10])
0056| 0x7fffffffd8d8 --> 0x0
[------------------------------------------------------------------------------]
Legend​: code, data, rodata, value
Stopped reason​: SIGSEGV
S_pad_findlex () at pad.c​:1141
1141 const PADLIST * const padlist = CvPADLIST(cv);
gdb-peda$ list
1136 int warn, SV** out_capture, PADNAME** out_name, int *out_flags)
1137 {
1138 I32 offset, new_offset;
1139 SV *new_capture;
1140 SV **new_capturep;
1141 const PADLIST * const padlist = CvPADLIST(cv);
1142 const bool staleok = !!(flags & padadd_STALEOK);
1143
1144 PERL_ARGS_ASSERT_PAD_FINDLEX;
1145

Hexdump of the 21-byte test case​:
0000000 7171 407b 5b7b 7d7b 2a7d 7573 7b62 5d5d
0000010 7d7d 3d7d 0075
0000015

System Info​: Debian 7, Kernel 3.2.65-1+deb7u2 x86_64, GCC 4.9.2, libc 2.13-38+deb7u8

@p5pRT
Copy link
Author

p5pRT commented Mar 27, 2015

From @geeknik

test1-min

@p5pRT
Copy link
Author

p5pRT commented Mar 27, 2015

From @geeknik

The test case (perl -e 'qq{@​{[{}}*sub{]]}}}=u') also crashes the following​:

perl 5, version 18, subversion 4 (v5.18.4) built for i386-freebsd-thread-multi-64int
perl 5, version 21, subversion 7 (v5.21.7 (v5.21.6-602-ge9d2bd8)) built for x86_64-linux

@p5pRT
Copy link
Author

p5pRT commented Mar 27, 2015

From @geeknik

Does not crash perl 5, version 16, subversion 3 (v5.16.3) built for amd64-freebsd-thread-multi, fails with this error​:

$ perl -e 'qq{@​{[{}}*sub{]]}}}=u'
syntax error at -e line 1, near "}}"
syntax error at -e line 1, near "{]"
Unmatched right curly bracket at -e line 1, at end of line
syntax error at -e line 1, at EOF
Execution of -e aborted due to compilation errors.

@p5pRT
Copy link
Author

p5pRT commented Mar 27, 2015

From @wolfsage

On Thu, Mar 26, 2015 at 11​:45 PM, Brian Carpenter
<perlbug-followup@​perl.org> wrote​:

# New Ticket Created by Brian Carpenter
# Please include the string​: [perl #124187]
# in the subject line of all future correspondence about this issue.
# <URL​: https://rt-archive.perl.org/perl5/Ticket/Display.html?id=124187 >

This started SEGVing between 5.17.5 and 5.17.6 (on vanilla -des
-Dusedevel builds)

  /home/mhorsfall/dpppperls/default/perl-5.17.5/bin/perl5.17.5
/home/mhorsfall/test1-min 2>&1
  syntax error at /home/mhorsfall/test1-min line 1, near "}}"
  syntax error at /home/mhorsfall/test1-min line 1, near "{]"
  Unmatched right curly bracket at /home/mhorsfall/test1-min line 1,
at end of line
  syntax error at /home/mhorsfall/test1-min line 1, at EOF
  Execution of /home/mhorsfall/test1-min aborted due to compilation errors.
  child exited with value 255
  /home/mhorsfall/dpppperls/default/perl-5.17.6/bin/perl5.17.6
/home/mhorsfall/test1-min 2>&1
  child died with signal 11 (SEGV)

-- Matthew Horsfall (alh)

@p5pRT
Copy link
Author

p5pRT commented Mar 27, 2015

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

@p5pRT
Copy link
Author

p5pRT commented Mar 27, 2015

From @wolfsage

On Fri, Mar 27, 2015 at 7​:21 AM, Matthew Horsfall (alh)
<wolfsage@​gmail.com> wrote​:

This started SEGVing between 5.17.5 and 5.17.6 (on vanilla -des
-Dusedevel builds)

/home/mhorsfall/dpppperls/default/perl-5.17.5/bin/perl5.17.5
/home/mhorsfall/test1-min 2>&1
syntax error at /home/mhorsfall/test1-min line 1, near "}}"
syntax error at /home/mhorsfall/test1-min line 1, near "{]"
Unmatched right curly bracket at /home/mhorsfall/test1-min line 1,
at end of line
syntax error at /home/mhorsfall/test1-min line 1, at EOF
Execution of /home/mhorsfall/test1-min aborted due to compilation errors.
child exited with value 255
/home/mhorsfall/dpppperls/default/perl-5.17.6/bin/perl5.17.6
/home/mhorsfall/test1-min 2>&1
child died with signal 11 (SEGV)

bad - non-zero exit from ./perl -Ilib /home/mhorsfall/test1-min
9ffcdca is the first bad commit
commit 9ffcdca
Author​: Father Chrysostomos <sprout@​cpan.org>
Date​: Mon Nov 12 23​:04​:16 2012 -0800

  Don’t leak subs containing syntax errors

  I fixed this for BEGIN blocks earlier, but missed the fact that
  all subs are affected.

  When called without an o argument (from newANONATTRSUB), newATTRSUB
  is expected to return a CV with an unowned reference count of which
  the caller will take ownership. We cannot have newATTRSUB returning
  a freed CV, so we have it return null instead. But that means
  ck_anoncode and pm_runtime have to account for that.

-- Matthew Horsfall (alh)

@p5pRT
Copy link
Author

p5pRT commented Mar 27, 2015

From @cpansprout

On Fri Mar 27 05​:06​:05 2015, alh wrote​:

bad - non-zero exit from ./perl -Ilib /home/mhorsfall/test1-min
9ffcdca is the first bad commit
commit 9ffcdca
Author​: Father Chrysostomos <sprout@​cpan.org>
Date​: Mon Nov 12 23​:04​:16 2012 -0800

Don’t leak subs containing syntax errors

Why are you always blaming me? :-)

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Mar 27, 2015

From @wolfsage

On Fri, Mar 27, 2015 at 11​:43 AM, Father Chrysostomos via RT
<perlbug-followup@​perl.org> wrote​:

On Fri Mar 27 05​:06​:05 2015, alh wrote​:

bad - non-zero exit from ./perl -Ilib /home/mhorsfall/test1-min
9ffcdca is the first bad commit
commit 9ffcdca
Author​: Father Chrysostomos <sprout@​cpan.org>
Date​: Mon Nov 12 23​:04​:16 2012 -0800

Don’t leak subs containing syntax errors

Why are you always blaming me? :-)

  $ git show 6a8dbfd
  commit 6a8dbfd
  Author​: Nicholas Clark <nick@​ccl4.org>
  Date​: Thu Sep 29 22​:44​:45 2011 +0200

  Add Porting/bisect.pl, to automate bisecting a perl code test case.

It's not me! :)

-- Matthew Horsfall (alh)

@p5pRT
Copy link
Author

p5pRT commented Apr 22, 2015

From @tonycoz

On Fri Mar 27 05​:06​:05 2015, alh wrote​:

bad - non-zero exit from ./perl -Ilib /home/mhorsfall/test1-min
9ffcdca is the first bad commit
commit 9ffcdca
Author​: Father Chrysostomos <sprout@​cpan.org>
Date​: Mon Nov 12 23​:04​:16 2012 -0800

Don’t leak subs containing syntax errors

The attached prevents the crash.

There maybe a deeper issue where PL_compcv isn't being restored
correctly.

Tony

@p5pRT
Copy link
Author

p5pRT commented Apr 22, 2015

From @tonycoz

0001-perl-124187-don-t-call-pad_findlex-on-a-NULL-CV.patch
From 2cb2d43c2aa5d6af0af9e32ae223501c7679fd74 Mon Sep 17 00:00:00 2001
From: Tony Cook <tony@develop-help.com>
Date: Wed, 22 Apr 2015 15:01:15 +1000
Subject: [PATCH] [perl #124187] don't call pad_findlex() on a NULL CV

---
 pad.c        |    4 ++++
 t/base/lex.t |    5 +++++
 2 files changed, 9 insertions(+)

diff --git a/pad.c b/pad.c
index 2d33779..984b5c1 100644
--- a/pad.c
+++ b/pad.c
@@ -959,6 +959,10 @@ Perl_pad_findmy_pvn(pTHX_ const char *namepv, STRLEN namelen, U32 flags)
 	Perl_croak(aTHX_ "panic: pad_findmy_pvn illegal flag bits 0x%" UVxf,
 		   (UV)flags);
 
+    /* compilation errors can zero PL_compcv */
+    if (!PL_compcv)
+        return NOT_IN_PAD;
+
     offset = pad_findlex(namepv, namelen, flags,
                 PL_compcv, PL_cop_seqmax, 1, NULL, &out_pn, &out_flags);
     if ((PADOFFSET)offset != NOT_IN_PAD) 
diff --git a/t/base/lex.t b/t/base/lex.t
index 0a07ab7..a34a508 100644
--- a/t/base/lex.t
+++ b/t/base/lex.t
@@ -506,3 +506,8 @@ eval q|s##[}#e|;
  eval q|my($_);0=split|;
  eval q|my $_; @x = split|;
 }
+
+{
+ # Used to crash [perl #124187]
+ eval q|qq{@{[{}}*sub{]]}}}=u|;
+}
-- 
1.7.10.4

@p5pRT
Copy link
Author

p5pRT commented Apr 27, 2015

From @rjbs

On Tue Apr 21 22​:03​:33 2015, tonyc wrote​:

The attached prevents the crash.

There maybe a deeper issue where PL_compcv isn't being restored
correctly.

Are we (read​: you) comfortable with this patch as the way to sort this out? Is your concern that this will paper over one symptom but leave a deeper problem still ready to spring, or is that an unrelated observation?

--
rjbs

@p5pRT
Copy link
Author

p5pRT commented Apr 28, 2015

From @tonycoz

On Mon Apr 27 15​:21​:58 2015, rjbs wrote​:

On Tue Apr 21 22​:03​:33 2015, tonyc wrote​:

The attached prevents the crash.

There maybe a deeper issue where PL_compcv isn't being restored
correctly.

Are we (read​: you) comfortable with this patch as the way to sort this
out? Is your concern that this will paper over one symptom but leave
a deeper problem still ready to spring, or is that an unrelated
observation?

Yes, that's my concern.

I haven't worked with the parser enough to know whether PL_compcv == NULL
is a normal case.

Ticket 124385 seems related - attemptting to do something with a NULL
PL_compcv, if that's a normal case then that code will need a similar fix.

Tony

@p5pRT
Copy link
Author

p5pRT commented May 5, 2015

From @tonycoz

On Mon Apr 27 18​:03​:33 2015, tonyc wrote​:

On Mon Apr 27 15​:21​:58 2015, rjbs wrote​:

On Tue Apr 21 22​:03​:33 2015, tonyc wrote​:

The attached prevents the crash.

There maybe a deeper issue where PL_compcv isn't being restored
correctly.

Are we (read​: you) comfortable with this patch as the way to sort this
out? Is your concern that this will paper over one symptom but leave
a deeper problem still ready to spring, or is that an unrelated
observation?

Yes, that's my concern.

I haven't worked with the parser enough to know whether PL_compcv == NULL
is a normal case.

These also crash in S_pad_findlex(), with different backtraces​:

qq{@​{[{}}*sub{]]}}}=<$foo>

qq{@​{[{}}*sub{]]}}}; foo()

In all three cases, PP_compcv == PL_main_cv and there's no sensible value to
replace into PL_compcv.

I tried changing the code from 9ffcdca to leave PL_compcv if it's PL_main_cv,
but that resulted in later assertion failures.

I've pushed my original patch from above to blead, since no-one has objected
to it.

Tony

@p5pRT p5pRT closed this as completed May 5, 2015
@p5pRT
Copy link
Author

p5pRT commented May 5, 2015

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

@p5pRT
Copy link
Author

p5pRT commented May 5, 2015

From @iabyn

On Mon, May 04, 2015 at 11​:51​:56PM -0700, Tony Cook via RT wrote​:

On Mon Apr 27 18​:03​:33 2015, tonyc wrote​:

On Mon Apr 27 15​:21​:58 2015, rjbs wrote​:

On Tue Apr 21 22​:03​:33 2015, tonyc wrote​:

The attached prevents the crash.

There maybe a deeper issue where PL_compcv isn't being restored
correctly.

Are we (read​: you) comfortable with this patch as the way to sort this
out? Is your concern that this will paper over one symptom but leave
a deeper problem still ready to spring, or is that an unrelated
observation?

Yes, that's my concern.

I haven't worked with the parser enough to know whether PL_compcv == NULL
is a normal case.

These also crash in S_pad_findlex(), with different backtraces​:

qq{@​{[{}}*sub{]]}}}=<$foo>

qq{@​{[{}}*sub{]]}}}; foo()

In all three cases, PP_compcv == PL_main_cv and there's no sensible value to
replace into PL_compcv.

I tried changing the code from 9ffcdca to leave PL_compcv if it's PL_main_cv,
but that resulted in later assertion failures.

I've pushed my original patch from above to blead, since no-one has objected
to it.

I think my description in de0885d covers all these​:

commit de0885d
Author​: David Mitchell <davem@​iabyn.com>
AuthorDate​: Tue May 5 10​:44​:16 2015 +0100
Commit​: David Mitchell <davem@​iabyn.com>
CommitDate​: Tue May 5 11​:23​:36 2015 +0100

  null ptr deref in Perl_cv_forget_slab
 
  RT #124385
 
  Parsing following a syntax error could result in a null ptr dereference.
 
  This commit contains a band-aid that returns from Perl_cv_forget_slab() if
  the cv arg is null; but the real issue is much deeper and needs a more
  general fix at some point.
 
  Basically, both the lexer and the parser use the save stack, and after an
  error, they can get out of sync.
 
  In particular​:
 
  1) when handling a double-quoted string, the lexer does an ENTER, saves
  most of its current state on the save stack, then uses the literal string
  as the toke source. When it reaches the end of the string, it LEAVEs,
  restores the lexer state and continues with the main source.
 
  2) Whenever the parser starts a new block or sub scope, it remembers the
  current save stack position, and at end of scope, pops the save stack back
  to that position.
 
  In something like
 
  "@​{ sub {]}} }}}"
 
  the lexer sees a double-quoted string, and saves the current lex state.
  The parser sees the start of a sub, and saves PL_compcv etc. Then a parse
  error occurs. The parser goes into error recovery, discarding tokens until
  it can return to a sane state. The lexer runs out of tokens when toking
  the string, does a LEAVE, and switches back to toking the main source.
  This LEAVE restores both the lexer's and the parser's state; in particular
  the parser gets its old PL_compcv restored, even though the parser hasn't
  finished compiling the current sub. Later, series of '}' tokens coming
  through allows the parser to finish the sub. Since PL_error_count > 0, it
  discards the just-compiled sub and sets PL_compcv to null. Normally the
  LEAVE_SCOPE done just after this would restore PL_compcv to its old value
  (e.g. PL_main_cv) but the stack has already been popped, so PL_compcv gets
  left null, and SEGVs occur.
 
  The two main ways I can think of fixing this in the long term are
  1) avoid the lexer using the save stack for long-term state storage;
  in particular, make S_sublex_push() malloc a new parser object rather
  than saving the current lexer state on the save stack.
  2) At the end of a sublex, if PL_error_count > 0, don't try to restore
  state and continue, instead just croak.
 
  N.B. the test that this commit adds to lex.t doesn't actually trigger the
  SEGV, since the bad code is wrapped in an eval which (for reasons I
  haven't researched) avoids the SEGV.

--
This is a great day for France!
  -- Nixon at Charles De Gaulle's funeral

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

No branches or pull requests

1 participant