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

heap-buffer-overflow Perl_pp_chdir (pp_sys.c:3685) #15569

Closed
p5pRT opened this issue Aug 29, 2016 · 8 comments

Comments

@p5pRT
Copy link
Collaborator

@p5pRT p5pRT commented Aug 29, 2016

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

Searchable as RT129130$

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Aug 29, 2016

From @geeknik

Found with AFL and Perl v5.25.4-20-gc2f7c0b*. I've attached the
non-minimized script because afl-tmin minimizes the crash away (again).

==26581==ERROR​: AddressSanitizer​: heap-buffer-overflow on address
0x619000009a80 at pc 0x000000ae54f7 bp 0x7ffc33c56be0 sp 0x7ffc33c56bd8
WRITE of size 8 at 0x619000009a80 thread T0
  #0 0xae54f6 in Perl_pp_chdir /root/perl/pp_sys.c​:3685​:9
  #1 0x7f1c63 in Perl_runops_debug /root/perl/dump.c​:2234​:23
  #2 0x5a10a6 in S_run_body /root/perl/perl.c​:2525​:2
  #3 0x5a10a6 in perl_run /root/perl/perl.c​:2448
  #4 0x4de6cd in main /root/perl/perlmain.c​:123​:9
  #5 0x7f32be284b44 in __libc_start_main
/build/glibc-uPj9cH/glibc-2.19/csu/libc-start.c​:287
  #6 0x4de33c in _start (/root/perl/perl+0x4de33c)

0x619000009a80 is located 0 bytes to the right of 1024-byte region
[0x619000009680,0x619000009a80)
allocated by thread T0 here​:
  #0 0x4c0cbb in malloc (/root/perl/perl+0x4c0cbb)
  #1 0x7f5aa7 in Perl_safesysmalloc /root/perl/util.c​:153​:21

SUMMARY​: AddressSanitizer​: heap-buffer-overflow /root/perl/pp_sys.c​:3685
Perl_pp_chdir
Shadow bytes around the buggy address​:
  0x0c327fff9300​: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c327fff9310​: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c327fff9320​: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c327fff9330​: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c327fff9340​: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c327fff9350​:[fa]fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c327fff9360​: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c327fff9370​: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c327fff9380​: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c327fff9390​: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c327fff93a0​: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes)​:
  Addressable​: 00
  Partially addressable​: 01 02 03 04 05 06 07
  Heap left redzone​: fa
  Heap right redzone​: fb
  Freed heap region​: fd
  Stack left redzone​: f1
  Stack mid redzone​: f2
  Stack right redzone​: f3
  Stack partial redzone​: f4
  Stack after return​: f5
  Stack use after scope​: f8
  Global redzone​: f9
  Global init order​: f6
  Poisoned by user​: f7
  Container overflow​: fc
  ASan internal​: fe
==26581==ABORTING

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Aug 29, 2016

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Sep 5, 2016

From @tonycoz

On Mon Aug 29 13​:54​:26 2016, brian.carpenter@​gmail.com wrote​:

Found with AFL and Perl v5.25.4-20-gc2f7c0b*. I've attached the
non-minimized script because afl-tmin minimizes the crash away (again).

chdir() isn't allocating stack for its result when called with no argument.

Most other ops default to $_ and are compiled that way, for example​:

tony@​mars​:.../git/perl$ ./perl -Ilib -MO=Concise -e sin
5 <@​> leave[1 ref] vKP/REFC ->(end)
1 <0> enter ->2
2 <;> nextstate(main 1 -e​:1) v​:{ ->3
4 <1> sin[t2] vK/1 ->5
- <1> ex-rv2sv sK/1 ->4
3 <$> gvsv(*_) s ->4

chdir() doesn't get that faked argument​:

tony@​mars​:.../git/perl$ ./perl -Ilib -MO=Concise -e chdir
4 <@​> leave[1 ref] vKP/REFC ->(end)
1 <0> enter ->2
2 <;> nextstate(main 1 -e​:1) v​:{ ->3
3 <0> chdir[t1] v ->4

since chdir() with no argument has it's own special behaviour.

A simpler reproducer​:

for $x (map $_+1, 1 .. 100) {
  map chdir, 1 .. $x;
}

Fix attached.

I don't think this would be exploitable beyond a denial of service if it
crashes perl - it only writes one pointersize beyond the end of the
allocated block, and is always the address of a new SV (should this use
PL_sv_yes and PL_sv_no?)

Tony

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Sep 5, 2016

From @tonycoz

0001-perl-129130-make-chdir-allocate-the-stack-it-needs.patch
From d6151b2a56d80af849778fda3364f392ea9032b1 Mon Sep 17 00:00:00 2001
From: Tony Cook <tony@develop-help.com>
Date: Mon, 5 Sep 2016 15:40:11 +1000
Subject: (perl #129130) make chdir allocate the stack it needs

chdir with no argument didn't ensure there was stack space available
for its result.
---
 pp_sys.c     | 1 +
 t/op/chdir.t | 8 +++++++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/pp_sys.c b/pp_sys.c
index a198d4e..7d74ea6 100644
--- a/pp_sys.c
+++ b/pp_sys.c
@@ -3639,6 +3639,7 @@ PP(pp_chdir)
 	HV * const table = GvHVn(PL_envgv);
 	SV **svp;
 
+        EXTEND(SP, 1);
         if (    (svp = hv_fetchs(table, "HOME", FALSE))
              || (svp = hv_fetchs(table, "LOGDIR", FALSE))
 #ifdef VMS
diff --git a/t/op/chdir.t b/t/op/chdir.t
index 9967707..38cbbe9 100644
--- a/t/op/chdir.t
+++ b/t/op/chdir.t
@@ -12,7 +12,7 @@ BEGIN {
     set_up_inc(qw(t . lib ../lib));
 }
 
-plan(tests => 47);
+plan(tests => 48);
 
 use Config;
 use Errno qw(ENOENT EBADF EINVAL);
@@ -162,6 +162,12 @@ sub check_env {
     }
 }
 
+fresh_perl_is(<<'EOP', '', { stderr => 1 }, "check stack handling");
+for $x (map $_+1, 1 .. 100) {
+  map chdir, 1 .. $x;
+}
+EOP
+
 my %Saved_Env = ();
 sub clean_env {
     foreach my $env (@magic_envs) {
-- 
2.1.4

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Sep 5, 2016

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

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Oct 24, 2016

From @tonycoz

On Sun Sep 04 22​:50​:16 2016, tonyc wrote​:

Fix attached.

Applied as 92c843f.

I don't think this would be exploitable beyond a denial of service if it
crashes perl - it only writes one pointersize beyond the end of the
allocated block, and is always the address of a new SV (should this use
PL_sv_yes and PL_sv_no?)

And made this public.

Tony

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Dec 6, 2017

From zefram@fysh.org

This bug was fixed by Tony's patch. This ticket should be closed.

-zefram

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Dec 7, 2017

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

@p5pRT p5pRT closed this Dec 7, 2017
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.