Possible Buffer Overflow Perl 5.21.8 #14416
Comments
From @geeknikI am currently fuzzing Perl 5.21.8 built from git source (This is perl 5, For compiling, I performed the following: All other Perl options during the process were defaults (pressed enter) Here is the Valgrind and GDB output. I've attached the crashing test case geeknik@deb7fuzz:~/findings/perl5/fuzzer02/crashes$ valgrind -q Backslash found where operator expected at *** stack smashing detected ***: /home/geeknik/perl5/perl terminated ==18174== Invalid read of size 1 geeknik@deb7fuzz:~/findings/perl5/fuzzer02/crashes$ Backslash found where operator expected at geeknik@deb7fuzz:~/findings/perl5/fuzzer02/crashes$ gdb GNU gdb (GDB) 7.4.1-debian This is free software: you are free to change and redistribute it. warning: Can't read pathname for load map: Input/output error. I have at least a dozen more of these but I'm not all that great at Regards, Brian 'geeknik' Carpenter |
From @hvdsFWIW I've managed to reduce this to: Hugo |
|
The RT System itself - Status changed from 'new' to 'open' |
From @hvdsI wrote: Ah, that's easy then, 'tmpbuf' below is a stack array: I'm not up on stack-smashing techniques, but the isALPHA() restriction This code goes back at least as far as origin/maint-5.004. Simplistic patch below is enough to defang the short test case above and Hugo Inline Patchdiff --git a/toke.c b/toke.c
index dfb5b20..84cb1e7 100644
--- a/toke.c
+++ b/toke.c
@@ -3828,11 +3828,13 @@ S_intuit_more(pTHX_ char *s)
|| last_un_char == '&')
&& isALPHA(*s) && s[1] && isALPHA(s[1])) {
char *d = tmpbuf;
- while (isALPHA(*s))
+ while (isALPHA(*s) && d < tmpbuf + sizeof(tmpbuf))
*d++ = *s++;
- *d = '\0';
- if (keyword(tmpbuf, d - tmpbuf, 0))
- weight -= 150;
+ if (d < tmpbuf + sizeof(tmpbuf)) {
+ *d = '\0';
+ if (keyword(tmpbuf, d - tmpbuf, 0))
+ weight -= 150;
+ }
}
if (un_char == last_un_char + 1)
weight += 5; |
From @hvds:Simplistic patch below is enough to defang the short test case [...] and Actually, we don't seem to need tmpbuf at all here; the alternative patch Hugo commit 128e9aa798d9b1fe194ebf84c97938fb8f720972 intuit_more: no need to copy before keyword check Inline Patchdiff --git a/toke.c b/toke.c
index dfb5b20..01b0527 100644
--- a/toke.c
+++ b/toke.c
@@ -3827,11 +3827,10 @@ S_intuit_more(pTHX_ char *s)
&& !(last_un_char == '$' || last_un_char == '@'
|| last_un_char == '&')
&& isALPHA(*s) && s[1] && isALPHA(s[1])) {
- char *d = tmpbuf;
+ char *d = s;
while (isALPHA(*s))
- *d++ = *s++;
- *d = '\0';
- if (keyword(tmpbuf, d - tmpbuf, 0))
+ s++;
+ if (keyword(d, s - d, 0))
weight -= 150;
}
if (un_char == last_un_char + 1) |
From @hvdsI hope that someone other than me will comment at some point. I can't decide whether this bug merits a CVE. It's probably exploitable That it is triggered at compile-time also makes it worse: I'm not sure Hugo |
From @iabynOn Sat, Jan 17, 2015 at 01:28:56PM +0000, hv@crypt.org wrote:
If I've understood correctly, the code is failing when parsing a *literal* Thus there is no way that someone can can pass a non-literal pattern while (<>) { And if some code allows people to specify *literal* patterns, then the /foo$a[do { BEGIN { system "rm -rf /" } 1}]/ -- |
From @hvdsDave Mitchell <davem@iabyn.com> wrote: Yes, I believe so. :Thus there is no way that someone can can pass a non-literal pattern Yep, makes sense to me. In that case I'll aim to push the fix on Sunday or Monday. Hugo |
From @khwilliamsonOn 01/20/2015 06:33 AM, hv@crypt.org wrote:
hv++ |
From @steve-m-hayOn Tue Jan 20 05:33:44 2015, hv wrote:
Thanks, I'm marking this ticket as "resolved" (since there doesn't seem to be a "pending release" here?) and added 56f81af to the cherry-pick-votes file for maint-5.20 (in be11ff5). |
|
@steve-m-hay - Status changed from 'open' to 'resolved' |
From @rjbsBarring any objection, I will move this to the perl5 queue on Sunday. -- |
Migrated from rt.perl.org#123604 (status was 'resolved')
Searchable as RT123604$
The text was updated successfully, but these errors were encountered: