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

Jump to bottom

heap-use-after-free S_mro_gather_and_rename (mro_core.c:930) #15653

Open
p5pRT opened this issue Oct 12, 2016 · 8 comments
Open

heap-use-after-free S_mro_gather_and_rename (mro_core.c:930) #15653

p5pRT opened this issue Oct 12, 2016 · 8 comments

Comments

@p5pRT
Copy link

p5pRT commented Oct 12, 2016

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

Searchable as RT129861$
@p5pRT
Copy link
Author

p5pRT commented Oct 12, 2016

From @geeknik

Triggered in Perl v5.25.6 (v5.25.5-76-g91dca83) with AFL+ASAN.

==982==ERROR​: AddressSanitizer​: heap-use-after-free on address
0x60600000e420 at pc 0x00000085cff0 bp 0x7ffded1cb150 sp 0x7ffded1cb148
READ of size 8 at 0x60600000e420 thread T0
  #0 0x85cfef in S_mro_gather_and_rename /root/perl-blead/mro_core.c​:930​:2
  #1 0x85b4d4 in S_mro_gather_and_rename
/root/perl-blead/mro_core.c​:1186​:4
  #2 0x856bec in Perl_mro_package_moved /root/perl-blead/mro_core.c​:851​:5
  #3 0x93196d in S_glob_assign_glob /root/perl-blead/sv.c​:3981​:6
  #4 0x921fe2 in Perl_sv_setsv_flags /root/perl-blead/sv.c​:4462​:7
  #5 0x8a219e in Perl_pp_sassign /root/perl-blead/pp_hot.c​:226​:5
  #6 0x7f4493 in Perl_runops_debug /root/perl-blead/dump.c​:2246​:23
  #7 0x5a1226 in S_run_body /root/perl-blead/perl.c​:2526​:2
  #8 0x5a1226 in perl_run /root/perl-blead/perl.c​:2449
  #9 0x4de5cd in main /root/perl-blead/perlmain.c​:123​:9
  #10 0x7ff37132bb44 in __libc_start_main
/build/glibc-daoqzt/glibc-2.19/csu/libc-start.c​:287
  #11 0x4de23c in _start (/root/perl/perl+0x4de23c)

0x60600000e420 is located 0 bytes inside of 64-byte region
[0x60600000e420,0x60600000e460)
freed by thread T0 here​:
  #0 0x4c0eae in realloc (/root/perl/perl+0x4c0eae)
  #1 0x7f89c6 in Perl_safesysrealloc /root/perl-blead/util.c​:274​:18

previously allocated by thread T0 here​:
  #0 0x4c0d10 in calloc (/root/perl/perl+0x4c0d10)
  #1 0x7f9311 in Perl_safesyscalloc /root/perl-blead/util.c​:442​:18
  #2 0x875a35 in Perl_hv_common_key_len /root/perl-blead/hv.c​:333​:12
  #3 0x856bec in Perl_mro_package_moved /root/perl-blead/mro_core.c​:851​:5

SUMMARY​: AddressSanitizer​: heap-use-after-free
/root/perl-blead/mro_core.c​:930 S_mro_gather_and_rename
Shadow bytes around the buggy address​:
  0x0c0c7fff9c30​: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0c7fff9c40​: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0c7fff9c50​: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0c7fff9c60​: 00 00 00 00 00 00 00 00 fa fa fa fa 00 00 00 00
  0x0c0c7fff9c70​: 00 00 00 00 fa fa fa fa 00 00 00 00 00 00 00 00
=>0x0c0c7fff9c80​: fa fa fa fa[fd]fd fd fd fd fd fd fd fa fa fa fa
  0x0c0c7fff9c90​: fd fd fd fd fd fd fd fd fa fa fa fa fd fd fd fd
  0x0c0c7fff9ca0​: fd fd fd fd fa fa fa fa 00 00 00 00 00 00 00 00
  0x0c0c7fff9cb0​: fa fa fa fa fd fd fd fd fd fd fd fd fa fa fa fa
  0x0c0c7fff9cc0​: 00 00 00 00 00 00 00 06 fa fa fa fa 00 00 00 00
  0x0c0c7fff9cd0​: 00 00 06 fa fa fa fa fa fd fd fd fd fd fd fd fd
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
==982==ABORTING

Valgrind + Perl v5.25.6 (v5.25.5-76-g91dca83)​:
Operator or semicolon missing before *sreEdV at test84 line 1.
Ambiguous use of * resolved as operator * at test84 line 1.
==12501== Invalid read of size 8
==12501== at 0x4EE82C​: S_mro_gather_and_rename (mro_core.c​:930)
==12501== by 0x4EF08C​: S_mro_gather_and_rename (mro_core.c​:1186)
==12501== by 0x4F0B7D​: Perl_mro_package_moved (mro_core.c​:851)
==12501== by 0x52EDB1​: S_glob_assign_glob (sv.c​:3981)
==12501== by 0x521F77​: Perl_sv_setsv_flags (sv.c​:4462)
==12501== by 0x5001B5​: Perl_pp_sassign (pp_hot.c​:226)
==12501== by 0x4D7131​: Perl_runops_debug (dump.c​:2246)
==12501== by 0x453146​: S_run_body (perl.c​:2526)
==12501== by 0x453146​: perl_run (perl.c​:2449)
==12501== by 0x421944​: main (perlmain.c​:123)
==12501== Address 0x5f854b0 is 32 bytes before a block of size 48 in arena
"client"
==12501==
==12501== Invalid read of size 8
==12501== at 0x4EE844​: S_mro_gather_and_rename (mro_core.c​:932)
==12501== by 0x4EF08C​: S_mro_gather_and_rename (mro_core.c​:1186)
==12501== by 0x4F0B7D​: Perl_mro_package_moved (mro_core.c​:851)
==12501== by 0x52EDB1​: S_glob_assign_glob (sv.c​:3981)
==12501== by 0x521F77​: Perl_sv_setsv_flags (sv.c​:4462)
==12501== by 0x5001B5​: Perl_pp_sassign (pp_hot.c​:226)
==12501== by 0x4D7131​: Perl_runops_debug (dump.c​:2246)
==12501== by 0x453146​: S_run_body (perl.c​:2526)
==12501== by 0x453146​: perl_run (perl.c​:2449)
==12501== by 0x421944​: main (perlmain.c​:123)
==12501== Address 0x98 is not stack'd, malloc'd or (recently) free'd
==12501==
==12501==
==12501== Process terminating with default action of signal 11 (SIGSEGV)
==12501== Access not within mapped region at address 0x98
==12501== at 0x4EE844​: S_mro_gather_and_rename (mro_core.c​:932)
==12501== by 0x4EF08C​: S_mro_gather_and_rename (mro_core.c​:1186)
==12501== by 0x4F0B7D​: Perl_mro_package_moved (mro_core.c​:851)
==12501== by 0x52EDB1​: S_glob_assign_glob (sv.c​:3981)
==12501== by 0x521F77​: Perl_sv_setsv_flags (sv.c​:4462)
==12501== by 0x5001B5​: Perl_pp_sassign (pp_hot.c​:226)
==12501== by 0x4D7131​: Perl_runops_debug (dump.c​:2246)
==12501== by 0x453146​: S_run_body (perl.c​:2526)
==12501== by 0x453146​: perl_run (perl.c​:2449)
==12501== by 0x421944​: main (perlmain.c​:123)
==12501== If you believe this happened as a result of a stack
==12501== overflow in your program's main thread (unlikely but
==12501== possible), you can try to increase the size of the
==12501== main thread stack using the --main-stacksize= flag.
==12501== The main thread stack size used in this run was 8388608.
Segmentation fault

I ran afl-tmin and it came up with ./perl -e
'%​::​::​::=*​::​::​::=​::;%0=*0=*​::​::=​::" which doesn't crash but results in
perl​: sv.c​:4569​: void Perl_sv_setsv_flags(SV *, SV *, const I32)​: Assertion
`((svtype)((_gvgp)->sv_flags & 0xff)) == SVt_PVGV ||
((svtype)((_gvgp)->sv_flags & 0xff)) == SVt_PVLV' failed.
Aborted

@p5pRT
Copy link
Author

p5pRT commented Oct 12, 2016

From @geeknik

test84.gz

@p5pRT
Copy link
Author

p5pRT commented Oct 13, 2016

From @tonycoz

On Wed Oct 12 12​:37​:14 2016, brian.carpenter@​gmail.com wrote​:

I ran afl-tmin and it came up with ./perl -e
'%​::​::​::=*​::​::​::=​::;%0=*0=*​::​::=​::" which doesn't crash but results in
perl​: sv.c​:4569​: void Perl_sv_setsv_flags(SV *, SV *, const I32)​: Assertion
`((svtype)((_gvgp)->sv_flags & 0xff)) == SVt_PVGV ||
((svtype)((_gvgp)->sv_flags & 0xff)) == SVt_PVLV' failed.
Aborted

This seems to be the simplified reproducer​:

./perl -e '*​::​::​::=*​::xx; %​::​::​::=*​::; *​::​::=*​::;'

At the point of failure SVf_OOK isn't set on oldstash.

Tony

@p5pRT
Copy link
Author

p5pRT commented Oct 13, 2016

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

@p5pRT
Copy link
Author

p5pRT commented Feb 8, 2017

From @iabyn

On Wed, Oct 12, 2016 at 09​:14​:18PM -0700, Tony Cook via RT wrote​:

On Wed Oct 12 12​:37​:14 2016, brian.carpenter@​gmail.com wrote​:

