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 pointer deref Perl_mess_sv (util.c:1508) #15538

Open
p5pRT opened this issue Aug 21, 2016 · 14 comments
Open

null pointer deref Perl_mess_sv (util.c:1508) #15538

p5pRT opened this issue Aug 21, 2016 · 14 comments

Comments

@p5pRT
Copy link

p5pRT commented Aug 21, 2016

Migrated from rt.perl.org#129027 (status was 'open')

Searchable as RT129027$

@p5pRT
Copy link
Author

p5pRT commented Aug 21, 2016

From @geeknik

The following script triggers a null pointer deref (Perl_mess_sv (util.c​:1508)) in Perl v5.25.4 (v5.25.3-305-g8c6b0c7). May only affect -DDEBUGGING builds because removing the `D` flag stifles the crash.

#!perl -D200000
${qq$\x5F$}=q 0f0and s 0.0qq e&$&e0ee

free op at 61900000db00, recorded in slab 61500000fa80 at (eval 1) line 1.
ASAN​:SIGSEGV

==6436==ERROR​: AddressSanitizer​: SEGV on unknown address 0x000000000021 (pc 0x0000007fdc8f bp 0x7ffeef401250 sp 0x7ffeef4011a0 T0)
  #0 0x7fdc8e in Perl_mess_sv /root/perl/util.c​:1508​:6
  #1 0x7fd28b in Perl_vmess /root/perl/util.c​:1561​:12
  #2 0x7fd28b in Perl_mess /root/perl/util.c​:1391
  #3 0x4e0415 in Perl_Slab_Free /root/perl/op.c​:442​:5
  #4 0x4e215d in Perl_op_free /root/perl/op.c​:855​:9
  #5 0x4e1da5 in Perl_op_free /root/perl/op.c​:837​:21
  #6 0xa23982 in Perl_leave_scope /root/perl/scope.c​:1109​:6
  #7 0xa56785 in S_pop_eval_context_maybe_croak /root/perl/pp_ctl.c​:1605​:5
  #8 0xa55e46 in Perl_die_unwind /root/perl/pp_ctl.c​:1733​:13
  #9 0x7ff91f in Perl_vcroak /root/perl/util.c​:1791​:5
  #10 0x7ff80c in Perl_die /root/perl/util.c​:1722​:5
  #11 0x8dc64f in Perl_pp_entersub /root/perl/pp_hot.c​:3826​:13
  #12 0x7f1b53 in Perl_runops_debug /root/perl/dump.c​:2234​:23
  #13 0x5a0ff6 in S_run_body /root/perl/perl.c​:2524​:2
  #14 0x5a0ff6 in perl_run /root/perl/perl.c​:2447
  #15 0x4de68d in main /root/perl/perlmain.c​:123​:9
  #16 0x7f0c50ba2b44 in __libc_start_main /build/glibc-uPj9cH/glibc-2.19/csu/libc-start.c​:287
  #17 0x4de2fc in _start (/root/perl/perl+0x4de2fc)

AddressSanitizer can not provide additional info.
SUMMARY​: AddressSanitizer​: SEGV /root/perl/util.c​:1508 Perl_mess_sv
==6436==ABORTING

@p5pRT
Copy link
Author

p5pRT commented Aug 21, 2016

From @cpansprout

On Sat Aug 20 21​:53​:50 2016, brian.carpenter@​gmail.com wrote​:

The following script triggers a null pointer deref (Perl_mess_sv
(util.c​:1508)) in Perl v5.25.4 (v5.25.3-305-g8c6b0c7). May only affect
-DDEBUGGING builds because removing the `D` flag stifles the crash.

#!perl -D200000

Equivalent to -DTSuxfP, but -DS alone is sufficient.

${qq$\x5F$}=q 0f0and s 0.0qq e&$&e0ee

${qq$\x5F$} is the strangest way I’ve seen of writing $_. :-)

Reduced and slightly readable​:

./miniperl -DS -e '$_="f"; s/./"&".$&/ee'

The string assigned to $_ can be anything with length to it.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Aug 21, 2016

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

@p5pRT
Copy link
Author

p5pRT commented Aug 25, 2016

From riusksk@qq.com

