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 -> Perl_parse_unicode_opts () at util.c:4425 #14562

Closed
p5pRT opened this issue Mar 5, 2015 · 13 comments
Closed

null ptr deref -> Perl_parse_unicode_opts () at util.c:4425 #14562

p5pRT opened this issue Mar 5, 2015 · 13 comments

Comments

@p5pRT
Copy link

p5pRT commented Mar 5, 2015

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

Searchable as RT123991$

@p5pRT
Copy link
Author

p5pRT commented Mar 5, 2015

From @geeknik

Built v5.21.10 (v5.21.9-73-gd98e5cd) with the following command line​:

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

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

Valgrind​:
==7644== Invalid read of size 1
==7644== at 0x8069FF​: Perl_parse_unicode_opts (util.c​:4425)
==7644== by 0x63165B​: Perl_yylex (toke.c​:4943)
==7644== by 0x65AC04​: Perl_yyparse (perly.c​:322)
==7644== by 0x531D30​: S_parse_body (perl.c​:2277)
==7644== by 0x539532​: perl_parse (perl.c​:1611)
==7644== by 0x42AED7​: main (perlmain.c​:114)
==7644== Address 0x0 is not stack'd, malloc'd or (recently) free'd
==7644==
==7644==
==7644== Process terminating with default action of signal 11 (SIGSEGV)​: dumping core
==7644== Access not within mapped region at address 0x0
==7644== at 0x8069FF​: Perl_parse_unicode_opts (util.c​:4425)
==7644== by 0x63165B​: Perl_yylex (toke.c​:4943)
==7644== by 0x65AC04​: Perl_yyparse (perly.c​:322)
==7644== by 0x531D30​: S_parse_body (perl.c​:2277)
==7644== by 0x539532​: perl_parse (perl.c​:1611)
==7644== by 0x42AED7​: main (perlmain.c​:114)
==7644== If you believe this happened as a result of a stack
==7644== overflow in your program's main thread (unlikely but
==7644== possible), you can try to increase the size of the
==7644== main thread stack using the --main-stacksize= flag.
==7644== The main thread stack size used in this run was 8388608.
Segmentation fault

GDB​:
gdb-peda$ file ~/perl/perl
gdb-peda$ set args test17-min
gdb-peda$ r
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".

Program received signal SIGSEGV, Segmentation fault.
[----------------------------------registers-----------------------------------]
RAX​: 0xffffffffffffffff
RBX​: 0x7fffffffda78 --> 0x11eb029 --> 0x3030 ('00')
RCX​: 0x0
RDX​: 0x0
RSI​: 0x7fffffffd9e0 --> 0x0
RDI​: 0x0
RBP​: 0x11eb020 ("#!perl -C00")
RSP​: 0x7fffffffd9e0 --> 0x0
RIP​: 0x8069ff (<Perl_parse_unicode_opts+1087>​: movzx r11d,BYTE PTR [rcx])
R8 : 0x11eb02a --> 0x30 ('0')
R9 : 0x7fffffffd9e0 --> 0x0
R10​: 0x0
R11​: 0x30 ('0')
R12​: 0x11eb028 --> 0x303043 ('C00')
R13​: 0x0
R14​: 0x0
R15​: 0x0
EFLAGS​: 0x10206 (carry PARITY adjust zero sign trap INTERRUPT direction overflow)
[-------------------------------------code-------------------------------------]
  0x8069f3 <Perl_parse_unicode_opts+1075>​: mov rsi,rsp
  0x8069f6 <Perl_parse_unicode_opts+1078>​: call 0xdcf610 <Perl_grok_atou>
  0x8069fb <Perl_parse_unicode_opts+1083>​: mov rcx,QWORD PTR [rsp]
