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

negative-size-param (size=-7) in S_scan_formline (toke.c:11414) #16169

Closed
p5pRT opened this issue Sep 25, 2017 · 14 comments
Closed

negative-size-param (size=-7) in S_scan_formline (toke.c:11414) #16169

p5pRT opened this issue Sep 25, 2017 · 14 comments

Comments

@p5pRT
Copy link

p5pRT commented Sep 25, 2017

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

Searchable as RT132158$

@p5pRT
Copy link
Author

p5pRT commented Sep 25, 2017

From @geeknik

Triggered while fuzzing v5.27.4-28-g60dfa51.

./perl -e "format=
@​
=h
=cut"

=================================================================
==22400==ERROR​: AddressSanitizer​: negative-size-param​: (size=-7)
  #0 0x451452 in __interceptor_memchr (/root/perl/perl+0x451452)
  #1 0xa16a2c in S_scan_formline /root/perl/toke.c​:11414​:17
  #2 0x8d06d8 in Perl_yylex /root/perl/toke.c​:5075​:6
  #3 0xae04ea in Perl_yyparse /root/perl/perly.c​:340​:34
  #4 0x722907 in S_parse_body /root/perl/perl.c​:2450​:9
  #5 0x70e273 in perl_parse /root/perl/perl.c​:1753​:2
  #6 0x504ea9 in main /root/perl/perlmain.c​:121​:18
  #7 0x7fbf7f07182f in __libc_start_main
/build/glibc-bfm8X4/glibc-2.23/csu/../csu/libc-start.c​:291
  #8 0x435f28 in _start (/root/perl/perl+0x435f28)

0x603000000678 is located 8 bytes inside of 24-byte region
[0x603000000670,0x603000000688)
allocated by thread T0 here​:
  #0 0x4d9a32 in realloc (/root/perl/perl+0x4d9a32)
  #1 0xf05194 in Perl_safesysrealloc /root/perl/util.c​:274​:18
  #2 0x12802d3 in Perl_sv_grow /root/perl/sv.c​:1600​:17
  #3 0x1443a45 in Perl_sv_gets /root/perl/sv.c​:8815​:2
  #4 0x8a6f20 in S_filter_gets /root/perl/toke.c​:4594​:17
  #5 0x8a6f20 in Perl_lex_next_chunk /root/perl/toke.c​:1352
  #6 0x8eeb48 in Perl_yylex /root/perl/toke.c​:5337​:11
  #7 0xae04ea in Perl_yyparse /root/perl/perly.c​:340​:34
  #8 0x722907 in S_parse_body /root/perl/perl.c​:2450​:9
  #9 0x70e273 in perl_parse /root/perl/perl.c​:1753​:2
  #10 0x504ea9 in main /root/perl/perlmain.c​:121​:18
  #11 0x7fbf7f07182f in __libc_start_main
/build/glibc-bfm8X4/glibc-2.23/csu/../csu/libc-start.c​:291

SUMMARY​: AddressSanitizer​: negative-size-param
(/root/perl/perl+0x451452) in __interceptor_memchr

@p5pRT
Copy link
Author

p5pRT commented Sep 26, 2017

From @tonycoz

On Mon, 25 Sep 2017 10​:27​:41 -0700, brian.carpenter@​gmail.com wrote​:

Triggered while fuzzing v5.27.4-28-g60dfa51.

./perl -e "format=
@​
=h
=cut"

=================================================================
==22400==ERROR​: AddressSanitizer​: negative-size-param​: (size=-7)
#0 0x451452 in __interceptor_memchr (/root/perl/perl+0x451452)
#1 0xa16a2c in S_scan_formline /root/perl/toke.c​:11414​:17
#2 0x8d06d8 in Perl_yylex /root/perl/toke.c​:5075​:6
#3 0xae04ea in Perl_yyparse /root/perl/perly.c​:340​:34
#4 0x722907 in S_parse_body /root/perl/perl.c​:2450​:9
#5 0x70e273 in perl_parse /root/perl/perl.c​:1753​:2
#6 0x504ea9 in main /root/perl/perlmain.c​:121​:18
#7 0x7fbf7f07182f in __libc_start_main
/build/glibc-bfm8X4/glibc-2.23/csu/../csu/libc-start.c​:291
#8 0x435f28 in _start (/root/perl/perl+0x435f28)