gdb-peda$ run poc.pl
Program received signal SIGSEGV, Segmentation fault.
[----------------------------------registers-----------------------------------]
RAX​: 0x2139 ('9!')
RBX​: 0xc2a888 --> 0xc0c6a0 --> 0x3f ('?')
RCX​: 0xbf8120 --> 0x40000000000
RDX​: 0xbf8120 --> 0x40000000000
RSI​: 0x0
RDI​: 0x0
RBP​: 0xc4
RSP​: 0x7fffffffd9d0 --> 0x8ba6b2 ("free op at %p, recorded in slab %p")
RIP​: 0x5e500f (<Perl_mess_sv+703>​: test BYTE PTR [rdi+0x21],0x40)
R8 : 0x0
R9 : 0xc2be70 --> 0xc2bdf0 --> 0xc2bdd0 --> 0x0
R10​: 0x20 (' ')
R11​: 0xfffffffffffffffc
R12​: 0xfffffffffffffffc
R13​: 0xffffffffffffffff
R14​: 0x8ba6b2 ("free op at %p, recorded in slab %p")
R15​: 0xc2a888 --> 0xc0c6a0 --> 0x3f ('?')
EFLAGS​: 0x10246 (carry PARITY adjust ZERO sign trap INTERRUPT direction overflow)
[-------------------------------------code-------------------------------------]
  0x5e4ffd <Perl_mess_sv+685>​: mov DWORD PTR fs​:[r12],0x17f3
  0x5e5006 <Perl_mess_sv+694>​: mov rdi,QWORD PTR [rip+0x6127e3] # 0xbf77f0 <PL_curcop>
  0x5e500d <Perl_mess_sv+701>​: xor esi,esi
=> 0x5e500f <Perl_mess_sv+703>​: test BYTE PTR [rdi+0x21],0x40
  0x5e5013 <Perl_mess_sv+707>​: je 0x5e5037 <Perl_mess_sv+743>
  0x5e5015 <Perl_mess_sv+709>​: movsxd rax,DWORD PTR fs​:[r12]
  0x5e501a <Perl_mess_sv+714>​: mov rcx,QWORD PTR [rip+0x612297] # 0xbf72b8 <__afl_area_ptr>
  0x5e5021 <Perl_mess_sv+721>​: xor rax,0xc2cf
[------------------------------------stack-------------------------------------]
0000| 0x7fffffffd9d0 --> 0x8ba6b2 ("free op at %p, recorded in slab %p")
0008| 0x7fffffffd9d8 --> 0xc2a888 --> 0xc0c6a0 --> 0x3f ('?')
0016| 0x7fffffffd9e0 --> 0x38931a43dc3a1100
0024| 0x7fffffffd9e8 --> 0xc2a888 --> 0xc0c6a0 --> 0x3f ('?')
0032| 0x7fffffffd9f0 --> 0xfffffffffffffffc
0040| 0x7fffffffd9f8 --> 0x8ba6b2 ("free op at %p, recorded in slab %p")
0048| 0x7fffffffda00 --> 0xc29f40 --> 0xc29fd0 --> 0xc2a010 --> 0xc2a058 --> 0xc2a0b8 --> 0xc2a0f8 --> 0xc2a138 --> 0x0
0056| 0x7fffffffda08 --> 0x5e490b (<Perl_mess+299>​: mov rcx,QWORD PTR fs​:0x28)
[------------------------------------------------------------------------------]
Legend​: code, data, rodata, value
Stopped reason​: SIGSEGV
0x00000000005e500f in Perl_mess_sv (basemsg=<optimized out>, consume=<optimized out>) at util.c​:1508
1508 closest_cop(PL_curcop, OpSIBLING(PL_curcop), PL_op, FALSE);
gdb-peda$ list
1503 * can try to find such a cop by searching through the optree starting
1504 * from the sibling of PL_curcop.
1505 */
1506
1507 const COP *cop =
1508 closest_cop(PL_curcop, OpSIBLING(PL_curcop), PL_op, FALSE);
1509 if (!cop)
1510 cop = PL_curcop;
1511
1512 if (CopLINE(cop))
gdb-peda$ exploitable
Undefined command​: "exploitable". Try "help".
gdb-peda$ source /usr/local/lib/python2.7/dist-packages/exploitable-1.32-py2.7.egg/exploitable/exploitable.py
gdb-peda$ exploitable
Description​: Access violation near NULL on destination operand
Short description​: DestAvNearNull (15/22)
Hash​: 1a151c037389315db0b68bce2f05b76e.560e17b3b11ae333722a75a54229d3d5
Exploitability Classification​: PROBABLY_EXPLOITABLE
Explanation​: The target crashed on an access violation at an address matching the destination operand of the instruction. This likely indicates a write access violation, which means the attacker may control write address and/or value. However, it there is a chance it could be a NULL dereference.
Other tags​: AccessViolation (21/22)

root@​Ubuntu​:~/perl# valgrind perl poc.pl

==22051== ERROR SUMMARY​: 1 errors from 1 contexts (suppressed​: 0 from 0)
==22051==
==22051== 1 errors in context 1 of 1​:
==22051== Invalid read of size 1
==22051== at 0x5E500F​: Perl_mess_sv (util.c​:1508)
==22051== by 0x5E490A​: Perl_vmess (util.c​:1561)
==22051== by 0x5E490A​: Perl_mess (util.c​:1391)
==22051== by 0x42044A​: Perl_Slab_Free (op.c​:442)
==22051== by 0x42192D​: Perl_op_free (op.c​:855)
==22051== by 0x421679​: Perl_op_free (op.c​:837)
==22051== by 0x731543​: Perl_leave_scope (scope.c​:1109)
==22051== by 0x74EAA0​: S_pop_eval_context_maybe_croak (pp_ctl.c​:1605)
==22051== by 0x74E55D​: Perl_die_unwind (pp_ctl.c​:1733)
==22051== by 0x5E6558​: Perl_vcroak (util.c​:1791)
==22051== by 0x5E6336​: Perl_die (util.c​:1722)
==22051== by 0x760728​: S_require_file (pp_ctl.c​:4062)
==22051== by 0x760728​: Perl_pp_require (pp_ctl.c​:4138)
==22051== by 0x5DD02C​: Perl_runops_debug (dump.c​:2234)
==22051== Address 0x21 is not stack'd, malloc'd or (recently) free'd
==22051==
==22051== ERROR SUMMARY​: 1 errors from 1 contexts (suppressed​: 0 from 0)
Segmentation fault

@p5pRT
Copy link
Author

p5pRT commented Aug 25, 2016

From riusksk@qq.com

poc1.pl

@p5pRT
Copy link
Author

p5pRT commented Aug 25, 2016

From @dcollinsn

Looks to me like the following patch is enough to resolve this issue. Valgrind is clean, and no failures with make test.

From 76d06fab84e6fdc64a1c24a335bbd3f40ee4a32e Mon Sep 17 00​:00​:00 2001
From​: Dan Collins <dcollinsn@​gmail.com>
Date​: Thu, 25 Aug 2016 09​:54​:49 -0400
Subject​: [PATCH] RT #129027​: Null pointer check


util.c | 3 +++
1 file changed, 3 insertions(+)

Inline Patch
diff --git a/util.c b/util.c
index 7748c6c..17dd69c 100644
--- a/util.c
+++ b/util.c
@@ -1504,6 +1504,7 @@ Perl_mess_sv(pTHX_ SV *basemsg, bool consume)
         * from the sibling of PL_curcop.
         */

+        if (PL_curcop) {
        const COP *cop =
            closest_cop(PL_curcop, OpSIBLING(PL_curcop), PL_op, FALSE);
        if (!cop)
@@ -1512,6 +1513,8 @@ Perl_mess_sv(pTHX_ SV *basemsg, bool consume)
        if (CopLINE(cop))
            Perl_sv_catpvf(aTHX_ sv, " at %s line %"IVdf,
            OutCopFILE(cop), (IV)CopLINE(cop));
+        }
+
        /* Seems that GvIO() can be untrustworthy during global destruction. */
        if (GvIO(PL_last_in_gv) && (SvTYPE(GvIOp(PL_last_in_gv)) == SVt_PVIO)
                && IoLINES(GvIOp(PL_last_in_gv)))
--
2.8.1

Thoughts? (Should there be a PL_curcop here, or is it OK for it to be null? I found this old thread which may be relevant​: http​://www.nntp.perl.org/group/perl.perl5.porters/2013/08/msg205604.html )

--
Dan Collins

@p5pRT
Copy link
Author

p5pRT commented Aug 25, 2016

From @dcollinsn

This appears to be a very complicated duplicate of RT #129027. This one spews debug output for about 30 seconds, and it doesn't actually segfault for me.

It's a null deref, the pointer is correctly zeroed and then incorrectly used (it's dereferencing 0x21 because that's the offset into the struct), so not a security bug. My patch in that ticket should also fix this, but I'm not sure because I can't actually repro.

--
Respectfully,
Dan Collins

@p5pRT
Copy link
Author

p5pRT commented Aug 25, 2016

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

@p5pRT
Copy link
Author

p5pRT commented Aug 25, 2016

From @cpansprout

On Thu Aug 25 06​:58​:45 2016, dcollinsn@​gmail.com wrote​:

Looks to me like the following patch is enough to resolve this issue.
Valgrind is clean, and no failures with make test.

From 76d06fab84e6fdc64a1c24a335bbd3f40ee4a32e Mon Sep 17 00​:00​:00 2001
From​: Dan Collins <dcollinsn@​gmail.com>
Date​: Thu, 25 Aug 2016 09​:54​:49 -0400
Subject​: [PATCH] RT #129027​: Null pointer check

---
util.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/util.c b/util.c
index 7748c6c..17dd69c 100644
--- a/util.c
+++ b/util.c
@​@​ -1504,6 +1504,7 @​@​ Perl_mess_sv(pTHX_ SV *basemsg, bool consume)
* from the sibling of PL_curcop.
*/

+ if (PL_curcop) {
const COP *cop =
closest_cop(PL_curcop, OpSIBLING(PL_curcop), PL_op,
FALSE);
if (!cop)
@​@​ -1512,6 +1513,8 @​@​ Perl_mess_sv(pTHX_ SV *basemsg, bool consume)
if (CopLINE(cop))
Perl_sv_catpvf(aTHX_ sv, " at %s line %"IVdf,
OutCopFILE(cop), (IV)CopLINE(cop));
+ }
+
/* Seems that GvIO() can be untrustworthy during global
destruction. */
if (GvIO(PL_last_in_gv) && (SvTYPE(GvIOp(PL_last_in_gv)) ==
SVt_PVIO)
&& IoLINES(GvIOp(PL_last_in_gv)))
--
2.8.1

Thoughts? (Should there be a PL_curcop here, or is it OK for it to be
null? I found this old thread which may be relevant​:
http​://www.nntp.perl.org/group/perl.perl5.porters/2013/08/msg205604.html
)

Yes, it is relevant. The backtrace shows that the PL_curcop is being read when ops are being freed. One of those ops is almost certainly a cop (too lazy to check) which happened to be PL_curcop, so freeing it set PL_curcop to null.

I do wonder, though, whether we should be adding more conditions to normal code for the sake of debugging code. That said, errors and warnings are certainly not hot code, so your fix, the simpler fix, is probably fine.

(gdb) bt
#0 0x000000010020299a in Perl_mess_sv (my_perl=0x100803200, basemsg=0x1008308f8, consume=1 '\001') at util.c​:1508
#1 0x0000000100203f58 in Perl_vmess (my_perl=0x100803200, pat=0x10056d800 "free op at %p, recorded in slab %p", args=0x7fff5fbfd4d0) at util.c​:1561
#2 0x0000000100211833 in Perl_mess (my_perl=0x100803200, pat=0x10056d800 "free op at %p, recorded in slab %p") at util.c​:1391
#3 0x000000010001bfbc in Perl_Slab_Free (my_perl=0x100803200, op=0x10070ffd8) at op.c​:442
#4 0x000000010001cafe in Perl_op_free (my_perl=0x100803200, o=0x10070ffd8) at op.c​:855
#5 0x000000010001c95e in Perl_op_free (my_perl=0x100803200, o=0x10080ebc0) at op.c​:837
#6 0x00000001003a8c8e in Perl_leave_scope (my_perl=0x100803200, base=0) at scope.c​:1109
...

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Aug 25, 2016

From @cpansprout

On Thu Aug 25 13​:43​:19 2016, sprout wrote​:

I do wonder, though, whether we should be adding more conditions to
normal code for the sake of debugging code. That said, errors and
warnings are certainly not hot code, so your fix, the simpler fix, is
probably fine.

This jogged my memory a bit....

When I added -DS, I did wonder whether it was appropriate to use SV-using diagnostic code for such low-level functionality. I decided not to worry about it, since I doubted that anyone would be using that option except during the first few months after the op slab allocator was added.

It seems that the misgivings that I ignored were correct. I do think a more appropriate fix would be to change DEBUG_S_warn in op.c to be smarter and bypass mess_sv and anything SVish. It’s more work though.

Fixing anything related to -D is certainly not high priority.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Aug 26, 2016

From riusksk@qq.com

在2016-八月-25 07​:16​:34 星期四时,dcollinsn@​gmail.com写到:

This appears to be a very complicated duplicate of RT #129027. This
one spews debug output for about 30 seconds, and it doesn't actually
segfault for me.

It's a null deref, the pointer is correctly zeroed and then
incorrectly used (it's dereferencing 0x21 because that's the offset
into the struct), so not a security bug. My patch in that ticket
should also fix this, but I'm not sure because I can't actually repro.

ok,thx, close it.

@p5pRT
Copy link
Author

p5pRT commented Aug 26, 2016

From @khwilliamson

On 08/25/2016 07​:50 PM, riusksk via RT wrote​:

在2016-八月-25 07​:16​:34 星期四时,dcollinsn@​gmail.com写到:

This appears to be a very complicated duplicate of RT #129027. This
one spews debug output for about 30 seconds, and it doesn't actually
segfault for me.

It's a null deref, the pointer is correctly zeroed and then
incorrectly used (it's dereferencing 0x21 because that's the offset
into the struct), so not a security bug. My patch in that ticket
should also fix this, but I'm not sure because I can't actually repro.

ok,thx, close it.

---
via perlbug​: queue​: perl5 status​: open
https://rt-archive.perl.org/perl5/Ticket/Display.html?id=129075

Instead of closing it, I merged it into #129027. You will then get
notified when that gets fixed, and if your issue isn't actually fixed by
that, this can be reopened.

@p5pRT
Copy link
Author

p5pRT commented Sep 12, 2016

From @atoomic

a simpler way to patch this would be to add a protection on the PL_curcop

Inline Patch
diff --git a/util.c b/util.c
index a69ddad..a1e44b4 100644
--- a/util.c
+++ b/util.c
@@ -1522,7 +1522,7 @@ Perl_mess_sv(pTHX_ SV *basemsg, bool consume)
        sv_copypv(sv, basemsg);
     }

-    if (!SvCUR(sv) || *(SvEND(sv) - 1) != '\n') {
+    if (PL_curcop && (!SvCUR(sv) || *(SvEND(sv) - 1) != '\n')) {
        /*
         * Try and find the file and line for PL_op.  This will usually be
         * PL_curcop, but it might be a cop that has been optimised away.  We

On Fri Aug 26 07:18:51 2016, public@khwilliamson.com wrote: > On 08/25/2016 07​:50 PM\, riusksk via RT wrote​: > > 在2016\-八月\-25 07​:16​:34 星期四时,dcollinsn@​gmail\.com写到: > >> This appears to be a very complicated duplicate of RT \#129027\. This > >> one spews debug output for about 30 seconds\, and it doesn't actually > >> segfault for me\. > >> > >> It's a null deref\, the pointer is correctly zeroed and then > >> incorrectly used \(it's dereferencing 0x21 because that's the offset > >> into the struct\)\, so not a security bug\. My patch in that ticket > >> should also fix this\, but I'm not sure because I can't actually repro\. > > > > > > ok\,thx\, close it\. > > > > \-\-\- > > via perlbug​: queue​: perl5 status​: open > > https://rt-archive.perl.org/perl5/Ticket/Display.html?id=129075 > > > > Instead of closing it\, I merged it into \#129027\. You will then get > notified when that gets fixed\, and if your issue isn't actually fixed by > that\, this can be reopened\. >

@p5pRT
Copy link
Author

p5pRT commented Sep 12, 2016

From @atoomic

0001-protect-Perl_mess_sv-against-undefined-PL_curcop.patch
From 3a2d2f9878ea8751e32098b5301c342b74013600 Mon Sep 17 00:00:00 2001
From: Nicolas R <atoomic@cpan.org>
Date: Mon, 12 Sep 2016 09:26:39 -0600
Subject: [PATCH 1/1] protect Perl_mess_sv against undefined PL_curcop

---
 util.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/util.c b/util.c
index a69ddad..a1e44b4 100644
--- a/util.c
+++ b/util.c
@@ -1522,7 +1522,7 @@ Perl_mess_sv(pTHX_ SV *basemsg, bool consume)
 	sv_copypv(sv, basemsg);
     }
 
-    if (!SvCUR(sv) || *(SvEND(sv) - 1) != '\n') {
+    if (PL_curcop && (!SvCUR(sv) || *(SvEND(sv) - 1) != '\n')) {
 	/*
 	 * Try and find the file and line for PL_op.  This will usually be
 	 * PL_curcop, but it might be a cop that has been optimised away.  We
-- 
2.10.0

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

2 participants