Skip Menu |
Report information
Id: 131598
Status: resolved
Priority: 0/
Queue: perl5

Owner: Nobody
Requestors: jwilk [at] jwilk.net
Cc:
AdminCc:

Operating System: (no value)
PatchStatus: (no value)
Severity: (no value)
Type: (no value)
Perl Version: (no value)
Fixed In: (no value)



Subject: Buffer over-read in S_grok_bslash_N
To: perl5-security-report [...] perl.org
From: Jakub Wilk <jwilk [...] jwilk.net>
Date: Sun, 18 Jun 2017 13:07:30 +0200
Download (untitled) / with headers
text/plain 1.7k
Consider the following program: m'\N{U+.}'; __END__ When you run it under valgrind, you get: Invalid read of size 2 at 0x402E020: memcpy (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so) by 0x815243E: Perl_sv_vcatpvfn_flags (sv.c:13067) by 0x814D860: Perl_sv_vsetpvfn (sv.c:10953) by 0x810985B: Perl_vmess (util.c:1485) by 0x8109E4C: Perl_vcroak (util.c:1714) by 0x8109EE5: Perl_croak (util.c:1761) by 0x80E8FFF: S_grok_bslash_N (regcomp.c:12168) by 0x80EB5B8: S_regatom (regcomp.c:12880) by 0x80E6A9D: S_regpiece (regcomp.c:11669) by 0x80E6872: S_regbranch (regcomp.c:11594) by 0x80E58D4: S_reg (regcomp.c:11332) by 0x80D7CCD: Perl_re_op_compile (regcomp.c:7101) Address 0x4288d1e is 6 bytes after a block of size 120 free'd at 0x402A3A8: free (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so) by 0x810837B: Perl_safesysfree (util.c:388) by 0x816F501: Perl_leave_scope (scope.c:929) by 0x816DA76: Perl_pop_scope (scope.c:124) by 0x809BF63: S_sublex_done (toke.c:2569) by 0x80A1E24: Perl_yylex (toke.c:4918) by 0x80BE6A1: Perl_yyparse (perly.c:340) by 0x8086637: S_parse_body (perl.c:2377) by 0x808578B: perl_parse (perl.c:1692) by 0x8061751: main (perlmain.c:121) The above was tested on v5.26.0 built for i686-linux. The exact symptoms seem to vary with Perl version. For example, when I run it on Debian perl v5.24.1 on i386 (without valgrind this time), I get the following warning on stderr: Invalid hexadecimal number in \N{U+...} in regex; marked by <-- HERE in m/ <-- HERE <binary garbage> where binary garbage looks like a large chunk of the program memory, including values of environment variables. :( -- Jakub Wilk
RT-Send-CC: rt-deliver-to-perl5-security-report [...] rt.perl.org
Download (untitled) / with headers
text/plain 524b
On Sun, 18 Jun 2017 08:53:27 -0700, jwilk@jwilk.net wrote: Show quoted text
> Consider the following program: > > m'\N{U+.}'; > __END__ > > When you run it under valgrind, you get: > > Invalid read of size 2
[...] I tried a few builds of perl - both blead and 5.26.0 - to reproduce this, without success. Please could you supply the output of 'perl -V' for the build with which you found this? For future reports, it would be helpful if you could use 'perlbug', which would include that sort of information automatically. Cheers, Hugo
Subject: Re: [perl #131598] Buffer over-read in S_grok_bslash_N
To: perl5-security-report [...] perl.org
Date: Mon, 19 Jun 2017 19:23:10 -0600
From: Karl Williamson <public [...] khwilliamson.com>
On 06/18/2017 09:53 AM, Jakub Wilk (via RT) wrote: Show quoted text
> # New Ticket Created by Jakub Wilk > # Please include the string: [perl #131598] > # in the subject line of all future correspondence about this issue. > # <URL: https://rt.perl.org/Ticket/Display.html?id=131598 > > > > Consider the following program: > > m'\N{U+.}'; > __END__ > > When you run it under valgrind, you get: > > Invalid read of size 2 > at 0x402E020: memcpy (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so) > by 0x815243E: Perl_sv_vcatpvfn_flags (sv.c:13067) > by 0x814D860: Perl_sv_vsetpvfn (sv.c:10953) > by 0x810985B: Perl_vmess (util.c:1485) > by 0x8109E4C: Perl_vcroak (util.c:1714) > by 0x8109EE5: Perl_croak (util.c:1761) > by 0x80E8FFF: S_grok_bslash_N (regcomp.c:12168) > by 0x80EB5B8: S_regatom (regcomp.c:12880) > by 0x80E6A9D: S_regpiece (regcomp.c:11669) > by 0x80E6872: S_regbranch (regcomp.c:11594) > by 0x80E58D4: S_reg (regcomp.c:11332) > by 0x80D7CCD: Perl_re_op_compile (regcomp.c:7101) > Address 0x4288d1e is 6 bytes after a block of size 120 free'd > at 0x402A3A8: free (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so) > by 0x810837B: Perl_safesysfree (util.c:388) > by 0x816F501: Perl_leave_scope (scope.c:929) > by 0x816DA76: Perl_pop_scope (scope.c:124) > by 0x809BF63: S_sublex_done (toke.c:2569) > by 0x80A1E24: Perl_yylex (toke.c:4918) > by 0x80BE6A1: Perl_yyparse (perly.c:340) > by 0x8086637: S_parse_body (perl.c:2377) > by 0x808578B: perl_parse (perl.c:1692) > by 0x8061751: main (perlmain.c:121) > > The above was tested on v5.26.0 built for i686-linux. > > The exact symptoms seem to vary with Perl version. For example, when I run it > on Debian perl v5.24.1 on i386 (without valgrind this time), I get the > following warning on stderr: > > Invalid hexadecimal number in \N{U+...} in regex; marked by <-- HERE in m/ <-- HERE <binary garbage> > > where binary garbage looks like a large chunk of the program memory, including > values of environment variables. :( >
I single stepped through this in blead. On a DEBUGGING build, instead an assertion fails: ==15879== Process terminating with default action of signal 6 (SIGABRT) ==15879== at 0x5B5A77F: raise (raise.c:58) ==15879== by 0x5B5C379: abort (abort.c:89) ==15879== by 0x5B52B46: __assert_fail_base (assert.c:92) ==15879== by 0x5B52BF1: __assert_fail (assert.c:101) ==15879== by 0x37E8AE: Perl_sv_vcatpvfn_flags (sv.c:12485) ==15879== by 0x37A431: Perl_sv_vsetpvfn (sv.c:10961) ==15879== by 0x2B3BEC: Perl_vmess (util.c:1487) ==15879== by 0x2B50C9: Perl_vcroak (util.c:1716) ==15879== by 0x2B54CA: Perl_croak (util.c:1763) ==15879== by 0x26D817: S_grok_bslash_N (regcomp.c:12168) ==15879== by 0x271CA3: S_regatom (regcomp.c:12932) ==15879== by 0x2694D5: S_regpiece (regcomp.c:11668) Without DEBUGGING, and with -O2 optimization, I get Using Valgrind-3.12.0 and LibVEX; rerun with -h for copyright info ==16556== Command: ./perl -le m'\\N{U+.}'; ==16556== ==16556== Invalid read of size 2 ==16556== at 0x4C34618: memmove (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==16556== by 0x1EB097: memcpy (string3.h:53) ==16556== by 0x1EB097: Perl_sv_vcatpvfn_flags (sv.c:13338) ==16556== by 0x1EE430: Perl_sv_vsetpvfn (sv.c:10961) ==16556== by 0x1BB42D: Perl_vmess (util.c:1487) ==16556== by 0x1BABB2: Perl_vcroak (util.c:1716) ==16556== by 0x1BB693: Perl_croak (util.c:1763) ==16556== by 0x19B638: S_grok_bslash_N (regcomp.c:12168) ==16556== by 0x1A20B5: S_regatom (regcomp.c:12932) ==16556== by 0x1A4C8C: S_regpiece (regcomp.c:11668) ==16556== by 0x1A4C8C: S_regbranch (regcomp.c:11593) ==16556== by 0x198431: S_reg (regcomp.c:11331) ==16556== by 0x1AED3B: Perl_re_op_compile (regcomp.c:7100) ==16556== by 0x14DE8C: Perl_pmruntime (op.c:5882) ==16556== Address 0x5bb16a6 is 14 bytes after a block of size 120 free'd ==16556== at 0x4C2ED5B: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==16556== by 0x2194BB: Perl_leave_scope (scope.c:929) ==16556== by 0x185986: S_sublex_done (toke.c:2562) ==16556== by 0x174DC9: Perl_yylex (toke.c:4912) ==16556== by 0x1865FB: Perl_yyparse (perly.c:340) ==16556== by 0x15A815: S_parse_body (perl.c:2401) ==16556== by 0x15A815: perl_parse (perl.c:1719) ==16556== by 0x1331F2: main (perlmain.c:121) ==16556== Block was alloc'd at ==16556== at 0x4C2DB2F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==16556== by 0x1BA191: Perl_safesysmalloc (util.c:153) ==16556== by 0x1703F8: S_sublex_push (toke.c:2475) ==16556== by 0x1703F8: Perl_yylex (toke.c:4908) ==16556== by 0x1865FB: Perl_yyparse (perly.c:340) ==16556== by 0x15A815: S_parse_body (perl.c:2401) ==16556== by 0x15A815: perl_parse (perl.c:1719) ==16556== by 0x1331F2: main (perlmain.c:121) ==16556== ==16556== Invalid read of size 2 ==16556== at 0x4C34627: memmove (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==16556== by 0x1EB097: memcpy (string3.h:53) ==16556== by 0x1EB097: Perl_sv_vcatpvfn_flags (sv.c:13338) ==16556== by 0x1EE430: Perl_sv_vsetpvfn (sv.c:10961) ==16556== by 0x1BB42D: Perl_vmess (util.c:1487) ==16556== by 0x1BABB2: Perl_vcroak (util.c:1716) ==16556== by 0x1BB693: Perl_croak (util.c:1763) ==16556== by 0x19B638: S_grok_bslash_N (regcomp.c:12168) ==16556== by 0x1A20B5: S_regatom (regcomp.c:12932) ==16556== by 0x1A4C8C: S_regpiece (regcomp.c:11668) ==16556== by 0x1A4C8C: S_regbranch (regcomp.c:11593) ==16556== by 0x198431: S_reg (regcomp.c:11331) ==16556== by 0x1AED3B: Perl_re_op_compile (regcomp.c:7100) ==16556== by 0x14DE8C: Perl_pmruntime (op.c:5882) ==16556== Address 0x5bb16aa is 18 bytes after a block of size 120 free'd ==16556== at 0x4C2ED5B: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==16556== by 0x2194BB: Perl_leave_scope (scope.c:929) ==16556== by 0x185986: S_sublex_done (toke.c:2562) ==16556== by 0x174DC9: Perl_yylex (toke.c:4912) ==16556== by 0x1865FB: Perl_yyparse (perly.c:340) ==16556== by 0x15A815: S_parse_body (perl.c:2401) ==16556== by 0x15A815: perl_parse (perl.c:1719) ==16556== by 0x1331F2: main (perlmain.c:121) ==16556== Block was alloc'd at ==16556== at 0x4C2DB2F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==16556== by 0x1BA191: Perl_safesysmalloc (util.c:153) ==16556== by 0x1703F8: S_sublex_push (toke.c:2475) ==16556== by 0x1703F8: Perl_yylex (toke.c:4908) ==16556== by 0x1865FB: Perl_yyparse (perly.c:340) ==16556== by 0x15A815: S_parse_body (perl.c:2401) ==16556== by 0x15A815: perl_parse (perl.c:1719) ==16556== by 0x1331F2: main (perlmain.c:121) ==16556== Invalid hexadecimal number in \N{U+...} in regex; marked by <-- HERE in m/ <-- HERE �PPp0��pP\N{U+.}/ at -e line 1. It looks to me that S_grok_bslash_N is passing correct stuff to the error routines. I single stepped through it, but I would have to do much more detailed study of an area I don't really understand to figure out what's going wrong.
Date: Tue, 20 Jun 2017 08:58:40 +0100
From: Dave Mitchell <davem [...] iabyn.com>
CC: perl5-security-report [...] perl.org
Subject: Re: [perl #131598] Buffer over-read in S_grok_bslash_N
To: Karl Williamson <public [...] khwilliamson.com>
Download (untitled) / with headers
text/plain 1.8k
On Mon, Jun 19, 2017 at 07:23:10PM -0600, Karl Williamson wrote: Show quoted text
> It looks to me that S_grok_bslash_N is passing correct stuff to the error > routines.
Or maybe it isn't :-) vFAIL() is using the "%"UTF8f mechanism to pass a (possibly utf8) string buffer for printing the diagnostic error. It uses this: #define REPORT_LOCATION_ARGS(xC) \ UTF8fARG(UTF, \ (xI(xC) > eC) /* Don't run off end */ \ ? eC - sC /* Length before the <--HERE */ \ : xI_offset(xC), \ sC), /* The input pattern printed up to the <--HERE */ \ UTF8fARG(UTF, \ (xI(xC) > eC) ? 0 : eC - xI(xC), /* Length after <--HERE */ \ (xI(xC) > eC) ? eC : xI(xC)) /* pattern after <--HERE */ Looking at the second UTF8fARG, eC expands to: pRExC_state->precomp_end xI(xC) expands to: pRExC_state->precomp + ((IV) (pRExC_state->precomp_adj + (pRExC_state->parse - pRExC_state->adjusted_start))) At this point I see the following values: (gdb) p pRExC_state->precomp $3 = 0xb30288 "\\N{U+.}" (gdb) p pRExC_state->precomp_end $4 = 0xb3028f "" (gdb) p pRExC_state->precomp_adj $7 = 0x0 (gdb) p pRExC_state->parse $8 = 0xb3028e "}" (gdb) p pRExC_state->adjusted_start $9 = 0xb2ef48 "?:\\x{})" It appears that pRExC_state->adjusted_start is "wild", causing potentially large chunks of garbage to be output. Can you handle it from here? (regcomp.c is, to me, a riddle wrapped in a mystery wrapped in an enigma). -- Red sky at night - gerroff my land! Red sky at morning - gerroff my land! -- old farmers' sayings #14
CC: perl5-security-report [...] perl.org
From: Karl Williamson <public [...] khwilliamson.com>
Date: Tue, 20 Jun 2017 10:46:34 -0600
Subject: Re: [perl #131598] Buffer over-read in S_grok_bslash_N
To: Dave Mitchell <davem [...] iabyn.com>
Download (untitled) / with headers
text/plain 2.4k
On 06/20/2017 01:58 AM, Dave Mitchell wrote: Show quoted text
> On Mon, Jun 19, 2017 at 07:23:10PM -0600, Karl Williamson wrote:
>> It looks to me that S_grok_bslash_N is passing correct stuff to the error >> routines.
> > Or maybe it isn't :-) > > vFAIL() is using the "%"UTF8f mechanism to pass a (possibly utf8) string > buffer for printing the diagnostic error. > > It uses this: > > #define REPORT_LOCATION_ARGS(xC) \ > UTF8fARG(UTF, \ > (xI(xC) > eC) /* Don't run off end */ \ > ? eC - sC /* Length before the <--HERE */ \ > : xI_offset(xC), \ > sC), /* The input pattern printed up to the <--HERE */ \ > UTF8fARG(UTF, \ > (xI(xC) > eC) ? 0 : eC - xI(xC), /* Length after <--HERE */ \ > (xI(xC) > eC) ? eC : xI(xC)) /* pattern after <--HERE */ > > Looking at the second UTF8fARG, > > eC expands to: pRExC_state->precomp_end > > xI(xC) expands to: pRExC_state->precomp > + ((IV) (pRExC_state->precomp_adj > + (pRExC_state->parse - pRExC_state->adjusted_start))) > > > At this point I see the following values: > > (gdb) p pRExC_state->precomp > $3 = 0xb30288 "\\N{U+.}" > (gdb) p pRExC_state->precomp_end > $4 = 0xb3028f "" > (gdb) p pRExC_state->precomp_adj > $7 = 0x0 > (gdb) p pRExC_state->parse > $8 = 0xb3028e "}" > (gdb) p pRExC_state->adjusted_start > $9 = 0xb2ef48 "?:\\x{})" > > It appears that pRExC_state->adjusted_start is "wild", causing potentially > large chunks of garbage to be output. > > Can you handle it from here?
Yes, and thank you for the diagnosis. I have a trivial fix. This is a case of macros hiding stuff not quite completely, allowing me to forget about that stuff, but not hiding it enough that it can safely be completely forgotten about. I will do an audit looking for similar cases in regcomp.c. The question now is how do we handle this. Since the fix is so trivial, it can be backported to whatever releases we deem doing it to. I don't understand what the consequences of this are for an attack. (regcomp.c is, to me, a riddle wrapped in Show quoted text
> a mystery wrapped in an enigma).
That's how I feel about a bunch of the other core code. (And about the regex optimizer)
Subject: Re: [perl #131598] Buffer over-read in S_grok_bslash_N
To: Dave Mitchell <davem [...] iabyn.com>
CC: perl5-security-report [...] perl.org
From: Karl Williamson <public [...] khwilliamson.com>
Date: Tue, 20 Jun 2017 15:46:03 -0600
Download (untitled) / with headers
text/plain 2.6k
On 06/20/2017 10:46 AM, Karl Williamson wrote: Show quoted text
> On 06/20/2017 01:58 AM, Dave Mitchell wrote:
>> On Mon, Jun 19, 2017 at 07:23:10PM -0600, Karl Williamson wrote:
>>> It looks to me that S_grok_bslash_N is passing correct stuff to the >>> error >>> routines.
>> >> Or maybe it isn't :-) >> >> vFAIL() is using the "%"UTF8f mechanism to pass a (possibly utf8) string >> buffer for printing the diagnostic error. >> >> It uses this: >> >> #define >> REPORT_LOCATION_ARGS(xC) \ >> >> UTF8fARG(UTF, \ >> (xI(xC) > eC) /* Don't run off end >> */ \ >> ? eC - sC /* Length before the <--HERE >> */ \ >> : >> xI_offset(xC), \ >> sC), /* The input pattern printed up to the >> <--HERE */ \ >> >> UTF8fARG(UTF, \ >> (xI(xC) > eC) ? 0 : eC - xI(xC), /* Length after <--HERE >> */ \ >> (xI(xC) > eC) ? eC : xI(xC)) /* pattern after >> <--HERE */ >> >> Looking at the second UTF8fARG, >> >> eC expands to: pRExC_state->precomp_end >> >> xI(xC) expands to: pRExC_state->precomp >> + ((IV) (pRExC_state->precomp_adj >> + (pRExC_state->parse - >> pRExC_state->adjusted_start))) >> >> >> At this point I see the following values: >> >> (gdb) p pRExC_state->precomp >> $3 = 0xb30288 "\\N{U+.}" >> (gdb) p pRExC_state->precomp_end >> $4 = 0xb3028f "" >> (gdb) p pRExC_state->precomp_adj >> $7 = 0x0 >> (gdb) p pRExC_state->parse >> $8 = 0xb3028e "}" >> (gdb) p pRExC_state->adjusted_start >> $9 = 0xb2ef48 "?:\\x{})" >> >> It appears that pRExC_state->adjusted_start is "wild", causing >> potentially >> large chunks of garbage to be output. >> >> Can you handle it from here?
> > Yes, and thank you for the diagnosis. I have a trivial fix. This is a > case of macros hiding stuff not quite completely, allowing me to forget > about that stuff, but not hiding it enough that it can safely be > completely forgotten about. > > I will do an audit looking for similar cases in regcomp.c.
My audit indicates this is the only problematic case in regcomp.c Show quoted text
> > The question now is how do we handle this. Since the fix is so trivial, > it can be backported to whatever releases we deem doing it to. I don't > understand what the consequences of this are for an attack. > > (regcomp.c is, to me, a riddle wrapped in
>> a mystery wrapped in an enigma).
> > That's how I feel about a bunch of the other core code. (And about the > regex optimizer) >
To: Karl Williamson <public [...] khwilliamson.com>
Subject: Re: [perl #131598] Buffer over-read in S_grok_bslash_N
From: Dave Mitchell <davem [...] iabyn.com>
Date: Wed, 21 Jun 2017 13:02:49 +0100
CC: perl5-security-report [...] perl.org
On Tue, Jun 20, 2017 at 10:46:34AM -0600, Karl Williamson wrote: Show quoted text
> The question now is how do we handle this. Since the fix is so trivial, it > can be backported to whatever releases we deem doing it to. I don't > understand what the consequences of this are for an attack.
It seems that for certain types of syntax error in a pattern, the error message will either contain the contents of random, possibly large, chunk of memory(*), or will SEGV. So if an attacker can control the pattern, they make be able to DoS, fill up log files, or potentially expose sensitive data containing in the process's address space. I think the last one is least likely - for example if a web page accepted a regex search expression, the error message would go the log file rather than back to the browser. Personally I think we we just fix it in blead, backport in the normal course of affairs, and CVE etc. (*) you probably have a better idea than me about how much memory could be exposed based on the specifics of the "wildness" of that pointer. -- Never work with children, animals, or actors.
Subject: Re: [perl #131598] Buffer over-read in S_grok_bslash_N
To: perl5-security-report [...] perl.org
Date: Wed, 21 Jun 2017 15:07:51 +0200
From: Jakub Wilk <jwilk [...] jwilk.net>
Download (untitled) / with headers
text/plain 651b
* Dave Mitchell, 2017-06-21, 05:03: Show quoted text
>So if an attacker can control the pattern, they make be able to DoS, fill up >log files, or potentially expose sensitive data containing in the process's >address space. > >I think the last one is least likely - for example if a web page accepted a >regex search expression, the error message would go the log file rather than >back to the browser.
It's a bad user experience if the search engine tells you that your query is wrong, but it doesn't tell you why. So for the implementer, it makes sense to wrap regex compilation in eval{} and send $@ back to the user when compilation fails. -- Jakub Wilk
Subject: Re: [perl #131598] Buffer over-read in S_grok_bslash_N
To: Dave Mitchell <davem [...] iabyn.com>
Date: Wed, 21 Jun 2017 22:15:19 -0600
From: Karl Williamson <public [...] khwilliamson.com>
CC: perl5-security-report [...] perl.org
Download (untitled) / with headers
text/plain 1.6k
On 06/21/2017 06:02 AM, Dave Mitchell wrote: Show quoted text
> On Tue, Jun 20, 2017 at 10:46:34AM -0600, Karl Williamson wrote:
>> The question now is how do we handle this. Since the fix is so trivial, it >> can be backported to whatever releases we deem doing it to. I don't >> understand what the consequences of this are for an attack.
> > It seems that for certain types of syntax error in a pattern, > the error message will either contain the contents of random, possibly > large, chunk of memory(*), or will SEGV.
The particular error happens only if someone has a single quotish pattern that contains precisely this sequence \N{U+.} In raising a fatal error, there is a wild pointer. I believe that should point to a copy of a portion of the pattern. The problem is the length is wild, and depends on where libc allocated this space which came from a newSV() call. Show quoted text
> > So if an attacker can control the pattern, they make be able to DoS, fill > up log files, or potentially expose sensitive data containing in the > process's address space. > > I think the last one is least likely - for example if a web page accepted > a regex search expression, the error message would go the log file rather > than back to the browser. > > Personally I think we we just fix it in blead, backport in the normal > course of affairs, and CVE etc.
I don't understand. I thought one wanted to synchronize things so the patch isn't disclosed until all releases are ready, at the same time as the CVE. Show quoted text
> > (*) you probably have a better idea than me about how much memory could be > exposed based on the specifics of the "wildness" of that pointer. >
To: Karl Williamson <public [...] khwilliamson.com>
Subject: Re: [perl #131598] Buffer over-read in S_grok_bslash_N
CC: perl5-security-report [...] perl.org
Date: Thu, 22 Jun 2017 10:31:28 +0100
From: Dave Mitchell <davem [...] iabyn.com>
On Wed, Jun 21, 2017 at 10:15:19PM -0600, Karl Williamson wrote: Show quoted text
> On 06/21/2017 06:02 AM, Dave Mitchell wrote: > In raising a fatal error, there is a wild pointer. I believe that should > point to a copy of a portion of the pattern. The problem is the length is > wild, and depends on where libc allocated this space which came from a > newSV() call.
So it sounds like it has the potential to dump an random large chunk of memory. Show quoted text
> > Personally I think we we just fix it in blead, backport in the normal > > course of affairs, and CVE etc.
> > I don't understand. I thought one wanted to synchronize things so the patch > isn't disclosed until all releases are ready, at the same time as the CVE.
I meant to say "and no CVE etc", although the OP's comment about web server code accepting a pattern string, compiling it under eval {} then returning $@ to the browser perhaps pushes this into CVE territory. -- Please note that ash-trays are provided for the use of smokers, whereas the floor is provided for the use of all patrons. -- Bill Royston
To: Dave Mitchell <davem [...] iabyn.com>
Subject: Re: [perl #131598] Buffer over-read in S_grok_bslash_N
From: Karl Williamson <public [...] khwilliamson.com>
Date: Mon, 10 Jul 2017 21:36:59 -0600
CC: perl5-security-report [...] perl.org
Download (untitled) / with headers
text/plain 1.2k
On 06/22/2017 03:31 AM, Dave Mitchell wrote: Show quoted text
> On Wed, Jun 21, 2017 at 10:15:19PM -0600, Karl Williamson wrote:
>> On 06/21/2017 06:02 AM, Dave Mitchell wrote: >> In raising a fatal error, there is a wild pointer. I believe that should >> point to a copy of a portion of the pattern. The problem is the length is >> wild, and depends on where libc allocated this space which came from a >> newSV() call.
> > So it sounds like it has the potential to dump an random large > chunk of memory.
>>> Personally I think we we just fix it in blead, backport in the normal >>> course of affairs, and CVE etc.
>> >> I don't understand. I thought one wanted to synchronize things so the patch >> isn't disclosed until all releases are ready, at the same time as the CVE.
> > I meant to say "and no CVE etc", although the OP's comment about web > server code accepting a pattern string, compiling it under eval {} then > returning $@ to the browser perhaps pushes this into CVE territory. >
FWIW, at dmq's suggestion at the core hackathon, I have code in the pipeline for 5.27 that makes this moot. But there is a trivial fix available for 5.26.1 and previous maintenance releases. I pinged sawyer about your idea of combining this and 131582. I could do some more investigation about the actual consequences
RT-Send-CC: rt-deliver-to-perl5-security-report [...] rt.perl.org
Download (untitled) / with headers
text/plain 1.2k
On Wed, 21 Jun 2017 21:15:51 -0700, public@khwilliamson.com wrote: Show quoted text
> On 06/21/2017 06:02 AM, Dave Mitchell wrote:
> > On Tue, Jun 20, 2017 at 10:46:34AM -0600, Karl Williamson wrote:
> >> The question now is how do we handle this. Since the fix is so > >> trivial, it > >> can be backported to whatever releases we deem doing it to. I don't > >> understand what the consequences of this are for an attack.
> > > > It seems that for certain types of syntax error in a pattern, > > the error message will either contain the contents of random, > > possibly > > large, chunk of memory(*), or will SEGV.
> > The particular error happens only if someone has a single quotish > pattern that contains precisely this sequence > > \N{U+.} > > In raising a fatal error, there is a wild pointer. I believe that > should point to a copy of a portion of the pattern. The problem is > the > length is wild, and depends on where libc allocated this space which > came from a newSV() call.
If this only happens for single-quotish patterns, then this isn't a security issue, since single quotish patterns don't do interpolation. I wasn't able to reproduce it (but I only tested on x64). Can it happen in other cases, like: $x = "\\N{U+.}"; $foo =~ $x ? Tony
Subject: Re: [perl #131598] Buffer over-read in S_grok_bslash_N
From: Jakub Wilk <jwilk [...] jwilk.net>
Date: Fri, 11 Aug 2017 11:59:51 +0200
To: perl5-security-report [...] perl.org
Download (untitled) / with headers
text/plain 439b
* Tony Cook via RT <perl5-security-report@perl.org>, 2017-08-11, 02:30: Show quoted text
>If this only happens for single-quotish patterns, then this isn't a security >issue, since single quotish patterns don't do interpolation. > >I wasn't able to reproduce it (but I only tested on x64). > >Can it happen in other cases, like: > > $x = "\\N{U+.}"; > $foo =~ $x > >?
Yes; I get the same symptoms for this code as for the original one. -- Jakub Wilk
RT-Send-CC: rt-deliver-to-perl5-security-report [...] rt.perl.org
Download (untitled) / with headers
text/plain 678b
On Fri, 11 Aug 2017 05:45:54 -0700, jwilk@jwilk.net wrote: Show quoted text
> * Tony Cook via RT <perl5-security-report@perl.org>, 2017-08-11, > 02:30:
> > If this only happens for single-quotish patterns, then this isn't a > > security > > issue, since single quotish patterns don't do interpolation. > > > > I wasn't able to reproduce it (but I only tested on x64). > > > > Can it happen in other cases, like: > > > > $x = "\\N{U+.}"; > > $foo =~ $x > > > > ?
> > Yes; I get the same symptoms for this code as for the original one.
Thanks, so it's a security issue. I think it should be treated as a separate CVE from #131582, since they're different bugs and different consequences. Tony
RT-Send-CC: rt-deliver-to-perl5-security-report [...] rt.perl.org
Download (untitled) / with headers
text/plain 590b
On Fri, 11 Aug 2017 18:00:15 -0700, tonyc wrote: Show quoted text
> On Fri, 11 Aug 2017 05:45:54 -0700, jwilk@jwilk.net wrote:
> > * Tony Cook via RT <perl5-security-report@perl.org>, 2017-08-11, > > 02:30:
> > > If this only happens for single-quotish patterns, then this isn't a > > > security > > > issue, since single quotish patterns don't do interpolation. > > > > > > I wasn't able to reproduce it (but I only tested on x64). > > > > > > Can it happen in other cases, like: > > > > > > $x = "\\N{U+.}"; > > > $foo =~ $x > > >
>
So my claim that it only happens with a single-quoted pattern was wrong
RT-Send-CC: rt-deliver-to-perl5-security-report [...] rt.perl.org
Download (untitled) / with headers
text/plain 794b
On Fri, 11 Aug 2017 18:00:15 -0700, tonyc wrote: Show quoted text
> On Fri, 11 Aug 2017 05:45:54 -0700, jwilk@jwilk.net wrote:
> > * Tony Cook via RT <perl5-security-report@perl.org>, 2017-08-11, > > 02:30:
> > > If this only happens for single-quotish patterns, then this isn't a > > > security > > > issue, since single quotish patterns don't do interpolation. > > > > > > I wasn't able to reproduce it (but I only tested on x64). > > > > > > Can it happen in other cases, like: > > > > > > $x = "\\N{U+.}"; > > > $foo =~ $x > > > > > > ?
> > > > Yes; I get the same symptoms for this code as for the original one.
> > Thanks, so it's a security issue. > > I think it should be treated as a separate CVE from #131582, since > they're different bugs and different consequences.
This is CVE-2017-12883. Tony
RT-Send-CC: rt-deliver-to-perl5-security-report [...] rt.perl.org
Download (untitled) / with headers
text/plain 327b
On Mon, 10 Jul 2017 20:37:20 -0700, public@khwilliamson.com wrote: Show quoted text
> FWIW, at dmq's suggestion at the core hackathon, I have code in the > pipeline for 5.27 that makes this moot. But there is a trivial fix > available for 5.26.1 and previous maintenance releases.
Could you post that fix here as a patch please? Thanks, Tony
RT-Send-CC: rt-deliver-to-perl5-security-report [...] rt.perl.org
Download (untitled) / with headers
text/plain 517b
On Wed, 16 Aug 2017 16:55:52 -0700, tonyc wrote: Show quoted text
> On Mon, 10 Jul 2017 20:37:20 -0700, public@khwilliamson.com wrote:
> > FWIW, at dmq's suggestion at the core hackathon, I have code in the > > pipeline for 5.27 that makes this moot. But there is a trivial fix > > available for 5.26.1 and previous maintenance releases.
> > Could you post that fix here as a patch please? > > Thanks, > Tony
Will do, somehow this request didn't get emailed to me. It should go in 26.1, but that discloses it -- Karl Williamson
RT-Send-CC: rt-deliver-to-perl5-security-report [...] rt.perl.org
Download (untitled) / with headers
text/plain 789b
On Thu, 24 Aug 2017 13:08:10 -0700, khw wrote: Show quoted text
> On Wed, 16 Aug 2017 16:55:52 -0700, tonyc wrote:
> > On Mon, 10 Jul 2017 20:37:20 -0700, public@khwilliamson.com wrote:
> > > FWIW, at dmq's suggestion at the core hackathon, I have code in the > > > pipeline for 5.27 that makes this moot. But there is a trivial fix > > > available for 5.26.1 and previous maintenance releases.
> > > > Could you post that fix here as a patch please? > > > > Thanks, > > Tony
> > > Will do, somehow this request didn't get emailed to me.
Yeah, it's possible not all RT responses are going through to the list. Show quoted text
> It should go > in 26.1, but that discloses it
I'm not asking that it go into blead, ideally the patch would get disclosed publicly and committed to blead, maint all at the same time. Tony
RT-Send-CC: davem [...] iabyn.com, rt-deliver-to-perl5-security-report [...] rt.perl.org
On Thu, 24 Aug 2017 21:45:31 -0700, tonyc wrote: Show quoted text
> On Thu, 24 Aug 2017 13:08:10 -0700, khw wrote:
> > On Wed, 16 Aug 2017 16:55:52 -0700, tonyc wrote:
> > > On Mon, 10 Jul 2017 20:37:20 -0700, public@khwilliamson.com wrote:
> > > > FWIW, at dmq's suggestion at the core hackathon, I have code in > > > > the > > > > pipeline for 5.27 that makes this moot. But there is a trivial > > > > fix > > > > available for 5.26.1 and previous maintenance releases.
> > > > > > Could you post that fix here as a patch please? > > > > > > Thanks, > > > Tony
> > > > > > Will do, somehow this request didn't get emailed to me.
> > Yeah, it's possible not all RT responses are going through to the > list.
Is there any way to fix this? Show quoted text
>
> > It should go > > in 26.1, but that discloses it
> > I'm not asking that it go into blead, ideally the patch would get > disclosed publicly and committed to blead, maint all at the same time. > > Tony
Attached. What I neglected to say is that I think this should go into 5.24.3, so that would mean that release and 5.26.1 and blead all have to be updated in sync. -- Karl Williamson
Subject: 0002-PATCH-perl-131598.patch
From d4857ae370c5a111b5f0ae61c3ae05e0b99e2237 Mon Sep 17 00:00:00 2001 From: Karl Williamson <khw@cpan.org> Date: Fri, 25 Aug 2017 11:33:58 -0600 Subject: [PATCH 2/2] PATCH: [perl #131598] The cause of this is that the vFAIL macro uses RExC_parse, and that variable has just been changed in preparation for code after the vFAIL. The solution is to not change RExC_parse until after the vFAIL. This is a case where the macro hides stuff that can bite you. --- regcomp.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/regcomp.c b/regcomp.c index 6a07bf2c70..4eff83e50e 100644 --- a/regcomp.c +++ b/regcomp.c @@ -12199,14 +12199,16 @@ S_grok_bslash_N(pTHX_ RExC_state_t *pRExC_state, } sv_catpv(substitute_parse, ")"); - RExC_parse = RExC_start = RExC_adjusted_start = SvPV(substitute_parse, - len); + len = SvCUR(substitute_parse); /* Don't allow empty number */ if (len < (STRLEN) 8) { RExC_parse = endbrace; vFAIL("Invalid hexadecimal number in \\N{U+...}"); } + + RExC_parse = RExC_start = RExC_adjusted_start + = SvPV_nolen(substitute_parse); RExC_end = RExC_parse + len; /* The values are Unicode, and therefore not subject to recoding, but -- 2.11.0
RT-Send-CC: perl5-security-report [...] perl.org
Download (untitled) / with headers
text/plain 117b
Now in blead as commit 2be4edede4ae226e2eebd4eff28cedd2041f300f. Will shortly also be in 5.24.3-RC1 and 5.26.1-RC1...
Subject: Re: [perl #131598] [CVE-2017-12883]Buffer over-read in S_grok_bslash_N
From: Sawyer X <xsawyerx [...] gmail.com>
Date: Mon, 11 Sep 2017 12:12:48 +0300
CC: Perl 5 Security Report <perl5-security-report [...] perl.org>
To: rt-comment [...] perl.org
Download (untitled) / with headers
text/plain 276b
This was set to be disclosed on September 22nd. Disclosure list was informed. On 10 September 2017 at 16:16, Steve Hay via RT <rt-comment@perl.org> wrote: Show quoted text
> Now in blead as commit 2be4edede4ae226e2eebd4eff28cedd2041f300f. Will shortly also be in 5.24.3-RC1 and 5.26.1-RC1...
RT-Send-CC: perl5-security-report [...] perl.org
Download (untitled) / with headers
text/plain 1.5k
On Wed, 16 Aug 2017 16:53:18 -0700, tonyc wrote: Show quoted text
> This is CVE-2017-12883.
The details I entered when requesting the CVE ID: Show quoted text
> [Suggested description] > Buffer over-read when compiling a crafted regular expression > ------------------------------------------ > [Vulnerability Type] > Buffer Overflow > > ------------------------------------------ > [Vendor of Product] > Perl5 Porters > ------------------------------------------ > [Affected Product Code Base] > perl - 5.26.0 > ------------------------------------------ > [Affected Component] > > ------------------------------------------ > [Attack Type] > Local > > ------------------------------------------ > [Impact ] > > [-] CVE_Request.Impact_Code_execution > [-] CVE_Request.Impact_Denial_of_Service > [-] CVE_Request.Impact_Escalation_of_Privileges > [+] CVE_Request.Impact_Information_Disclosure
Proposed update for the CVE entry once the issue is public (the field names are from the CVE allocation form): Affected components: regular expression compiler, S_grok_bslash_N() in regcomp.c Attack vector: An attacker can provide a crafted regular expression with an invalid \N{U+...} escape, the error message produced will access memory beyond the end of a memory block, possibly including sensitive information stored after the regular expression string, or crashing perl. Discoverer: Jakub Wilk <jwilk@jwilk.net> Affected Product Code Base: perl - 5.26.0, fixed in 5.26.1 perl - 5.24.x, fixed in 5.24.3 perl - 5.22.x and earlier References: https://rt.perl.org/Public/Bug/Display.html?id=131598 Additional information: (none) Tony
RT-Send-CC: perl5-security-report [...] perl.org
Download (untitled) / with headers
text/plain 272b
On Mon, 11 Sep 2017 15:48:41 -0700, tonyc wrote: Show quoted text
> Affected Product Code Base: > > perl - 5.26.0, fixed in 5.26.1 > perl - 5.24.x, fixed in 5.24.3 > perl - 5.22.x and earlier
perl - 5.26.0, fixed in 5.26.1 perl - 5.24.x, fixed in 5.24.3 perl - 5.20 through 5.22.x Tony
RT-Send-CC: perl5-porters [...] perl.org
Now open.
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 104b
On Mon, 25 Sep 2017 03:10:11 -0700, xsawyerx@cpan.org wrote: Show quoted text
> Now open.
Update to CVE requested. Tony


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

For issues related to this RT instance (aka "perlbug"), please contact perlbug-admin at perl.org