Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.94k stars 554 forks source link

Null pointer in S_set_haseval #16016

Open p5pRT opened 7 years ago

p5pRT commented 7 years ago

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

Searchable as RT131568$

p5pRT commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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

p5pRT commented 7 years ago

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 commented 7 years ago

From @Mipu94

poc

p5pRT commented 7 years ago

From @tux

On Wed\, 14 Jun 2017 18​:00​:38 +0700\, Ta Sung \tadinhsung@&#8203;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 commented 7 years ago

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 commented 7 years ago

From @demerphq

On 24 Jun 2017 16​:01\, "Dave Mitchell" \davem@&#8203;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 commented 7 years ago

From @iabyn

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

On 24 Jun 2017 16​:01\, "Dave Mitchell" \davem@&#8203;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 commented 7 years ago

From @demerphq

Hi On 26 Jun 2017 10​:21\, "Dave Mitchell" \davem@&#8203;iabyn\.com wrote​:

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

On 24 Jun 2017 16​:01\, "Dave Mitchell" \davem@&#8203;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 commented 7 years ago

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 commented 7 years ago

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 commented 2 years ago

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

tonycoz commented 2 years ago

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

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