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

Possible Buffer Overflow Perl 5.21.8 #14416

Closed
p5pRT opened this issue Jan 15, 2015 · 14 comments

Comments

@p5pRT
Copy link
Collaborator

@p5pRT p5pRT commented Jan 15, 2015

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

Searchable as RT123604$

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 15, 2015

From @geeknik

I am currently fuzzing Perl 5.21.8 built from git source (This is perl 5,
version 21, subversion 8 (v5.21.8 (v5.21.7-359-geb9df70)) built for
x86_64-linux) on 14 January 2015. I am using AFL (
http​://lcamtuf.coredump.cx/afl/) for this process.

For compiling, I performed the following​:
CC=/home/geeknik/afl/afl-gcc CXX=/home/geeknik/afl/afl-g++ ./Configure
make

All other Perl options during the process were defaults (pressed enter)
except for adding -g as a CFLAG.

Here is the Valgrind and GDB output. I've attached the crashing test case
and the core dump to this email.

geeknik@​deb7fuzz​:~/findings/perl5/fuzzer02/crashes$ valgrind -q
/home/geeknik/perl5/perl
id\​:000000\,sig\​:11\,src\​:002478+018343\,op\​:splice\,rep\​:4

Backslash found where operator expected at
id​:000000,sig​:11,src​:002478+018343,op​:splice,rep​:4 line 1, near "/le\b
encodingusr/le\"
(Missing operator before \?)
String found where operator expected at
id​:000000,sig​:11,src​:002478+018343,op​:splice,rep​:4 line 1, near "print ""
(Missing semicolon on previous line?)
Bareword found where operator expected at
id​:000000,sig​:11,src​:002478+018343,op​:splice,rep​:4 line 2, near "print
"other"
  (Might be a runaway multi-line "" string starting on line 1)
(Do you need to predeclare print?)
String found where operator expected at
id​:000000,sig​:11,src​:002478+018343,op​:splice,rep​:4 line 2, near "n""
(Missing semicolon on previous line?)
Bareword found where operator expected at
id​:000000,sig​:11,src​:002478+018343,op​:splice,rep​:4 line 5, near ")V"
(Missing operator before V?)

*** stack smashing detected ***​: /home/geeknik/perl5/perl terminated

==18174== Invalid read of size 1
==18174== at 0x62BE730​: ??? (in /lib/x86_64-linux-gnu/libgcc_s.so.1)
==18174== by 0x62BF3BB​: _Unwind_Backtrace (in
/lib/x86_64-linux-gnu/libgcc_s.so.1)
==18174== by 0x59F814D​: backtrace (backtrace.c​:91)
==18174== by 0x59741DE​: __libc_message (libc_fatal.c​:168)
==18174== by 0x59F7FE6​: __fortify_fail (fortify_fail.c​:32)
==18174== by 0x59F7FAF​: __stack_chk_fail (stack_chk_fail.c​:29)
==18174== by 0x53A8C4​: S_intuit_more.part.4 (toke.c​:3849)
==18174== by 0x505050505050504F​: ???
==18174== by 0x505050505050504F​: ???
==18174== by 0x505050505050504F​: ???
==18174== by 0x505050505050504F​: ???
==18174== by 0x505050505050504F​: ???
==18174== Address 0x5050505050505050 is not stack'd, malloc'd or
(recently) free'd
==18174==
==18174==
==18174== Process terminating with default action of signal 11 (SIGSEGV)​:
dumping core
==18174== General Protection Fault
==18174== at 0x62BE730​: ??? (in /lib/x86_64-linux-gnu/libgcc_s.so.1)
==18174== by 0x62BF3BB​: _Unwind_Backtrace (in
/lib/x86_64-linux-gnu/libgcc_s.so.1)
==18174== by 0x59F814D​: backtrace (backtrace.c​:91)
==18174== by 0x59741DE​: __libc_message (libc_fatal.c​:168)
==18174== by 0x59F7FE6​: __fortify_fail (fortify_fail.c​:32)
==18174== by 0x59F7FAF​: __stack_chk_fail (stack_chk_fail.c​:29)
==18174== by 0x53A8C4​: S_intuit_more.part.4 (toke.c​:3849)
==18174== by 0x505050505050504F​: ???
==18174== by 0x505050505050504F​: ???
==18174== by 0x505050505050504F​: ???
==18174== by 0x505050505050504F​: ???
==18174== by 0x505050505050504F​: ???
Segmentation fault

geeknik@​deb7fuzz​:~/findings/perl5/fuzzer02/crashes$
/home/geeknik/perl5/perl
id\​:000000\,sig\​:11\,src\​:002478+018343\,op\​:splice\,rep\​:4

Backslash found where operator expected at
id​:000000,sig​:11,src​:002478+018343,op​:splice,rep​:4 line 1, near "/le\b
encodingusr/le\"
(Missing operator before \?)
String found where operator expected at
id​:000000,sig​:11,src​:002478+018343,op​:splice,rep​:4 line 1, near "print ""
(Missing semicolon on previous line?)
Bareword found where operator expected at
id​:000000,sig​:11,src​:002478+018343,op​:splice,rep​:4 line 2, near "print
"other"
  (Might be a runaway multi-line "" string starting on line 1)
(Do you need to predeclare print?)
String found where operator expected at
id​:000000,sig​:11,src​:002478+018343,op​:splice,rep​:4 line 2, near "n""
(Missing semicolon on previous line?)
Bareword found where operator expected at
id​:000000,sig​:11,src​:002478+018343,op​:splice,rep​:4 line 5, near ")V"
(Missing operator before V?)
*** stack smashing detected ***​: /home/geeknik/perl5/perl terminated
Segmentation fault (core dumped)

geeknik@​deb7fuzz​:~/findings/perl5/fuzzer02/crashes$ gdb
/home/geeknik/perl5/perl core

GNU gdb (GDB) 7.4.1-debian
Copyright (C) 2012 Free Software Foundation, Inc.
License GPLv3+​: GNU GPL version 3 or later <http​://gnu.org/licenses/gpl.html

This is free software​: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law. Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-linux-gnu".
For bug reporting instructions, please see​:
<http​://www.gnu.org/software/gdb/bugs/>...
Reading symbols from /home/geeknik/perl5/perl...done.
[New LWP 42899]

warning​: Can't read pathname for load map​: Input/output error.
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
Core was generated by `/home/geeknik/perl5/perl
id​:000000,sig​:11,src​:002478+018343,op​:splice,rep​:4'.
Program terminated with signal 11, Segmentation fault.
#0 0x00007fb02f739730 in ?? () from /lib/x86_64-linux-gnu/libgcc_s.so.1
(gdb) bt
#0 0x00007fb02f739730 in ?? () from /lib/x86_64-linux-gnu/libgcc_s.so.1
#1 0x00007fb02f73a3bc in _Unwind_Backtrace () from
/lib/x86_64-linux-gnu/libgcc_s.so.1
#2 0x00007fb02fc4d14e in *__GI___backtrace (array=<optimized out>,
size=64) at ../sysdeps/x86_64/../ia64/backtrace.c​:91
#3 0x00007fb02fbc91df in __libc_message (do_abort=<optimized out>,
fmt=<optimized out>)
  at ../sysdeps/unix/sysv/linux/libc_fatal.c​:168
#4 0x00007fb02fc4cfe7 in *__GI___fortify_fail (msg=0x7fb02fcaafaa "stack
smashing detected") at fortify_fail.c​:32
#5 0x00007fb02fc4cfb0 in __stack_chk_fail () at stack_chk_fail.c​:29
#6 0x000000000053a8c5 in S_intuit_more (s=<optimized out>) at toke.c​:3849
#7 0x5050505050505050 in ?? ()
#8 0x5050505050505050 in ?? ()
#9 0x5050505050505050 in ?? ()
#10 0x5050505050505050 in ?? ()
#11 0x5050505050505050 in ?? ()
#12 0x5050505050505050 in ?? ()
#13 0x5050505050505050 in ?? ()
#14 0x5050505050505050 in ?? ()
#15 0x5050505050505050 in ?? ()
#16 0x5050505050505050 in ?? ()
#17 0x5050505050505050 in ?? ()
#18 0x5050505050505050 in ?? ()
#19 0x5050505050505050 in ?? ()
#20 0x5050505050505050 in ?? ()
#21 0x5050505050505050 in ?? ()
#22 0x5050505050505050 in ?? ()
---Type <return> to continue, or q <return> to quit---
#23 0x5050505050505050 in ?? ()
#24 0x5050505050505050 in ?? ()
#25 0x5050505050505050 in ?? ()
#26 0x5050505050505050 in ?? ()
#27 0x5050505050505050 in ?? ()
#28 0x5050505050505050 in ?? ()
#29 0x5050505050505050 in ?? ()
#30 0x5050505050505050 in ?? ()
#31 0x0000005050505050 in ?? ()
#32 0x0000000000000005 in ?? ()
#33 0x000000000000001f in ?? ()
#34 0x0000000002172600 in ?? ()
#35 0x0000000000000000 in ?? ()
(gdb) i r
rax 0x5 5
rbx 0x0 0
rcx 0x5050505050505050 5787213827046133840
rdx 0x7fff17130550 140733580510544
rsi 0x0 0
rdi 0x0 0
rbp 0x7fff1712f520 0x7fff1712f520
rsp 0x7fff1712f3d0 0x7fff1712f3d0
r8 0x6474e551 1685382481
r9 0x7fb02f93f000 140394689196032
r10 0x7fb02f93f678 140394689197688
r11 0x0 0
r12 0x0 0
r13 0x7fff1712f430 140733580506160
r14 0x45 69
r15 0x5 5
rip 0x7fb02f739730 0x7fb02f739730
eflags 0x10246 [ PF ZF IF RF ]
cs 0x33 51
ss 0x2b 43
ds 0x0 0
es 0x0 0
fs 0x0 0
gs 0x0 0
(gdb)

I have at least a dozen more of these but I'm not all that great at
debugging them, so if someone wants to meet up on IRC or have a Skype
session and talk about these, let me know.

Regards,

Brian 'geeknik' Carpenter

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 15, 2015

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 16, 2015

From @hvds

FWIW I've managed to reduce this to​:
% perl -ce '/$x[PPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPP!
PPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPP]/'
*** stack smashing detected ***​: perl terminated
Aborted (core dumped)
%

Hugo

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 16, 2015

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

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 16, 2015

From @hvds

I wrote​:
:FWIW I've managed to reduce this to​:
:% perl -ce '/$x[PPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPP!
P!
:PPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPPP]/'
:*** stack smashing detected ***​: perl terminated
:Aborted (core dumped)
:%

Ah, that's easy then, 'tmpbuf' below is a stack array​:
  char tmpbuf[sizeof PL_tokenbuf * 4];

I'm not up on stack-smashing techniques, but the isALPHA() restriction
might stop it being readily exploitable; I doubt it stops it entirely
though.

This code goes back at least as far as origin/maint-5.004.

Simplistic patch below is enough to defang the short test case above and
the original fuzzed case; a slightly cleaner version of that is probably
about all that's needed. (Maybe we can actually call scan_ident instead,
not sure.)

Hugo

Inline Patch
diff --git a/toke.c b/toke.c
index dfb5b20..84cb1e7 100644
--- a/toke.c
+++ b/toke.c
@@ -3828,11 +3828,13 @@ S_intuit_more(pTHX_ char *s)
                         || last_un_char == '&')
                    && isALPHA(*s) && s[1] && isALPHA(s[1])) {
                    char *d = tmpbuf;
-                   while (isALPHA(*s))
+                   while (isALPHA(*s) && d < tmpbuf + sizeof(tmpbuf))
                        *d++ = *s++;
-                   *d = '\0';
-                   if (keyword(tmpbuf, d - tmpbuf, 0))
-                       weight -= 150;
+                    if (d < tmpbuf + sizeof(tmpbuf)) {
+                        *d = '\0';
+                        if (keyword(tmpbuf, d - tmpbuf, 0))
+                            weight -= 150;
+                    }
                }
                if (un_char == last_un_char + 1)
                    weight += 5;
@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 16, 2015

From @hvds

:Simplistic patch below is enough to defang the short test case [...] and
:the original fuzzed case.

Actually, we don't seem to need tmpbuf at all here; the alternative patch
below seems much simpler. It's possible the older version of keyword()
(before 26ea9e1, in 5.14 and later) requires the keyword nul-terminated,
so something closer to my first version may still be needed for porting
to 5.12 and earlier.

Hugo


commit 128e9aa798d9b1fe194ebf84c97938fb8f720972
Author​: Hugo van der Sanden <hv@​crypt.org>
Date​: Fri Jan 16 12​:11​:32 2015 +0000

  intuit_more​: no need to copy before keyword check

Inline Patch
diff --git a/toke.c b/toke.c
index dfb5b20..01b0527 100644
--- a/toke.c
+++ b/toke.c
@@ -3827,11 +3827,10 @@ S_intuit_more(pTHX_ char *s)
 		    && !(last_un_char == '$' || last_un_char == '@'
 			 || last_un_char == '&')
 		    && isALPHA(*s) && s[1] && isALPHA(s[1])) {
-		    char *d = tmpbuf;
+		    char *d = s;
 		    while (isALPHA(*s))
-			*d++ = *s++;
-		    *d = '\0';
-		    if (keyword(tmpbuf, d - tmpbuf, 0))
+			s++;
+		    if (keyword(d, s - d, 0))
 			weight -= 150;
 		}
 		if (un_char == last_un_char + 1)
@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 17, 2015

From @hvds

I hope that someone other than me will comment at some point.

I can't decide whether this bug merits a CVE. It's probably exploitable
(with difficulty), but can be triggered for privilege escalation only if
you compile a regular expression from untrusted data; if I remember right,
we've decided on previous occasions that our traditional warnings against
doing that were strong enough that no CVE was required, but I don't recall
if those were for issues of equivalent severity when triggered.

That it is triggered at compile-time also makes it worse​: I'm not sure
the traditional warnings would cover people doing trial eval of regexp
compilation from untrusted data, even if they never actually feed a string
to the resulting regexp object.

Hugo

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 17, 2015

From @iabyn

On Sat, Jan 17, 2015 at 01​:28​:56PM +0000, hv@​crypt.org wrote​:

I hope that someone other than me will comment at some point.

I can't decide whether this bug merits a CVE. It's probably exploitable
(with difficulty), but can be triggered for privilege escalation only if
you compile a regular expression from untrusted data; if I remember right,
we've decided on previous occasions that our traditional warnings against
doing that were strong enough that no CVE was required, but I don't recall
if those were for issues of equivalent severity when triggered.

That it is triggered at compile-time also makes it worse​: I'm not sure
the traditional warnings would cover people doing trial eval of regexp
compilation from untrusted data, even if they never actually feed a string
to the resulting regexp object.

If I've understood correctly, the code is failing when parsing a *literal*
pattern, i.e. a silly array index within an interpolated string.

Thus there is no way that someone can can pass a non-literal pattern
string to the regex engine and trigger the failure; i.e. this isn't
exploitable​:

  while (<>) {
  chomp;
  $foo =~ /$_/;
  }

And if some code allows people to specify *literal* patterns, then the
code is already insecure​:

  /foo$a[do { BEGIN { system "rm -rf /" } 1}]/

--
Justice is when you get what you deserve.
Law is when you get what you pay for.

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 18, 2015

From @hvds

Dave Mitchell <davem@​iabyn.com> wrote​:
:If I've understood correctly, the code is failing when parsing a *literal*
:pattern, i.e. a silly array index within an interpolated string.

Yes, I believe so.

:Thus there is no way that someone can can pass a non-literal pattern
:string to the regex engine and trigger the failure; i.e. this isn't
:exploitable​:
:
: while (<>) {
: chomp;
: $foo =~ /$_/;
: }
:
:And if some code allows people to specify *literal* patterns, then the
:code is already insecure​:
:
: /foo$a[do { BEGIN { system "rm -rf /" } 1}]/

Yep, makes sense to me.

In that case I'll aim to push the fix on Sunday or Monday.

Hugo

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 20, 2015

From @hvds

I wrote​:
:In that case I'll aim to push the fix on Sunday or Monday.

Well, close.

Now pushed as 56f81af.

I can't actually see these tickets in RT, but I think it should be closed,
disembargoed, and the fix considered for inclusion to maint.

Hugo

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 20, 2015

From @khwilliamson

On 01/20/2015 06​:33 AM, hv@​crypt.org wrote​:

I wrote​:
:In that case I'll aim to push the fix on Sunday or Monday.

Well, close.

Now pushed as 56f81af.

I can't actually see these tickets in RT, but I think it should be closed,
disembargoed, and the fix considered for inclusion to maint.

Hugo

hv++

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 20, 2015

From @steve-m-hay

On Tue Jan 20 05​:33​:44 2015, hv wrote​:

I wrote​:
:In that case I'll aim to push the fix on Sunday or Monday.

Well, close.

Now pushed as 56f81af.

I can't actually see these tickets in RT, but I think it should be closed,
disembargoed, and the fix considered for inclusion to maint.

Thanks, I'm marking this ticket as "resolved" (since there doesn't seem to be a "pending release" here?) and added 56f81af to the cherry-pick-votes file for maint-5.20 (in be11ff5).

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jan 20, 2015

@steve-m-hay - Status changed from 'open' to 'resolved'

@p5pRT p5pRT closed this Jan 20, 2015
@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Apr 10, 2015

From @rjbs

Barring any objection, I will move this to the perl5 queue on Sunday.

--
rjbs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.