Perl / perl5

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

NULL pointer dereference in S_pending_ident() #17397

Open fcambus opened 4 years ago

fcambus commented 4 years ago

Hi,

While fuzzing Perl 5.30.1 with Honggfuzz, I found a NULL pointer dereference in the S_pending_ident() function, in toke.c.

Attaching a reproducer (gzipped so GitHub accepts it): test01.pl.gz

Issue can be reproduced by running:

perl test01.pl
AddressSanitizer:DEADLYSIGNAL
=================================================================
==13609==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000008 (pc 0x0000005857d1 bp 0x7ffd300e8150 sp 0x7ffd300e6f80 T0)
==13609==The signal is caused by a READ memory access.
==13609==Hint: address points to the zero page.
    #0 0x5857d0 in S_pending_ident /home/fcambus/perl-5.30.1/toke.c:9111:17
    #1 0x5857d0 in Perl_yylex /home/fcambus/perl-5.30.1/toke.c:4903:13
    #2 0x5d21cc in Perl_yyparse /home/fcambus/perl-5.30.1/perly.c:340:34
    #3 0x54cfa0 in S_parse_body /home/fcambus/perl-5.30.1/perl.c:2531:9
    #4 0x54cfa0 in perl_parse /home/fcambus/perl-5.30.1/perl.c:1822:2
    #5 0x4df38c in main /home/fcambus/perl-5.30.1/perlmain.c:126:10
    #6 0x7f1b520c11e2 in __libc_start_main /build/glibc-4WA41p/glibc-2.30/csu/../csu/libc-start.c:308:16
    #7 0x437bfd in _start (/home/fcambus/perl-5.30.1/perl+0x437bfd)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /home/fcambus/perl-5.30.1/toke.c:9111:17 in S_pending_ident
==13609==ABORTING
jkeenan commented 4 years ago

What happens if you rewrite the test program to include:

use strict;
use warnings;

... and then re-fuzz it?

Thank you very much. Jim Keenan

jkeenan commented 4 years ago

FWIW, from at least perl-5.6.2 to perl-5.8.9, the test program fails to compile.

$ perlbrew use perl-5.6.2
$ perl ghi-17397-test01.pl 
syntax error at ghi-17397-test01.pl line 197, near "{]"
syntax error at ghi-17397-test01.pl line 197, near "{]"
syntax error at ghi-17397-test01.pl line 265, near "print"
syntax error at ghi-17397-test01.pl line 271, near "E1 "
syntax error at ghi-17397-test01.pl line 346, near "{ ^"
syntax error at ghi-17397-test01.pl line 352, near "{^"
syntax error at ghi-17397-test01.pl line 355, near "{ ^"
Execution of ghi-17397-test01.pl aborted due to compilation errors.

As of (at least) perl-5.10.1, it fails to compile and segfaults at the point where the program dies.

$ perlbrew use perl-5.10.1
$ perl ghi-17397-test01.pl 
Semicolon seems to be missing at ghi-17397-test01.pl line 270.
String found where operator expected at ghi-17397-test01.pl line 363, near "}" ne ""
    (Missing operator before " ne "?)
Bareword found where operator expected at ghi-17397-test01.pl line 363, near "" ne "bar"
    (Missing operator before bar?)
String found where operator expected at ghi-17397-test01.pl line 364, near "print ""
  (Might be a runaway multi-line "" string starting on line 363)
    (Missing semicolon on previous line?)
Bareword found where operator expected at ghi-17397-test01.pl line 364, near "print "ok"
    (Do you need to predeclare print?)
Backslash found where operator expected at ghi-17397-test01.pl line 364, near "$test\"
    (Missing operator before \?)
String found where operator expected at ghi-17397-test01.pl line 366, near "print ""
  (Might be a runaway multi-line "" string starting on line 364)
    (Missing semicolon on previous line?)
Scalar found where operator expected at ghi-17397-test01.pl line 366, near "" if "${^TEST}"
    (Missing operator before ${^TEST}?)
Bareword found where operator expected at ghi-17397-test01.pl line 366, near "" ne "splat"
    (Missing operator before splat?)
Bareword found where operator expected at ghi-17397-test01.pl line 367, near "print "ok"
  (Might be a runaway multi-line "" string starting on line 366)
    (Do you need to predeclare print?)
Backslash found where operator expected at ghi-17397-test01.pl line 367, near "$test\"
    (Missing operator before \?)
String found where operator expected at ghi-17397-test01.pl line 369, near "print ""
  (Might be a runaway multi-line "" string starting on line 367)
    (Missing semicolon on previous line?)
Scalar found where operator expected at ghi-17397-test01.pl line 369, near "" if "$"
    (Missing operator before $?)
Segmentation fault (core dumped)

As yet I see no evidence that the program ever ran to completion.