0x603000000678 is located 8 bytes inside of 24-byte region
[0x603000000670,0x603000000688)
allocated by thread T0 here​:
#0 0x4d9a32 in realloc (/root/perl/perl+0x4d9a32)
#1 0xf05194 in Perl_safesysrealloc /root/perl/util.c​:274​:18
#2 0x12802d3 in Perl_sv_grow /root/perl/sv.c​:1600​:17
#3 0x1443a45 in Perl_sv_gets /root/perl/sv.c​:8815​:2
#4 0x8a6f20 in S_filter_gets /root/perl/toke.c​:4594​:17
#5 0x8a6f20 in Perl_lex_next_chunk /root/perl/toke.c​:1352
#6 0x8eeb48 in Perl_yylex /root/perl/toke.c​:5337​:11
#7 0xae04ea in Perl_yyparse /root/perl/perly.c​:340​:34
#8 0x722907 in S_parse_body /root/perl/perl.c​:2450​:9
#9 0x70e273 in perl_parse /root/perl/perl.c​:1753​:2
#10 0x504ea9 in main /root/perl/perlmain.c​:121​:18
#11 0x7fbf7f07182f in __libc_start_main
/build/glibc-bfm8X4/glibc-2.23/csu/../csu/libc-start.c​:291

SUMMARY​: AddressSanitizer​: negative-size-param
(/root/perl/perl+0x451452) in __interceptor_memchr

This is a parser bug - it requires feeding code to the parser and hence isn't a security issue.

In this case scan_formline() returns a valid s (== PL_bufend) then jumps to rightbracket, which then increments s beyond PL_bufend.

This is simply fixed by adding a conditional to the increment, but then other things go wrong, eventually crashing in the parser.

In any case I'll move this to the public queue in a few days unless someone objects.

Tony

@p5pRT
Copy link
Author

p5pRT commented Sep 26, 2017

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

@p5pRT
Copy link
Author

p5pRT commented Oct 12, 2017

From @tonycoz

On Mon, 25 Sep 2017 17​:29​:52 -0700, tonyc wrote​:

In any case I'll move this to the public queue in a few days unless
someone objects.

Done.

Tony

@p5pRT
Copy link
Author

p5pRT commented Nov 5, 2017

From zefram@fysh.org

Tony Cook via RT wrote​:

This is simply fixed by adding a conditional to the increment, but then
other things go wrong, eventually crashing in the parser.

Yes, the parser state gets very messed up indeed. By the time s is
incremented past PL_bufend, the parser is already in a broken state.
In commit b1f87de I've added some
assertions which this test case fails, but which don't fail on anything
in the core test suite. The earliest of these assertions to be hit is
that PL_lex_formbrack is non-zero when calling scan_formline(), and I've
found that it's becoming zero due to a scope being popped during parser
error recovery.

We've seen problems before with the scope stack getting out of synch with
the parser state during error recovery, and I don't have a solution to it.
This ticket should remain open, because the test case still fails.

-zefram

@p5pRT
Copy link
Author

p5pRT commented Nov 29, 2017

From @iabyn

On Mon, Sep 25, 2017 at 10​:27​:41AM -0700, Brian Carpenter wrote​:

Triggered while fuzzing v5.27.4-28-g60dfa51.

./perl -e "format=
@​
=h
=cut"

=================================================================
==22400==ERROR​: AddressSanitizer​: negative-size-param​: (size=-7)
#0 0x451452 in __interceptor_memchr (/root/perl/perl+0x451452)
#1 0xa16a2c in S_scan_formline /root/perl/toke.c​:11414​:17
#2 0x8d06d8 in Perl_yylex /root/perl/toke.c​:5075​:6

In 5.27.6 and later its failing differently, with an assertion failure​:

