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 in S_set_haseval #16016

Open
p5pRT opened this issue Jun 13, 2017 · 14 comments
Open

Null pointer in S_set_haseval #16016

p5pRT opened this issue Jun 13, 2017 · 14 comments

Comments

@p5pRT
Copy link

p5pRT commented Jun 13, 2017

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

Searchable as RT131568$

@p5pRT
Copy link
Author

p5pRT commented Jun 13, 2017

From @Mipu94

File attached bellow was found by hongfuzz that triggered a crash in
S_set_haseval function. This bug affect on perl v5.27.1

some info about this bug on GDB and ASAN.

GDB-peda :

Program received signal SIGSEGV, Segmentation fault.
[----------------------------------registers-----------------------------------]
RAX​: 0x1
RBX​: 0x604000006950 --> 0x0
RCX​: 0x158
RDX​: 0x6040000041d0 --> 0x604000003ffa --> 0x2000000007824
RSI​: 0x1
RDI​: 0x0
RBP​: 0x1
RSP​: 0x7fffffffe0e8 --> 0x53b538 (<Perl_ck_eval+344>​: mov
ecx,DWORD PTR [rip+0x104d7da] # 0x1588d18 <PL_compiling+56>)
RIP​: 0x5325e8 (<S_set_haseval+136>​: mov rdx,QWORD PTR [rdi])
R8 : 0x0
R9 : 0x0
R10​: 0x604000004010 --> 0x3
R11​: 0x2
R12​: 0x616000000698 --> 0x2
R13​: 0x17
R14​: 0x604000006910 (0x0000604000006910)
R15​: 0x604000006910 (0x0000604000006910)
EFLAGS​: 0x10202 (carry parity adjust zero sign trap INTERRUPT
direction overflow)
[-------------------------------------code-------------------------------------]
  0x5325e0 <S_set_haseval+128>​: mov eax,DWORD PTR [rdx+0x18]
  0x5325e3 <S_set_haseval+131>​: test rax,rax
  0x5325e6 <S_set_haseval+134>​: je 0x532650 <S_set_haseval+240>
=> 0x5325e8 <S_set_haseval+136>​: mov rdx,QWORD PTR [rdi]
  0x5325eb <S_set_haseval+139>​: mov rdi,QWORD PTR [rdx+0x50]
  0x5325ef <S_set_haseval+143>​: test rdi,rdi
  0x5325f2 <S_set_haseval+146>​: je 0x532650 <S_set_haseval+240>
  0x5325f4 <S_set_haseval+148>​: mov rdx,QWORD PTR [rdi]
[------------------------------------stack-------------------------------------]
0000| 0x7fffffffe0e8 --> 0x53b538 (<Perl_ck_eval+344>​: mov
ecx,DWORD PTR [rip+0x104d7da] # 0x1588d18 <PL_compiling+56>)
0008| 0x7fffffffe0f0 --> 0x158
0016| 0x7fffffffe0f8 --> 0x158
0024| 0x7fffffffe100 --> 0x1
0032| 0x7fffffffe108 --> 0x52cdf3 (<Perl_newUNOP+211>​: mov rbx,rax)
0040| 0x7fffffffe110 --> 0xd6
0048| 0x7fffffffe118 --> 0xd8
0056| 0x7fffffffe120 --> 0x63 ('c')
[------------------------------------------------------------------------------]
blue
Legend​: code, data, rodata, value
Stopped reason​: SIGSEGV
0x00000000005325e8 in S_set_haseval ()

ASAN :

ASAN​:DEADLYSIGNAL

==46788==ERROR​: AddressSanitizer​: SEGV on unknown address
0x000000000000 (pc 0x0000005325e8 bp 0x000000000001 sp 0x7ffcb61afe38
T0)
==46788==The signal is caused by a READ memory access.
==46788==Hint​: address points to the zero page.
  #0 0x5325e7 (/home/mipu/fuzzperl/crash/perl-asan+0x5325e7)
  #1 0x53b537 (/home/mipu/fuzzperl/crash/perl-asan+0x53b537)
  #2 0x52cdf2 (/home/mipu/fuzzperl/crash/perl-asan+0x52cdf2)
  #3 0x57eb5c (/home/mipu/fuzzperl/crash/perl-asan+0x57eb5c)
  #4 0x547b49 (/home/mipu/fuzzperl/crash/perl-asan+0x547b49)
  #5 0x52724f (/home/mipu/fuzzperl/crash/perl-asan+0x52724f)
  #6 0x7f18707fa82f (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
  #7 0x435588 (/home/mipu/fuzzperl/crash/perl-asan+0x435588)

AddressSanitizer can not provide additional info.
SUMMARY​: AddressSanitizer​: SEGV (/home/mipu/fuzzperl/crash/perl-asan+0x5325e7)

--
Ta Dinh Sung,

@p5pRT
Copy link
Author

p5pRT commented Jun 14, 2017

From @iabyn

On Tue, Jun 13, 2017 at 08​:27​:12AM -0700, sung wrote​:

File attached bellow was found by hongfuzz that triggered a crash in
S_set_haseval function. This bug affect on perl v5.27.1

The file doesn't appear to have been attached, or has been lost by the
ticketing system.

--
In economics, the exam questions are the same every year.
They just change the answers.

@p5pRT
Copy link
Author

p5pRT commented Jun 14, 2017

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

@p5pRT
Copy link
Author

p5pRT commented Jun 14, 2017

From @Mipu94

May be i forgot attach file. I resend you that poc was attached bellow.

2017-06-14 15​:36 GMT+07​:00 Dave Mitchell via RT <
perl5-security-report@​perl.org>​:

On Tue, Jun 13, 2017 at 08​:27​:12AM -0700, sung wrote​:

File attached bellow was found by hongfuzz that triggered a crash in
S_set_haseval function. This bug affect on perl v5.27.1

The file doesn't appear to have been attached, or has been lost by the
ticketing system.

--
In economics, the exam questions are the same every year.
They just change the answers.

--
Ta Dinh Sung,

@p5pRT
Copy link
Author

p5pRT commented Jun 14, 2017

From @Mipu94

poc

@p5pRT
Copy link
Author

p5pRT commented Jun 14, 2017

From @Tux

On Wed, 14 Jun 2017 18​:00​:38 +0700, Ta Sung <tadinhsung@​gmail.com>
wrote​:

May be i forgot attach file. I resend you that poc was attached bellow.

This is perl 5, version 26, subversion 0 (v5.26.0) built for x86_64-linux-thread-multi-ld

Program received signal SIGSEGV, Segmentation fault.
0x0000000000420f2e in S_mark_padname_lvalue ()
(gdb) where
#0 0x0000000000420f2e in S_mark_padname_lvalue ()
#1 0x000000000042239b in S_set_haseval ()
#2 0x000000000042c694 in Perl_ck_eval ()
#3 0x000000000042ab18 in Perl_newUNOP ()
#4 0x00000000004736d4 in Perl_yyparse ()
#5 0x00000000004480e4 in perl_parse ()
#6 0x0000000000420d3b in main ()

This is perl 5, version 27, subversion 0 (v5.27.0) built for x86_64-linux
ELF 64-bit LSB executable, x86-64, version 1 (SYSV), dynamically linked, with debug_info, not stripped

Program received signal SIGSEGV, Segmentation fault.
0x000000000042589e in S_mark_padname_lvalue ()
(gdb) where
#0 0x000000000042589e in S_mark_padname_lvalue ()
#1 0x00000000004268c8 in S_set_haseval ()
#2 0x0000000000434392 in Perl_ck_eval ()
#3 0x000000000043225c in Perl_newUNOP ()
#4 0x0000000000497c92 in Perl_yyparse ()
#5 0x0000000000456398 in perl_parse ()
#6 0x00000000004251a0 in main ()
(gdb) quit

--
H.Merijn Brand http​://tux.nl Perl Monger http​://amsterdam.pm.org/
using perl5.00307 .. 5.27 porting perl5 on HP-UX, AIX, and openSUSE
http​://mirrors.develooper.com/hpux/ http​://www.test-smoke.org/
http​://qa.perl.org http​://www.goldmark.org/jeff/stupid-disclaimers/

@p5pRT
Copy link
Author

p5pRT commented Jun 24, 2017

From @iabyn

On Wed, Jun 14, 2017 at 01​:51​:15PM +0200, H.Merijn Brand wrote​:

This is perl 5, version 27, subversion 0 (v5.27.0) built for x86_64-linux
ELF 64-bit LSB executable, x86-64, version 1 (SYSV), dynamically linked, with debug_info, not stripped

Program received signal SIGSEGV, Segmentation fault.
0x000000000042589e in S_mark_padname_lvalue ()
(gdb) where
#0 0x000000000042589e in S_mark_padname_lvalue ()
#1 0x00000000004268c8 in S_set_haseval ()
#2 0x0000000000434392 in Perl_ck_eval ()
#3 0x000000000043225c in Perl_newUNOP ()
#4 0x0000000000497c92 in Perl_yyparse ()
#5 0x0000000000456398 in perl_parse ()
#6 0x00000000004251a0 in main ()

This is code containing lots of nested closures and formats, and which has
a syntax error. Perl's attempts to continue parsing after the syntax error
result in code being called which expects PL_compcv to be non-NULL, which
it isn't in this case.

I think this is another example of why we should instead just stop parsing
after an error rather than trying to harden all perl's parsing code to be
robust after errors.

In any case I don't think it counts as a security issue.

--
You live and learn (although usually you just live).

@p5pRT
Copy link
Author

p5pRT commented Jun 26, 2017

From @demerphq

On 24 Jun 2017 16​:01, "Dave Mitchell" <davem@​iabyn.com> wrote​:

On Wed, Jun 14, 2017 at 01​:51​:15PM +0200, H.Merijn Brand wrote​:

This is perl 5, version 27, subversion 0 (v5.27.0) built for x86_64-linux
ELF 64-bit LSB executable, x86-64, version 1 (SYSV), dynamically linked,
with debug_info, not stripped

Program received signal SIGSEGV, Segmentation fault.
0x000000000042589e in S_mark_padname_lvalue ()
(gdb) where
#0 0x000000000042589e in S_mark_padname_lvalue ()
#1 0x00000000004268c8 in S_set_haseval ()
#2 0x0000000000434392 in Perl_ck_eval ()
#3 0x000000000043225c in Perl_newUNOP ()
#4 0x0000000000497c92 in Perl_yyparse ()
#5 0x0000000000456398 in perl_parse ()
#6 0x00000000004251a0 in main ()

This is code containing lots of nested closures and formats, and which has
a syntax error. Perl's attempts to continue parsing after the syntax error
result in code being called which expects PL_compcv to be non-NULL, which
it isn't in this case.

I think this is another example of why we should instead just stop parsing
after an error rather than trying to harden all perl's parsing code to be
robust after errors.

What are the trade offs involved in this?

Yves

@p5pRT
Copy link
Author

p5pRT commented Jun 26, 2017

From @iabyn

On Mon, Jun 26, 2017 at 09​:39​:54AM +0200, demerphq wrote​:

On 24 Jun 2017 16​:01, "Dave Mitchell" <davem@​iabyn.com> wrote​:

I think this is another example of why we should instead just stop parsing
after an error rather than trying to harden all perl's parsing code to be
robust after errors.

What are the trade offs involved in this?

There was, IIRC, a p5p thread about this recently, but I can't find it
now.

It means only the first compilation error gets reported to the user,
rather than carrying on for a bit and spewing out some further error
messages, some of which may be spurious.

Whether this is good or bad depends on your viewpoint. The cases where
a compilation cycle takes so much time that you want to spot as many
errors as possible in one run, are, I think, very limited these days.

It's possible that the first error message is misleading, and could be
clarified by a second error message.

It's possible that the second and subsequent error messages are
misleading, and should be ignored.

Personally I hardly ever check beyond the first error message.

Its *very* hard to keep compilation going after an error without leaks,
crashes etc. It means the entirety of op.c and perl.y, plus probably a
good chunk of the runtime, needs hardening against 'this should never
happen' scenarios.

--
Modern art​:
  "That's easy, I could have done that!"
  "Ah, but you didn't!"

@p5pRT
Copy link
Author

p5pRT commented Jun 26, 2017

From @demerphq

Hi On 26 Jun 2017 10​:21, "Dave Mitchell" <davem@​iabyn.com> wrote​:

On Mon, Jun 26, 2017 at 09​:39​:54AM +0200, demerphq wrote​:

On 24 Jun 2017 16​:01, "Dave Mitchell" <davem@​iabyn.com> wrote​:

I think this is another example of why we should instead just stop
parsing
after an error rather than trying to harden all perl's parsing code to
be
robust after errors.

What are the trade offs involved in this?

There was, IIRC, a p5p thread about this recently, but I can't find it
now.

Thanks for the summary.

It means only the first compilation error gets reported to the user,
rather than carrying on for a bit and spewing out some further error
messages, some of which may be spurious.

Whether this is good or bad depends on your viewpoint. The cases where
a compilation cycle takes so much time that you want to spot as many
errors as possible in one run, are, I think, very limited these days.

It's possible that the first error message is misleading, and could be
clarified by a second error message.

It's possible that the second and subsequent error messages are
misleading, and should be ignored.

Personally I hardly ever check beyond the first error message.

Its *very* hard to keep compilation going after an error without leaks,
crashes etc. It means the entirety of op.c and perl.y, plus probably a
good chunk of the runtime, needs hardening against 'this should never
happen' scenarios.

So the cost is high and the benefit unclear and possibly even negative.

Is it easy to build a perl that does not continue to see how that would
work in practice?

Also would it be possible to treat strict-only errors differently? The only
case I can think of where lots of errors at once is useful is correcting
misspelled var names. For instance during refactoring or something it can
be useful to see you misspelled 3 vars on 25 different lines.

If we are talking about errors/panics that would apply even without strict
I don't see the problem failing on the first error. Especially if doing so
made it easier to produce better diagnostics.

Cheers,
Yves

@p5pRT
Copy link
Author

p5pRT commented Jun 26, 2017

From @jhi

Is it easy to build a perl that does not continue to see how that would
work in practice?

I have blissfully manaaged to forget most all of I ever learned of
Perl parser. (Years of therapy, with single malts.) But as a general
principle of erroring out, regardless of language, the first possible
sign of a problem is usually the best thing to show. In other words,
show the errors (or just *the error*) strictly in the order of
increasing source location, no going back (which can happen if one
recurses).

Also would it be possible to treat strict-only errors differently? The
only case I can think of where lots of errors at once is useful is
correcting misspelled var names. For instance during refactoring or
something it can be useful to see you misspelled 3 vars on 25 different
lines.

If we are talking about errors/panics that would apply even without
strict I don't see the problem failing on the first error. Especially if
doing so made it easier to produce better diagnostics.

@p5pRT
Copy link
Author

p5pRT commented Aug 23, 2017

From @tonycoz

On Sat, 24 Jun 2017 07​:01​:54 -0700, davem wrote​:

On Wed, Jun 14, 2017 at 01​:51​:15PM +0200, H.Merijn Brand wrote​:

This is perl 5, version 27, subversion 0 (v5.27.0) built for x86_64-
linux
ELF 64-bit LSB executable, x86-64, version 1 (SYSV), dynamically
linked, with debug_info, not stripped

Program received signal SIGSEGV, Segmentation fault.
0x000000000042589e in S_mark_padname_lvalue ()
(gdb) where
#0 0x000000000042589e in S_mark_padname_lvalue ()
#1 0x00000000004268c8 in S_set_haseval ()
#2 0x0000000000434392 in Perl_ck_eval ()
#3 0x000000000043225c in Perl_newUNOP ()
#4 0x0000000000497c92 in Perl_yyparse ()
#5 0x0000000000456398 in perl_parse ()
#6 0x00000000004251a0 in main ()

This is code containing lots of nested closures and formats, and which
has
a syntax error. Perl's attempts to continue parsing after the syntax
error
result in code being called which expects PL_compcv to be non-NULL,
which
it isn't in this case.

I think this is another example of why we should instead just stop
parsing
after an error rather than trying to harden all perl's parsing code to
be
robust after errors.

In any case I don't think it counts as a security issue.

Moved to the public queue.

Wasn't the the cause of some similar issues that failed sub-parses didn't restore the shift-reduce parser stack to the state before the sub-parse?

The parser would shift in new tokens, reduce based on tokens from the sub-parse and use inconsisten PL_parser state (and crash).

Tony

@demerphq
Copy link
Collaborator

demerphq commented Sep 8, 2022

This appears to be fixed in 5.34, possibly via @tonycoz and bb4e4c3 but i havent bisected yet.

@tonycoz
Copy link
Contributor

tonycoz commented Sep 12, 2022

It still fails under valgrind in 5.28 so I don't think bb4e4c3 fixed it - the valgrind backtrace is the same in 5.26.

It completes "successfully" (reporting the syntax error) in 5.30.

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

3 participants