Thank you very much. Jim Keenan

jkeenan commented 4 years ago

The test file appears to be a fork from an older version of t/base/lex.t. If, in the test file, you make this one correction:

$ diff -w test01.pl ghi-17397-test04.pl
363c363
<   print "not " if${ ^TEST [1] }" ne "bar";
---
>   print "not " if "${ ^TEST [1] }" ne "bar";

... the segfault goes away (although the file still does not compile).

Thank you very much. Jim Keenan

hvds commented 4 years ago

It's probably not profitable to critique perl code generated by fuzzing. If we can reproduce it, the next step is to attempt to reduce it to a minimal test case and concentrate on why it triggers a coredump.

miniperl@blead happily reproduces it; it reduces at least to:

<<E1;
${sub{b{]]]{} @{[ <<E2 ]}
E2
E1

I hope someone with fresher knowledge of the lexer will look at this, so I don't have to.

Hugo

khwilliamson commented 2 years ago

In 5.35.10, if I run either the fuzzed program or the reduction above, the dereference is gone. In the reduction, we get

syntax error at 17397.pl line 2, near "{]" syntax error at 17397.pl line 1, near "<<E1" panic: pad_swipe po=3, fill=1 at 17397.pl line 1. panic: pad_swipe po=3, fill=1 at 17397.pl line 1.

In the unreduced version, we get lots of syntax errors.

But I believe this is now closable

hvds commented 2 years ago

Let me see if I can bisect to find what fixed it: it would be useful to know at least if it was intentional.

hvds commented 2 years ago

In 5.35.10, if I run either the fuzzed program or the reduction above, the dereference is gone. In the reduction, we get

syntax error at 17397.pl line 2, near "{]" syntax error at 17397.pl line 1, near "<<E1" panic: pad_swipe po=3, fill=1 at 17397.pl line 1. panic: pad_swipe po=3, fill=1 at 17397.pl line 1.

In the unreduced version, we get lots of syntax errors.

But I believe this is now closable

Huh, with blead@f5469426bb I get:

% ./Configure -des -Dcc=gcc -Dprefix=/opt/scratch -Doptimize='-g -O6' -Dusedevel -Uversiononly \
  && make miniperl
[...]
% ./miniperl
<<E1;
${sub{b{]]]{} @{[ <<E2 ]}
E2
E1
Segmentation fault (core dumped)
% 

So I don't think this is closable. @khwilliamson how did you build and test?

I'm pretty sure this is yet another case of foolishly attempting to continue after errors, which @iabyn may one day save us from.

demerphq commented 2 years ago

On Sun, 17 Apr 2022, 10:36 Hugo van der Sanden, @.***> wrote:

In 5.35.10, if I run either the fuzzed program or the reduction above, the dereference is gone. In the reduction, we get

syntax error at 17397.pl line 2, near "{]" syntax error at 17397.pl line 1, near "<<E1" panic: pad_swipe po=3, fill=1 at 17397.pl line 1. panic: pad_swipe po=3, fill=1 at 17397.pl line 1.

In the unreduced version, we get lots of syntax errors.

But I believe this is now closable

Huh, with @.*** I get:

% ./Configure -des -Dcc=gcc -Dprefix=/opt/scratch -Doptimize='-g -O6' -Dusedevel -Uversiononly \ && make miniperl [...] % ./miniperl <<E1; ${sub{b{]]]{} @{[ <<E2 ]} E2 E1 Segmentation fault (core dumped) %

So I don't think this is closable. @khwilliamson https://github.com/khwilliamson how did you build and test?

I'm pretty sure this is yet another case of foolishly attempting to continue after errors, which @iabyn https://github.com/iabyn may one day save us from.

That will be a good day indeed.

Yves

khwilliamson commented 2 years ago

It turns out that the crucial distinction between my Configure call and yours was -DDEBUGGING in mine

I fell prey to confirmation bias. I had made some changes in toke.c some releases ago that fixed a number of segfaults from malformed input, and I thought that that could very well have fixed this.

Bisecting should be done in order to close tickets like this. Is there some place to say that in the docs?

jkeenan commented 2 years ago

[snip]

Bisecting should be done in order to close tickets like this. Is there some place to say that in the docs?

I don't think that's necessary. Quite a few committers have gotten fluent in the use of Porting/bisect.pl and we have a good sense of the kind of bug tickets for which it is useful. The trick is to figure out the arguments for that program. As we've encountered new cases, we've added them to the EXAMPLES section of the documentation in Porting/bisect-runner.pl.

jkeenan commented 2 years ago

Closed by mistake; re-opened.

demerphq commented 2 years ago

This is fixed by eeaa14560ec85b0b27e1525f54911f13d61ec55f (unmerged as yet).

demerphq commented 2 years ago

Currently in https://github.com/Perl/perl5/pull/20168