perl5276​: toke.c​:5095​: Perl_yylex​: Assertion `(PL_parser->lex_formbrack)' failed.

but on non-debugging builds, it still gives valgrind errors.

Since it appears to require crafted code to be fed to the compiler, I
suspect that it isn't a security issue. But it needs further
investigation.

--
In my day, we used to edit the inodes by hand. With magnets.

@p5pRT
Copy link
Author

p5pRT commented Nov 29, 2017

From @iabyn

On Wed, Nov 29, 2017 at 11​:36​:47AM +0000, Dave Mitchell wrote​:

On Mon, Sep 25, 2017 at 10​:27​:41AM -0700, Brian Carpenter wrote​:

Triggered while fuzzing v5.27.4-28-g60dfa51.

./perl -e "format=

D'oh - another ticket which has already been moved to the public queue.
I could have sworn I checked first :-(

--
"Foul and greedy Dwarf - you have eaten the last candle."
  -- "Hordes of the Things", BBC Radio.

@p5pRT
Copy link
Author

p5pRT commented Aug 28, 2018

From @tonycoz

On Mon, 25 Sep 2017 10​:27​:41 -0700, brian.carpenter@​gmail.com wrote​:

Triggered while fuzzing v5.27.4-28-g60dfa51.

./perl -e "format=
@​
=h
=cut"

The attached works around the crash for me.

Tony

@p5pRT
Copy link
Author

p5pRT commented Aug 28, 2018

From @tonycoz

0001-perl-132158-abort-compilation-if-we-see-an-error-com.patch
From e5ebbe8d422a5adb60a9ee7f23d6a90f611bd51e Mon Sep 17 00:00:00 2001
From: Tony Cook <tony@develop-help.com>
Date: Tue, 28 Aug 2018 14:11:10 +1000
Subject: (perl #132158) abort compilation if we see an error compiling a form

---
 t/lib/croak/toke | 9 +++++++++
 toke.c           | 9 +++++++++
 2 files changed, 18 insertions(+)

diff --git a/t/lib/croak/toke b/t/lib/croak/toke
index 1d45a3fdf5..a3852900e5 100644
--- a/t/lib/croak/toke
+++ b/t/lib/croak/toke
@@ -480,3 +480,12 @@ Bareword found where operator expected at - line 2, near "2p0"
 	(Missing operator before p0?)
 syntax error at - line 2, near "2p0"
 Execution of - aborted due to compilation errors.
+########
+# NAME [perl #132158] format with syntax errors
+format=
+@
+=h
+=cut
+EXPECT
+syntax error at - line 4, next token ???
+Execution of - aborted due to compilation errors.
diff --git a/toke.c b/toke.c
index 24e614fd50..08c2ffc2de 100644
--- a/toke.c
+++ b/toke.c
@@ -5099,6 +5099,14 @@ Perl_yylex(pTHX)
 
 	return yylex();
     case LEX_FORMLINE:
+        if (PL_parser->sub_error_count != PL_error_count) {
+            /* There was an error parsing a formline, which tends to
+               mess up the parser.
+               Unlike interpolated sub-parsing, we can't treat any of
+               these as recoverable, so no need to check sub_no_recover.
+            */
+            yyquit();
+        }
 	assert(PL_lex_formbrack);
 	s = scan_formline(PL_bufptr);
 	if (!PL_lex_formbrack)
@@ -6518,6 +6526,7 @@ Perl_yylex(pTHX)
 		SAVEI32(PL_lex_formbrack);
 		PL_parser->form_lex_state = PL_lex_state;
 		PL_lex_formbrack = PL_lex_brackets + 1;
+                PL_parser->sub_error_count = PL_error_count;
 		goto leftbracket;
 	    }
 	}
-- 
2.11.0

@p5pRT
Copy link
Author

p5pRT commented Aug 28, 2018

From @tonycoz

0002-simplify-the-error-reporting-from-the-125351-fix.patch
From c3bad15042a392e0b33246a0f75aab368e63df73 Mon Sep 17 00:00:00 2001
From: Tony Cook <tony@develop-help.com>
Date: Tue, 28 Aug 2018 15:02:32 +1000
Subject: simplify the error reporting from the #125351 fix

---
 toke.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/toke.c b/toke.c
index 08c2ffc2de..e968442abf 100644
--- a/toke.c
+++ b/toke.c
@@ -2575,16 +2575,8 @@ S_sublex_done(pTHX)
 	const line_t l = CopLINE(PL_curcop);
 	LEAVE;
         if (PL_parser->sub_error_count != PL_error_count) {
-            const char * const name = OutCopFILE(PL_curcop);
             if (PL_parser->sub_no_recover) {
-                const char * msg = "";
-                if (PL_in_eval) {
-                    SV *errsv = ERRSV;
-                    if (SvCUR(ERRSV)) {
-                        msg = Perl_form(aTHX_ "%" SVf, SVfARG(errsv));
-                    }
-                }
-                abort_execution(msg, name);
+                yyquit();
                 NOT_REACHED;
             }
         }
-- 
2.11.0

@p5pRT
Copy link
Author

p5pRT commented Jan 3, 2019

From @tonycoz

On Mon, 27 Aug 2018 23​:07​:34 -0700, tonyc wrote​:

On Mon, 25 Sep 2017 10​:27​:41 -0700, brian.carpenter@​gmail.com wrote​:

Triggered while fuzzing v5.27.4-28-g60dfa51.

./perl -e "format=
@​
=h
=cut"

The attached works around the crash for me.

Tony

Applied as 8174801 and ad1ecdf.

Tony

@p5pRT
Copy link
Author

p5pRT commented Jan 3, 2019

@tonycoz - Status changed from 'open' to 'pending release'

@p5pRT
Copy link
Author

p5pRT commented May 22, 2019

From @khwilliamson

Thank you for filing this report. You have helped make Perl better.

With the release today of Perl 5.30.0, this and 160 other issues have been
resolved.

Perl 5.30.0 may be downloaded via​:
https://metacpan.org/release/XSAWYERX/perl-5.30.0

If you find that the problem persists, feel free to reopen this ticket.

@p5pRT
Copy link
Author

p5pRT commented May 22, 2019

@khwilliamson - Status changed from 'pending release' to 'resolved'

@p5pRT p5pRT closed this as completed May 22, 2019
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

1 participant