=> 0x8069ff <Perl_parse_unicode_opts+1087>​: movzx r11d,BYTE PTR [rcx]
  0x806a03 <Perl_parse_unicode_opts+1091>​: cmp r11b,0xa
  0x806a07 <Perl_parse_unicode_opts+1095>​: setne dl
  0x806a0a <Perl_parse_unicode_opts+1098>​: test r11b,r11b
  0x806a0d <Perl_parse_unicode_opts+1101>​: setne dil
[------------------------------------stack-------------------------------------]
0000| 0x7fffffffd9e0 --> 0x0
0008| 0x7fffffffd9e8 --> 0x45da6e43a59fc800
0016| 0x7fffffffd9f0 --> 0x0
0024| 0x7fffffffd9f8 --> 0x63165c (<Perl_yylex+381068>​: cmp eax,DWORD PTR [rip+0xb9a486] # 0x11cbae8 <PL_unicode>)
0032| 0x7fffffffda00 --> 0xffffefbd57760000
0040| 0x7fffffffda08 --> 0x7fff00000000
0048| 0x7fffffffda10 --> 0x7fffffffda00 --> 0xffffefbd57760000
0056| 0x7fffffffda18 --> 0x7fffffffdd90 --> 0x11eac70 --> 0x0
[------------------------------------------------------------------------------]
Legend​: code, data, rodata, value
Stopped reason​: SIGSEGV
Perl_parse_unicode_opts () at util.c​:4425
4425 if (*p && *p != '\n' && *p != '\r') {
gdb-peda$ exploit
Description​: Access violation near NULL on source operand
Short description​: SourceAvNearNull (16/22)
Hash​: 0a4dc21ff15ea521d67c09e67134a13b.ffb4303733f33b8867640b47870629a8
Exploitability Classification​: PROBABLY_NOT_EXPLOITABLE
Explanation​: The target crashed on an access violation at an address matching the source operand of the current instruction. This likely indicates a read access violation, which may mean the application crashed on a simple NULL dereference to data structure that has no immediate effect on control of the processor.
Other tags​: AccessViolation (21/22)

Hexdump of 11-byte test case​:
0000000 2123 6570 6c72 2d20 3043 0030
000000b

Debian 7, Kernel 3.2.65-1+deb7u1 x86_64, GCC 4.9.2, libc 2.13-38+deb7u7

@p5pRT
Copy link
Author

p5pRT commented Mar 5, 2015

From @geeknik

test17-min

@p5pRT
Copy link
Author

p5pRT commented Mar 7, 2015

From @hvds

This is another failure to check for errors after grok_atou; a fix has already been prepared in the branch smoke-me/hv-grok (see http​://perl5.git.perl.org/perl.git/commitdiff/85dba01391#patch22).

Checking this case against that branch, it doesn't crash; but I'm also not sure it's doing the right thing, it probably needs an 'else croak' case on the 'if grokatoUV'.

Hugo

@p5pRT
Copy link
Author

p5pRT commented Mar 7, 2015

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

@p5pRT
Copy link
Author

p5pRT commented Mar 9, 2015

From @hvds

The work for [perl #123814] is now merged, which improves this from a crash to silently doing nothing; it needs further improvement to give an appropriate error instead, but that may not make the cut for 5.22 (since it will require adding a new error).

Hugo

@p5pRT
Copy link
Author

p5pRT commented Nov 26, 2015

From @tonycoz

On Mon Mar 09 15​:35​:07 2015, hv wrote​:

The work for [perl #123814] is now merged, which improves this from a
crash to silently doing nothing; it needs further improvement to give
an appropriate error instead, but that may not make the cut for 5.22
(since it will require adding a new error).

Something like the attached?

Tony

@p5pRT
Copy link
Author

p5pRT commented Nov 26, 2015

From @tonycoz

0001-perl-123991-report-an-error-if-we-can-t-parse-the-nu.patch
From b10ad801e7e7f28cb98536697ac01c26690a72f5 Mon Sep 17 00:00:00 2001
From: Tony Cook <tony@develop-help.com>
Date: Thu, 26 Nov 2015 11:18:52 +1100
Subject: [perl #123991] report an error if we can't parse the number after -C

---
 pod/perldiag.pod | 5 +++++
 t/run/switchC.t  | 8 +++++++-
 util.c           | 3 +++
 3 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/pod/perldiag.pod b/pod/perldiag.pod
index 5111410..15eb9d1 100644
--- a/pod/perldiag.pod
+++ b/pod/perldiag.pod
@@ -2639,6 +2639,11 @@ provides a list context to its subscript, which can do weird things
 if you're expecting only one subscript.  When called in list context,
 it also returns the key in addition to the value.
 
+=item Invalid number '%s' for -C option.
+
+(F) You supplied number to the -C option that either has extra leading
+zeroes or overflows perl's unsigned integer representation.
+
 =item %s() is deprecated on :utf8 handles
 
 (W deprecated) The sysread(), recv(), syswrite() and send() operators
diff --git a/t/run/switchC.t b/t/run/switchC.t
index 4f63c3b..6583010 100644
--- a/t/run/switchC.t
+++ b/t/run/switchC.t
@@ -11,7 +11,7 @@ BEGIN {
     skip_all_if_miniperl('-C and $ENV{PERL_UNICODE} are disabled on miniperl');
 }
 
-plan(tests => 14);
+plan(tests => 15);
 
 my $r;
 
@@ -111,3 +111,9 @@ SKIP: {
     like( $r, qr/^Too late for "-CS" option at -e line 1\.$/s,
           '#!perl -C but not command line' );
 }
+
+$r = runperl ( switches => [ '-C00' ],
+               prog    => '1',
+               stderr   => 1, );
+like($r, qr/^Invalid number '00' for -C option\.$/s,
+     "perl -C00 [perl #123991]");
diff --git a/util.c b/util.c
index aeec4c0..17b62dd 100644
--- a/util.c
+++ b/util.c
@@ -4538,6 +4538,9 @@ Perl_parse_unicode_opts(pTHX_ const char **popt)
                         Perl_croak(aTHX_ "Unknown Unicode option letter '%c'", *p);
                 }
             }
+            else {
+                Perl_croak(aTHX_ "Invalid number '%s' for -C option.\n", p);
+            }
         }
         else {
 	    for (; *p; p++) {
-- 
2.1.4

@p5pRT
Copy link
Author

p5pRT commented Nov 26, 2015

From @hvds

On Wed Nov 25 16​:19​:53 2015, tonyc wrote​:

Something like the attached?

That looks credible; I'll try to refresh my memory and look in more depth over the weekend (but feel free to poke me if I don't appear to have got to it).

Hugo

@p5pRT
Copy link
Author

p5pRT commented Nov 28, 2015

From @hvds

"Tony Cook via RT" <perlbug-followup@​perl.org> wrote​:
:Something like the attached?

Yes, that looks good to me.

Hugo

@p5pRT
Copy link
Author

p5pRT commented Dec 7, 2015

From @tonycoz

On Wed Nov 25 16​:19​:53 2015, tonyc wrote​:

On Mon Mar 09 15​:35​:07 2015, hv wrote​:

The work for [perl #123814] is now merged, which improves this from a
crash to silently doing nothing; it needs further improvement to give
an appropriate error instead, but that may not make the cut for 5.22
(since it will require adding a new error).

Something like the attached?

Applied as 817e3e2.

Tony

@p5pRT
Copy link
Author

p5pRT commented Dec 7, 2015

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

@p5pRT
Copy link
Author

p5pRT commented May 13, 2016

From @khwilliamson

Thank you for submitting this report. You have helped make Perl better.
 
With the release of Perl 5.24.0 on May 9, 2016, this and 149 other issues have been resolved.

Perl 5.24.0 may be downloaded via https://metacpan.org/release/RJBS/perl-5.24.0

@p5pRT p5pRT closed this as completed May 13, 2016
@p5pRT
Copy link
Author

p5pRT commented May 13, 2016

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

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