I ran afl-tmin and it came up with ./perl -e
'%​::​::​::=*​::​::​::=​::;%0=*0=*​::​::=​::" which doesn't crash but results in
perl​: sv.c​:4569​: void Perl_sv_setsv_flags(SV *, SV *, const I32)​: Assertion
`((svtype)((_gvgp)->sv_flags & 0xff)) == SVt_PVGV ||
((svtype)((_gvgp)->sv_flags & 0xff)) == SVt_PVLV' failed.
Aborted

This seems to be the simplified reproducer​:

./perl -e '*​::​::​::=*​::xx; %​::​::​::=*​::; *​::​::=*​::;'

At the point of failure SVf_OOK isn't set on oldstash.

It can be simplified (or at least deobfuscated) further to this​:
(amongst other things renaming all the zero-length package names between
the pairs of colons to 'p', 'q' etc)​:

  %x = qw(foo bar);
  $q​::p​::{'p​::'} = *x;
  *q​::p​:: = *q​::;

(Or it may be a separate but related case - the reduction process took a
lot of twisty paths).

The two basic issues here are that

a) line 2 in the code above sticks an ordinary glob (*x) containing an
ordinary hash (%x) into the 'p​::' slot of the q​::p​:: symbol table.

b) during the glob assignment in line 3, the recursive 'fix up stashes and
stuff' done by Perl_mro_package_moved() expects that any entries in
stashes that have a name ending in '​::' point to another stash.

Its not clear to me whether the fix is to

1) ensure that anything added to a stash which could be treated as a stash
should be upgraded to a stash whenever it's inserted;

2) that Perl_mro_package_moved()/S_mro_gather_and_rename() should be fixed
to cope with non-stashes masquerading as stashes (by simply skipping
them).

I suspect that (1) is impossible, so (2) would be the way to go, but I'd
be interested to hear any further opinions.

--
Hofstadter's Law​: It always takes longer than you expect, even when you
take into account Hofstadter's Law.

@p5pRT
Copy link
Author

p5pRT commented Feb 10, 2017

From @iabyn

On Wed, Feb 08, 2017 at 04​:48​:17PM +0000, Dave Mitchell wrote​:

It can be simplified (or at least deobfuscated) further to this​:
(amongst other things renaming all the zero-length package names between
the pairs of colons to 'p', 'q' etc)​:

%x = qw\(foo bar\);
$q​::p​::\{'p​::'\} = \*x;
\*q​::p​::  = \*q​::;

(Or it may be a separate but related case - the reduction process took a
lot of twisty paths).

The two basic issues here are that

a) line 2 in the code above sticks an ordinary glob (*x) containing an
ordinary hash (%x) into the 'p​::' slot of the q​::p​:: symbol table.

b) during the glob assignment in line 3, the recursive 'fix up stashes and
stuff' done by Perl_mro_package_moved() expects that any entries in
stashes that have a name ending in '​::' point to another stash.

Its not clear to me whether the fix is to

1) ensure that anything added to a stash which could be treated as a stash
should be upgraded to a stash whenever it's inserted;

2) that Perl_mro_package_moved()/S_mro_gather_and_rename() should be fixed
to cope with non-stashes masquerading as stashes (by simply skipping
them).

I suspect that (1) is impossible, so (2) would be the way to go, but I'd
be interested to hear any further opinions.

I also don't think its a security issue. If an attacker can already force
perl to execute weird code that allows attacker-specified stash and
typeglob aliasing, then you've got bigger problems already (just think of
any sub or package var suddenly moving to another package of your choice).

The bug is unlikely to be found in existing code, as that would have
crashed the first time it was run.

--
"I do not resent criticism, even when, for the sake of emphasis,
it parts for the time with reality".
  -- Winston Churchill, House of Commons, 22nd Jan 1941.

@p5pRT
Copy link
Author

p5pRT commented Feb 21, 2017

From @iabyn

On Fri, Feb 10, 2017 at 09​:38​:28AM +0000, Dave Mitchell wrote​:

On Wed, Feb 08, 2017 at 04​:48​:17PM +0000, Dave Mitchell wrote​:

It can be simplified (or at least deobfuscated) further to this​:
(amongst other things renaming all the zero-length package names between
the pairs of colons to 'p', 'q' etc)​:

%x = qw\(foo bar\);
$q​::p​::\{'p​::'\} = \*x;
\*q​::p​::  = \*q​::;

(Or it may be a separate but related case - the reduction process took a
lot of twisty paths).

The two basic issues here are that

a) line 2 in the code above sticks an ordinary glob (*x) containing an
ordinary hash (%x) into the 'p​::' slot of the q​::p​:: symbol table.

b) during the glob assignment in line 3, the recursive 'fix up stashes and
stuff' done by Perl_mro_package_moved() expects that any entries in
stashes that have a name ending in '​::' point to another stash.

Its not clear to me whether the fix is to

1) ensure that anything added to a stash which could be treated as a stash
should be upgraded to a stash whenever it's inserted;

2) that Perl_mro_package_moved()/S_mro_gather_and_rename() should be fixed
to cope with non-stashes masquerading as stashes (by simply skipping
them).

I suspect that (1) is impossible, so (2) would be the way to go, but I'd
be interested to hear any further opinions.

I also don't think its a security issue. If an attacker can already force
perl to execute weird code that allows attacker-specified stash and
typeglob aliasing, then you've got bigger problems already (just think of
any sub or package var suddenly moving to another package of your choice).

The bug is unlikely to be found in existing code, as that would have
crashed the first time it was run.

I've move this ticket to the public queue in a few days time if no-one
objects.

--
"There's something wrong with our bloody ships today, Chatfield."
  -- Admiral Beatty at the Battle of Jutland, 31st May 1916.

@p5pRT
Copy link
Author

p5pRT commented Feb 27, 2017

From @iabyn

On Tue, Feb 21, 2017 at 11​:02​:59AM +0000, Dave Mitchell wrote​:

On Fri, Feb 10, 2017 at 09​:38​:28AM +0000, Dave Mitchell wrote​:

On Wed, Feb 08, 2017 at 04​:48​:17PM +0000, Dave Mitchell wrote​:
I also don't think its a security issue. If an attacker can already force
perl to execute weird code that allows attacker-specified stash and
typeglob aliasing, then you've got bigger problems already (just think of
any sub or package var suddenly moving to another package of your choice).

The bug is unlikely to be found in existing code, as that would have
crashed the first time it was run.

I've move this ticket to the public queue in a few days time if no-one
objects.

which I am now doing.

--
The Enterprise is captured by a vastly superior alien intelligence which
does not put them on trial.
  -- Things That Never Happen in "Star Trek" #10

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@p5pRT