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

Vulnerability report - perl sprintf implementation #15970

Closed
p5pRT opened this issue May 5, 2017 · 28 comments
Closed

Vulnerability report - perl sprintf implementation #15970

p5pRT opened this issue May 5, 2017 · 28 comments

Comments

@p5pRT
Copy link
Collaborator

@p5pRT p5pRT commented May 5, 2017

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

Searchable as RT131260$

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented May 5, 2017

From eyal.itkin@gmail.com

Hello,

In a security audit to the sprintf implementation in perl (version 5.24.1)
I found a major security vulnerability, here are the full details​:

*Technical Details​:*
file​: sv.c
function​: Perl_sv_vcatpvfn_flags

  - precis - represents a format's precision, and can be any size_t value
  - width - represents the format's width, and can be any non-negative
  size_t value
  - Using large values can cause multiple Integer-Overflows when
  calculating 'need' = the needed allocated space for a fraction
  - line 12300​: need += has_precis ? precis : 6; /* known
  default */
  - later on more values are added to need (need += 20, ...)
  - Later on the use of 'width' for padding with spaces assumes that there
  is enough space in the buffer, causing a Buffer Overflow

*PoC Script​:*

print sprintf("%2000.2000f this is a spacer %4000.4294967245a", 1,
0x0.00008234p+9);

*Crash trace - tested on a 32 bit linux machine​:*

Program terminated with signal SIGSEGV, Segmentation fault.
#0 0x080ebbe0 in Perl_runops_standard ()
(gdb) bt
#0 0x080ebbe0 in Perl_runops_standard ()
#1 0x08069356 in S_fold_constants ()
#2 0x080a8336 in Perl_yyparse ()
#3 0x08083219 in perl_parse ()
#4 0x0806218c in main ()
(gdb) info reg
eax 0x20202020 538976288
ecx 0x9f72900 167192832
edx 0x0 0
ebx 0x0 0
esp 0xbfe0e8c0 0xbfe0e8c0
ebp 0x9f6c4d0 0x9f6c4d0
esi 0x9f6f58c 167179660
edi 0x0 0
eip 0x80ebbe0 0x80ebbe0 <Perl_runops_standard+16>
eflags 0x10202 [ IF RF ]
cs 0x73 115
ss 0x7b 123
ds 0x7b 123
es 0x7b 123
fs 0x0 0
gs 0x33 51
(gdb) x /10i $eip
=> 0x80ebbe0 <Perl_runops_standard+16>​: call *0x8(%eax)
  0x80ebbe3 <Perl_runops_standard+19>​: test %eax,%eax
  0x80ebbe5 <Perl_runops_standard+21>​: mov %eax,0x8207a88
  0x80ebbea <Perl_runops_standard+26>​:
  jne 0x80ebbe0 <Perl_runops_standard+16>
  0x80ebbec <Perl_runops_standard+28>​: mov 0x8207290,%eax
  0x80ebbf1 <Perl_runops_standard+33>​: test %eax,%eax
  0x80ebbf3 <Perl_runops_standard+35>​:
  jne 0x80ebc02 <Perl_runops_standard+50>
  0x80ebbf5 <Perl_runops_standard+37>​: movb $0x0,0x82072b0
  0x80ebbfc <Perl_runops_standard+44>​: xor %eax,%eax
  0x80ebbfe <Perl_runops_standard+46>​: add $0xc,%esp

As you can see, the first %f print enlarged the buffer, and the
controllable width in the second overflowed the buffer, causing eax to be
corrupt (0x20 == ' ').

*Threat Analysis​:*

This report demonstrates a potential code execution vulnerability, in case
a sprintf format will be hostile. As format string attacks are relatively
more popular on C/C++ languages, they are caused by the same programming
bad practices​:

  - "Generic" logging \ monitoring module that receives format + args, and
  build the message string

Such kind of vulnerabilities are still common in modern code projects, and
during my career I found such vulnerabilities even in many high-profile
companies. Since Perl (such as Python, and Ruby) is a memory-managed
language, the programmer relies on the interpreter to "catch" problems for
him, meaning that most programmers assume that these programs won't be
vulnerable to format string attacks.

*Conclusion​:*
The sprintf implementation has a severe vulnerability when handling a
hostile format string. This vulnerability was demonstrated and can be
easily leveraged into a remote code execution on the Perl interpreter (as I
demonstrated last month in a similar vulnerability in the MRuby project).

In case there are any questions regarding the vulnerability or the report,
I will be more than happy to help.
Thanks for your attention,
Eyal Itkin.

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented May 8, 2017

From @iabyn

On Fri, May 05, 2017 at 03​:01​:51PM -0700, Eyal Itkin wrote​:

In a security audit to the sprintf implementation in perl (version 5.24.1)
I found a major security vulnerability, here are the full details​:

The same vulnerability persists in blead.

Here is my provisional analysis.

This function, Perl_sv_vcatpvfn_flags, is perl's lowest-level sprintf-ish
function​: all the others call this. It is a huge sprawling mess of code,
with about 1700 lines and 36 goto's to 11 labels.

There appear to be three places within that function where memory is
allocated and could thus potentially under-allocate if the 'needed'
calculations wrapped at some point. Of these,

The main SvGROW() allocates extra space in the output string to append the
next element, including any sign, padding, zeros etc. This appears to be
safe from wrapping.

A second SvGROW() is used when appending a bad format element as-is, e.g.
""%7. 6s". Triggering a wrap here would require the output-so-far plus the
length of the malformed format segment to wrap, which is improbable and
almost certainly impossible.

Finally, a Newx(PL_efloatbuf, PL_efloatsize, char) is done for
floating-point-related format elements. That is the one for which a buffer
overrun is demonstrated in this ticket.

Given the complexity of this function, it's possible that the description
above is incomplete. But for the rest of this email I'll only discuss the
third case above.

When dealing with floating-point formats like e/f/g/a, perl allocates a
buffer big enough to hold the actual stringified numerical value. Later,
this buffer is appended to the output SV along with any padding, sign etc.

As the OP points out, there are a number of calculations done to determine
the needed size of PL_efloatbuf, which rely on the (possibly hostile)
width and precision values in the "%NNN.PPPf" format, and many of these
calculations can overflow, leading to an under-allocated PL_efloatbuf.

This means that part of the f/p string will be written beyond the end of
PL_efloatbuf. It's not possible write arbitrary data, but any sequence
of bytes mainly from the set /[0-9 ep+\-]/, with long runs of 0x20's or
0x30's easily selectable by varying the width and precision.

As the OP demonstrated, it's possible to overrun a PL_op->op_ppaddr field
or similar, corrupting the address of a function to call. In the case of
the OP, this resulting in trying to call the function at the address
0x202020..., and the inevitable crash.

So this is bad, but the question is how bad.

Obviously, any badly-written logging code which accepts a hostile sprintf
format string and possibly hostile sprintf args, is already broken even if
perl's sprintf implementation was bug-free, with hostile widths and
precison allowing the attacker to force the perl executable to consume as
much memory as the attacker desired, or to cause the perl process to die
with an "out of memory" error or similar. Or to fill up a log file.

But with a buggy perl implementation, this could cause an existing DoS
situation to escalate to something worse.

My gut feeling is that given the limited "palette" of byte values that
could be used to overwrite the end of the buffer, it would be hard to
achieve an arbitrary code execution. But I'm sure it could do a lot of
damage.

I will have a patch ready shortly for the specific PL_efloatbuf issues.
In the longer term Perl_sv_vcatpvfn_flags() really needs an overhaul
to make it simpler to analyse and avoid re-introducing bugs.

So in the short term, is this issue serious enough that my patch or
similar should be added to branches leading to the 5.26.0, 5.24.2 and
5.22.4 releases, or should it wait? This issue or similar appears to have
been present since at least 5.10.0 and probably earlier.

--
"You may not work around any technical limitations in the software"
  -- Windows Vista license

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented May 8, 2017

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

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented May 9, 2017

From @iabyn

On Mon, May 08, 2017 at 06​:17​:27PM +0100, Dave Mitchell wrote​:

So in the short term, is this issue serious enough that my patch or
similar should be added to branches leading to the 5.26.0, 5.24.2 and
5.22.4 releases, or should it wait? This issue or similar appears to have
been present since at least 5.10.0 and probably earlier.

With the pumpking's consent I've just committed v5.25.12-29-gbd1a29f to
blead, which has a basic fix for this issue. This is just before the RC1
release is due.

That patch is also suitable for maint-5.22 and maint-5.24.

--
Technology is dominated by two types of people​: those who understand what
they do not manage, and those who manage what they do not understand.

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented May 9, 2017

From eyal.itkin@gmail.com

Thanks for the feedback. I saw the commit in the git and it seems to solve
the reported vulnerability.

Please notify me when the commit will be merged to all relevant branches,
so I would be able to proceed with the publication to the bug bounty
program of perl in hackerOne.

Thanks again for the fast reply.
Eyal

On May 9, 2017 10​:25 AM, "Dave Mitchell via RT" <perl5-security-report@​perl.
org> wrote​:

On Mon, May 08, 2017 at 06​:17​:27PM +0100, Dave Mitchell wrote​:

So in the short term, is this issue serious enough that my patch or
similar should be added to branches leading to the 5.26.0, 5.24.2 and
5.22.4 releases, or should it wait? This issue or similar appears to have
been present since at least 5.10.0 and probably earlier.

With the pumpking's consent I've just committed v5.25.12-29-gbd1a29f to
blead, which has a basic fix for this issue. This is just before the RC1
release is due.

That patch is also suitable for maint-5.22 and maint-5.24.

--
Technology is dominated by two types of people​: those who understand what
they do not manage, and those who manage what they do not understand.

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented May 15, 2017

From @iabyn

On Tue, May 09, 2017 at 08​:24​:51AM +0100, Dave Mitchell wrote​:

On Mon, May 08, 2017 at 06​:17​:27PM +0100, Dave Mitchell wrote​:

So in the short term, is this issue serious enough that my patch or
similar should be added to branches leading to the 5.26.0, 5.24.2 and
5.22.4 releases, or should it wait? This issue or similar appears to have
been present since at least 5.10.0 and probably earlier.

With the pumpking's consent I've just committed v5.25.12-29-gbd1a29f to
blead, which has a basic fix for this issue. This is just before the RC1
release is due.

That patch is also suitable for maint-5.22 and maint-5.24.

Steve, would it be ok for me to cherry-pick these into maint-5.22 and
maint-5.24 now? IIUC, 5.24.2 and 5.22.4 are supposed to be security-only
releases, but this is a security fix.

To everyone else - I'm currently auditing/hardening the code in
Perl_sv_vcatpvfn_flags(), with a (not-yet-pushed) branch that I intend to
merge into blead post 5.16.09 release (unless I find anything else
exceptionally nasty in the meantime.

--
A problem shared is a problem doubled.

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented May 15, 2017

From @khwilliamson

On 05/15/2017 04​:15 AM, Dave Mitchell wrote​:

On Tue, May 09, 2017 at 08​:24​:51AM +0100, Dave Mitchell wrote​:

On Mon, May 08, 2017 at 06​:17​:27PM +0100, Dave Mitchell wrote​:

So in the short term, is this issue serious enough that my patch or
similar should be added to branches leading to the 5.26.0, 5.24.2 and
5.22.4 releases, or should it wait? This issue or similar appears to have
been present since at least 5.10.0 and probably earlier.

With the pumpking's consent I've just committed v5.25.12-29-gbd1a29f to
blead, which has a basic fix for this issue. This is just before the RC1
release is due.

That patch is also suitable for maint-5.22 and maint-5.24.

Steve, would it be ok for me to cherry-pick these into maint-5.22 and
maint-5.24 now? IIUC, 5.24.2 and 5.22.4 are supposed to be security-only
releases, but this is a security fix.

To everyone else - I'm currently auditing/hardening the code in
Perl_sv_vcatpvfn_flags(), with a (not-yet-pushed) branch that I intend to
merge into blead post 5.16.09 release (unless I find anything else
exceptionally nasty in the meantime.

I'm trying to grok what '5.16.09' means here, without success.

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented May 15, 2017

From @iabyn

On Mon, May 15, 2017 at 09​:39​:03AM -0600, Karl Williamson wrote​:

On 05/15/2017 04​:15 AM, Dave Mitchell wrote​:

On Tue, May 09, 2017 at 08​:24​:51AM +0100, Dave Mitchell wrote​:

On Mon, May 08, 2017 at 06​:17​:27PM +0100, Dave Mitchell wrote​:

So in the short term, is this issue serious enough that my patch or
similar should be added to branches leading to the 5.26.0, 5.24.2 and
5.22.4 releases, or should it wait? This issue or similar appears to have
been present since at least 5.10.0 and probably earlier.

With the pumpking's consent I've just committed v5.25.12-29-gbd1a29f to
blead, which has a basic fix for this issue. This is just before the RC1
release is due.

That patch is also suitable for maint-5.22 and maint-5.24.

Steve, would it be ok for me to cherry-pick these into maint-5.22 and
maint-5.24 now? IIUC, 5.24.2 and 5.22.4 are supposed to be security-only
releases, but this is a security fix.

To everyone else - I'm currently auditing/hardening the code in
Perl_sv_vcatpvfn_flags(), with a (not-yet-pushed) branch that I intend to
merge into blead post 5.16.09 release (unless I find anything else
exceptionally nasty in the meantime.

I'm trying to grok what '5.16.09' means here, without success.

That's an astoundingly badly typed 5.26.0

--
Monto Blanco... scorchio!

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented May 15, 2017

From @xsawyerx

Hi Eyal,

Do you have a CVE number to go along with the disclosure?
(Not all security issues must have a CVE, I'm just wondering if you have
one for this.)

On Tue, May 9, 2017 at 10​:02 AM, Eyal Itkin <eyal.itkin@​gmail.com> wrote​:

Thanks for the feedback. I saw the commit in the git and it seems to solve
the reported vulnerability.

Please notify me when the commit will be merged to all relevant branches,
so I would be able to proceed with the publication to the bug bounty
program of perl in hackerOne.

Thanks again for the fast reply.
Eyal

On May 9, 2017 10​:25 AM, "Dave Mitchell via RT" <
perl5-security-report@​perl.org> wrote​:

On Mon, May 08, 2017 at 06​:17​:27PM +0100, Dave Mitchell wrote​:

So in the short term, is this issue serious enough that my patch or
similar should be added to branches leading to the 5.26.0, 5.24.2 and
5.22.4 releases, or should it wait? This issue or similar appears to
have
been present since at least 5.10.0 and probably earlier.

With the pumpking's consent I've just committed v5.25.12-29-gbd1a29f to
blead, which has a basic fix for this issue. This is just before the RC1
release is due.

That patch is also suitable for maint-5.22 and maint-5.24.

--
Technology is dominated by two types of people​: those who understand what
they do not manage, and those who manage what they do not understand.

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented May 16, 2017

From eyal.itkin@gmail.com

No, I didn't​ issue a CVE id for this vulnerability.

Issuing CVE ids to open source interpreter languages is always tricky, as
one cannot know when it will be "good enough". PHP issues CVEs even to
simple DoS vulnerabilities, while (C)Ruby will argue all the way that "this
is just a simple bug" and won't agree to tag it as a "security" issue.

If you prefer that a CVE id will be assigned to this vulnerability, I can
contact MITRE to issue one.

On May 16, 2017 00​:12, "Sawyer X via RT" <perl5-security-report@​perl.org>
wrote​:

Hi Eyal,

Do you have a CVE number to go along with the disclosure?
(Not all security issues must have a CVE, I'm just wondering if you have
one for this.)

On Tue, May 9, 2017 at 10​:02 AM, Eyal Itkin <eyal.itkin@​gmail.com> wrote​:

Thanks for the feedback. I saw the commit in the git and it seems to
solve
the reported vulnerability.

Please notify me when the commit will be merged to all relevant branches,
so I would be able to proceed with the publication to the bug bounty
program of perl in hackerOne.

Thanks again for the fast reply.
Eyal

On May 9, 2017 10​:25 AM, "Dave Mitchell via RT" <
perl5-security-report@​perl.org> wrote​:

On Mon, May 08, 2017 at 06​:17​:27PM +0100, Dave Mitchell wrote​:

So in the short term, is this issue serious enough that my patch or
similar should be added to branches leading to the 5.26.0, 5.24.2 and
5.22.4 releases, or should it wait? This issue or similar appears to
have
been present since at least 5.10.0 and probably earlier.

With the pumpking's consent I've just committed v5.25.12-29-gbd1a29f to
blead, which has a basic fix for this issue. This is just before the RC1
release is due.

That patch is also suitable for maint-5.22 and maint-5.24.

--
Technology is dominated by two types of people​: those who understand
what
they do not manage, and those who manage what they do not understand.

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented May 16, 2017

From @xsawyerx

If someone hasn't expressed an opinion that this requires a CVE by now, we
are not likely to assign one ourselves. If it did have one, we would have
included it in the change log.

Thanks. :)

On Tue, May 16, 2017 at 8​:08 AM, Eyal Itkin <eyal.itkin@​gmail.com> wrote​:

No, I didn't​ issue a CVE id for this vulnerability.

Issuing CVE ids to open source interpreter languages is always tricky, as
one cannot know when it will be "good enough". PHP issues CVEs even to
simple DoS vulnerabilities, while (C)Ruby will argue all the way that "this
is just a simple bug" and won't agree to tag it as a "security" issue.

If you prefer that a CVE id will be assigned to this vulnerability, I can
contact MITRE to issue one.

On May 16, 2017 00​:12, "Sawyer X via RT" <perl5-security-report@​perl.org>
wrote​:

Hi Eyal,

Do you have a CVE number to go along with the disclosure?
(Not all security issues must have a CVE, I'm just wondering if you have
one for this.)

On Tue, May 9, 2017 at 10​:02 AM, Eyal Itkin <eyal.itkin@​gmail.com> wrote​:

Thanks for the feedback. I saw the commit in the git and it seems to
solve
the reported vulnerability.

Please notify me when the commit will be merged to all relevant
branches,
so I would be able to proceed with the publication to the bug bounty
program of perl in hackerOne.

Thanks again for the fast reply.
Eyal

On May 9, 2017 10​:25 AM, "Dave Mitchell via RT" <
perl5-security-report@​perl.org> wrote​:

On Mon, May 08, 2017 at 06​:17​:27PM +0100, Dave Mitchell wrote​:

So in the short term, is this issue serious enough that my patch or
similar should be added to branches leading to the 5.26.0, 5.24.2 and
5.22.4 releases, or should it wait? This issue or similar appears to
have
been present since at least 5.10.0 and probably earlier.

With the pumpking's consent I've just committed v5.25.12-29-gbd1a29f to
blead, which has a basic fix for this issue. This is just before the
RC1
release is due.

That patch is also suitable for maint-5.22 and maint-5.24.

--
Technology is dominated by two types of people​: those who understand
what
they do not manage, and those who manage what they do not understand.

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented May 17, 2017

From @demerphq

Personally I would say that the use of untrusted format strings is rare in
the perl community and even less likely in hostile environments. Thus it
probably doesn't merit a CVE but the core bug class does and if this use
case were more common we would want one.

So I guess we are somewhere in the middle of the Ruby and PHP guys on this
subject.

Yves

On 16 May 2017 11​:24, "Sawyer X" <xsawyerx@​gmail.com> wrote​:

If someone hasn't expressed an opinion that this requires a CVE by now, we
are not likely to assign one ourselves. If it did have one, we would have
included it in the change log.

Thanks. :)
On Tue, May 16, 2017 at 8​:08 AM, Eyal Itkin <eyal.itkin@​gmail.com> wrote​:

No, I didn't​ issue a CVE id for this vulnerability.

Issuing CVE ids to open source interpreter languages is always tricky, as
one cannot know when it will be "good enough". PHP issues CVEs even to
simple DoS vulnerabilities, while (C)Ruby will argue all the way that "this
is just a simple bug" and won't agree to tag it as a "security" issue.

If you prefer that a CVE id will be assigned to this vulnerability, I can
contact MITRE to issue one.

On May 16, 2017 00​:12, "Sawyer X via RT" <perl5-security-report@​perl.org>
wrote​:

Hi Eyal,

Do you have a CVE number to go along with the disclosure?
(Not all security issues must have a CVE, I'm just wondering if you have
one for this.)

On Tue, May 9, 2017 at 10​:02 AM, Eyal Itkin <eyal.itkin@​gmail.com> wrote​:

Thanks for the feedback. I saw the commit in the git and it seems to
solve
the reported vulnerability.

Please notify me when the commit will be merged to all relevant
branches,
so I would be able to proceed with the publication to the bug bounty
program of perl in hackerOne.

Thanks again for the fast reply.
Eyal

On May 9, 2017 10​:25 AM, "Dave Mitchell via RT" <
perl5-security-report@​perl.org> wrote​:

On Mon, May 08, 2017 at 06​:17​:27PM +0100, Dave Mitchell wrote​:

So in the short term, is this issue serious enough that my patch or
similar should be added to branches leading to the 5.26.0, 5.24.2 and
5.22.4 releases, or should it wait? This issue or similar appears to
have
been present since at least 5.10.0 and probably earlier.

With the pumpking's consent I've just committed v5.25.12-29-gbd1a29f to
blead, which has a basic fix for this issue. This is just before the
RC1
release is due.

That patch is also suitable for maint-5.22 and maint-5.24.

--
Technology is dominated by two types of people​: those who understand
what
they do not manage, and those who manage what they do not understand.

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented May 28, 2017

From eyal.itkin@gmail.com

OK, sounds legit.

Is there a green light to proceed with a disclosure to the Perl hackerOne
project? Are all branched were patched with the fix?

Take in notice that the average time for handling a ticket in hackerOne (in
IBB projects) is 2 months, and the ticket won't get publicly disclosed
without your confirmation.

Thanks again,
Eyal

On May 17, 2017 09​:53, "yves orton via RT" <perl5-security-report@​perl.org>
wrote​:

Personally I would say that the use of untrusted format strings is rare in
the perl community and even less likely in hostile environments. Thus it
probably doesn't merit a CVE but the core bug class does and if this use
case were more common we would want one.

So I guess we are somewhere in the middle of the Ruby and PHP guys on this
subject.

Yves

On 16 May 2017 11​:24, "Sawyer X" <xsawyerx@​gmail.com> wrote​:

If someone hasn't expressed an opinion that this requires a CVE by now, we
are not likely to assign one ourselves. If it did have one, we would have
included it in the change log.

Thanks. :)
On Tue, May 16, 2017 at 8​:08 AM, Eyal Itkin <eyal.itkin@​gmail.com> wrote​:

No, I didn't​ issue a CVE id for this vulnerability.

Issuing CVE ids to open source interpreter languages is always tricky, as
one cannot know when it will be "good enough". PHP issues CVEs even to
simple DoS vulnerabilities, while (C)Ruby will argue all the way that
"this
is just a simple bug" and won't agree to tag it as a "security" issue.

If you prefer that a CVE id will be assigned to this vulnerability, I can
contact MITRE to issue one.

On May 16, 2017 00​:12, "Sawyer X via RT" <perl5-security-report@​perl.org>
wrote​:

Hi Eyal,

Do you have a CVE number to go along with the disclosure?
(Not all security issues must have a CVE, I'm just wondering if you have
one for this.)

On Tue, May 9, 2017 at 10​:02 AM, Eyal Itkin <eyal.itkin@​gmail.com> wrote​:

Thanks for the feedback. I saw the commit in the git and it seems to
solve
the reported vulnerability.

Please notify me when the commit will be merged to all relevant
branches,
so I would be able to proceed with the publication to the bug bounty
program of perl in hackerOne.

Thanks again for the fast reply.
Eyal

On May 9, 2017 10​:25 AM, "Dave Mitchell via RT" <
perl5-security-report@​perl.org> wrote​:

On Mon, May 08, 2017 at 06​:17​:27PM +0100, Dave Mitchell wrote​:

So in the short term, is this issue serious enough that my patch or
similar should be added to branches leading to the 5.26.0, 5.24.2
and
5.22.4 releases, or should it wait? This issue or similar appears to
have
been present since at least 5.10.0 and probably earlier.

With the pumpking's consent I've just committed v5.25.12-29-gbd1a29f
to
blead, which has a basic fix for this issue. This is just before the
RC1
release is due.

That patch is also suitable for maint-5.22 and maint-5.24.

--
Technology is dominated by two types of people​: those who understand
what
they do not manage, and those who manage what they do not understand.

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jun 3, 2017

From @Leont

On Sun, May 28, 2017 at 5​:27 PM, Eyal Itkin <eyal.itkin@​gmail.com> wrote​:

OK, sounds legit.

Is there a green light to proceed with a disclosure to the Perl hackerOne
project? Are all branched were patched with the fix?

Take in notice that the average time for handling a ticket in hackerOne
(in IBB projects) is 2 months, and the ticket won't get publicly disclosed
without your confirmation.

Thanks again,
Eyal

It doesn't seem to have been backported to 5.24 and 5.22 yet.

Leon

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jun 3, 2017

From eyal.itkin@gmail.com

Thanks for the update.

Please notify me when the patch will be backported to all relevant versions.

On Jun 3, 2017 15​:58, "Leon Timmermans via RT" <
perl5-security-report@​perl.org> wrote​:

On Sun, May 28, 2017 at 5​:27 PM, Eyal Itkin <eyal.itkin@​gmail.com> wrote​:

OK, sounds legit.

Is there a green light to proceed with a disclosure to the Perl hackerOne
project? Are all branched were patched with the fix?

Take in notice that the average time for handling a ticket in hackerOne
(in IBB projects) is 2 months, and the ticket won't get publicly
disclosed
without your confirmation.

Thanks again,
Eyal

It doesn't seem to have been backported to 5.24 and 5.22 yet.

Leon

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jun 7, 2017

From @iabyn

On Sat, Jun 03, 2017 at 02​:57​:56PM +0200, Leon Timmermans wrote​:

It doesn't seem to have been backported to 5.24 and 5.22 yet.

Steve never replied to my query as to whether it was ok to backport it.

In the meantime, I've just merged my general reworking of
Perl_sv_vcatpvfn_flags into blead.

--
Diplomacy is telling someone to go to hell in such a way that they'll
look forward to the trip

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jun 7, 2017

From @xsawyerx

I think we should backport it. However, Steve is working on 5.24.2 and
5.22.4 which should be focused on the remaining base.pm fix, I believe.

On 7 June 2017 at 12​:23, Dave Mitchell <davem@​iabyn.com> wrote​:

On Sat, Jun 03, 2017 at 02​:57​:56PM +0200, Leon Timmermans wrote​:

It doesn't seem to have been backported to 5.24 and 5.22 yet.

Steve never replied to my query as to whether it was ok to backport it.

In the meantime, I've just merged my general reworking of
Perl_sv_vcatpvfn_flags into blead.

--
Diplomacy is telling someone to go to hell in such a way that they'll
look forward to the trip

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Jul 28, 2017

From @steve-m-hay

On Wed, 07 Jun 2017 03​:23​:36 -0700, davem wrote​:

On Sat, Jun 03, 2017 at 02​:57​:56PM +0200, Leon Timmermans wrote​:

It doesn't seem to have been backported to 5.24 and 5.22 yet.

Steve never replied to my query as to whether it was ok to backport it.

In the meantime, I've just merged my general reworking of
Perl_sv_vcatpvfn_flags into blead.

I apologize - I completely missed your question to me. It's probably because an email filter saw it was sent to 'perl5-security-report' and moved the message out of my Inbox to the relevant list folder... I'm afraid I don't read through every message on that (or any other) list. Please feel free to send email to me offlist to draw my attention to anything you fear I maye have missed -- as Karl has just done :-)

I don't know exactly where things are with this now, but if you still want to merge anything back to maint-5.2[46] then yes, go ahead as long as at least two other committers have approved. (I doubt another maint-5.22 will get made now.) If it's in blead and ripe for cherry-picking anyway then it can equally go in the voting file instead and I'll pick it up myself in due course.

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Aug 4, 2017

From eyal.itkin@gmail.com

Thanks for the update.

As steve issued a "green light" for back porting the fixes to 5.2[46], it
seems that the code fixes for the format string vulnerability can be closed
now in all relevant versions.

In addition, since the original report of the vulnerability was sent
roughly 3 months ago, it would be very appreciated if the fixes will be
integrated, and I'll get your approval to disclose the vulnerability to the
perl-ibb bounty program through hackerOne. The disclosure won't be a public
disclosure, however the Internet Bug Bounty mandates that the ticket will
be opened only after the project's security team will approve it.

Thanks again for your cooperation,
Eyal Itkin.

On Fri, Jul 28, 2017 at 8​:37 PM, Steve Hay via RT <
perl5-security-report@​perl.org> wrote​:

On Wed, 07 Jun 2017 03​:23​:36 -0700, davem wrote​:

On Sat, Jun 03, 2017 at 02​:57​:56PM +0200, Leon Timmermans wrote​:

It doesn't seem to have been backported to 5.24 and 5.22 yet.

Steve never replied to my query as to whether it was ok to backport it.

In the meantime, I've just merged my general reworking of
Perl_sv_vcatpvfn_flags into blead.

I apologize - I completely missed your question to me. It's probably
because an email filter saw it was sent to 'perl5-security-report' and
moved the message out of my Inbox to the relevant list folder... I'm afraid
I don't read through every message on that (or any other) list. Please feel
free to send email to me offlist to draw my attention to anything you fear
I maye have missed -- as Karl has just done :-)

I don't know exactly where things are with this now, but if you still want
to merge anything back to maint-5.2[46] then yes, go ahead as long as at
least two other committers have approved. (I doubt another maint-5.22 will
get made now.) If it's in blead and ripe for cherry-picking anyway then it
can equally go in the voting file instead and I'll pick it up myself in due
course.

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Aug 23, 2017

From @tonycoz

On Fri, 28 Jul 2017 10​:37​:34 -0700, shay wrote​:

On Wed, 07 Jun 2017 03​:23​:36 -0700, davem wrote​:

On Sat, Jun 03, 2017 at 02​:57​:56PM +0200, Leon Timmermans wrote​:

It doesn't seem to have been backported to 5.24 and 5.22 yet.

Steve never replied to my query as to whether it was ok to backport
it.

In the meantime, I've just merged my general reworking of
Perl_sv_vcatpvfn_flags into blead.

I apologize - I completely missed your question to me. It's probably
because an email filter saw it was sent to 'perl5-security-report' and
moved the message out of my Inbox to the relevant list folder... I'm
afraid I don't read through every message on that (or any other) list.
Please feel free to send email to me offlist to draw my attention to
anything you fear I maye have missed -- as Karl has just done :-)

I don't know exactly where things are with this now, but if you still
want to merge anything back to maint-5.2[46] then yes, go ahead as
long as at least two other committers have approved. (I doubt another
maint-5.22 will get made now.) If it's in blead and ripe for cherry-
picking anyway then it can equally go in the voting file instead and
I'll pick it up myself in due course.

I see this was backported to maint-5.26 as ddb03b7, but I don't see it in maint-5.24 nor maint-5.22 and I don't see the commits in the votes files.

Should it be?

Tony

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Aug 23, 2017

From eyal.itkin@gmail.com

I I didn't write the fix, nor did I committed it to 5.26. If you look at
the email conversation you could see all of the details regarding the fix.

The bottom line is that it was fixed in 5.26, and the fixes were NOT yet
merged to prior versions. Since the backporting was approved, it only takes
someone that will issue this backporting and will get it merged and
approved.

I do not have the matching permissions for it, and I really hope that
someone will merge it already as this issue is open for almost 4 months
now...

On Aug 23, 2017 09​:15, "Tony Cook via RT" <perl5-security-report@​perl.org>
wrote​:

On Fri, 28 Jul 2017 10​:37​:34 -0700, shay wrote​:

On Wed, 07 Jun 2017 03​:23​:36 -0700, davem wrote​:

On Sat, Jun 03, 2017 at 02​:57​:56PM +0200, Leon Timmermans wrote​:

It doesn't seem to have been backported to 5.24 and 5.22 yet.

Steve never replied to my query as to whether it was ok to backport
it.

In the meantime, I've just merged my general reworking of
Perl_sv_vcatpvfn_flags into blead.

I apologize - I completely missed your question to me. It's probably
because an email filter saw it was sent to 'perl5-security-report' and
moved the message out of my Inbox to the relevant list folder... I'm
afraid I don't read through every message on that (or any other) list.
Please feel free to send email to me offlist to draw my attention to
anything you fear I maye have missed -- as Karl has just done :-)

I don't know exactly where things are with this now, but if you still
want to merge anything back to maint-5.2[46] then yes, go ahead as
long as at least two other committers have approved. (I doubt another
maint-5.22 will get made now.) If it's in blead and ripe for cherry-
picking anyway then it can equally go in the voting file instead and
I'll pick it up myself in due course.

I see this was backported to maint-5.26 as ddb03b7,
but I don't see it in maint-5.24 nor maint-5.22 and I don't see the commits
in the votes files.

Should it be?

Tony

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Aug 23, 2017

From @steve-m-hay

Commit ddb03b7 wasn't a backport to
maint-5.26; it was committed before 5.26.0 was released, i.e. before
maint-5.26 was even created. I didn't realize this had happened, but I
will add it to the maint-5.24 voting file now since there is now a
commit to be voting for. (In general, I'm unsure how voting should
work on security tickets that aren't public yet...)

On 23 August 2017 at 09​:34, Eyal Itkin <eyal.itkin@​gmail.com> wrote​:

I I didn't write the fix, nor did I committed it to 5.26. If you look at the
email conversation you could see all of the details regarding the fix.

The bottom line is that it was fixed in 5.26, and the fixes were NOT yet
merged to prior versions. Since the backporting was approved, it only takes
someone that will issue this backporting and will get it merged and
approved.

I do not have the matching permissions for it, and I really hope that
someone will merge it already as this issue is open for almost 4 months
now...

On Aug 23, 2017 09​:15, "Tony Cook via RT" <perl5-security-report@​perl.org>
wrote​:

On Fri, 28 Jul 2017 10​:37​:34 -0700, shay wrote​:

On Wed, 07 Jun 2017 03​:23​:36 -0700, davem wrote​:

On Sat, Jun 03, 2017 at 02​:57​:56PM +0200, Leon Timmermans wrote​:

It doesn't seem to have been backported to 5.24 and 5.22 yet.

Steve never replied to my query as to whether it was ok to backport
it.

In the meantime, I've just merged my general reworking of
Perl_sv_vcatpvfn_flags into blead.

I apologize - I completely missed your question to me. It's probably
because an email filter saw it was sent to 'perl5-security-report' and
moved the message out of my Inbox to the relevant list folder... I'm
afraid I don't read through every message on that (or any other) list.
Please feel free to send email to me offlist to draw my attention to
anything you fear I maye have missed -- as Karl has just done :-)

I don't know exactly where things are with this now, but if you still
want to merge anything back to maint-5.2[46] then yes, go ahead as
long as at least two other committers have approved. (I doubt another
maint-5.22 will get made now.) If it's in blead and ripe for cherry-
picking anyway then it can equally go in the voting file instead and
I'll pick it up myself in due course.

I see this was backported to maint-5.26 as
ddb03b7, but I don't see it in maint-5.24
nor maint-5.22 and I don't see the commits in the votes files.

Should it be?

Tony

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Aug 23, 2017

From @xsawyerx

On 23 August 2017 at 15​:09, Steve Hay via perl5-security-report <
perl5-security-report@​perl.org> wrote​:

Commit ddb03b7 wasn't a backport to
maint-5.26; it was committed before 5.26.0 was released, i.e. before
maint-5.26 was even created. I didn't realize this had happened, but I
will add it to the maint-5.24 voting file now since there is now a
commit to be voting for. (In general, I'm unsure how voting should
work on security tickets that aren't public yet...)

I presume the security list needs to vote.

On 23 August 2017 at 09​:34, Eyal Itkin <eyal.itkin@​gmail.com> wrote​:

I I didn't write the fix, nor did I committed it to 5.26. If you look at
the
email conversation you could see all of the details regarding the fix.

The bottom line is that it was fixed in 5.26, and the fixes were NOT yet
merged to prior versions. Since the backporting was approved, it only
takes
someone that will issue this backporting and will get it merged and
approved.

I do not have the matching permissions for it, and I really hope that
someone will merge it already as this issue is open for almost 4 months
now...

On Aug 23, 2017 09​:15, "Tony Cook via RT" <perl5-security-report@​perl.
org>
wrote​:

On Fri, 28 Jul 2017 10​:37​:34 -0700, shay wrote​:

On Wed, 07 Jun 2017 03​:23​:36 -0700, davem wrote​:

On Sat, Jun 03, 2017 at 02​:57​:56PM +0200, Leon Timmermans wrote​:

It doesn't seem to have been backported to 5.24 and 5.22 yet.

Steve never replied to my query as to whether it was ok to backport
it.

In the meantime, I've just merged my general reworking of
Perl_sv_vcatpvfn_flags into blead.

I apologize - I completely missed your question to me. It's probably
because an email filter saw it was sent to 'perl5-security-report' and
moved the message out of my Inbox to the relevant list folder... I'm
afraid I don't read through every message on that (or any other) list.
Please feel free to send email to me offlist to draw my attention to
anything you fear I maye have missed -- as Karl has just done :-)

I don't know exactly where things are with this now, but if you still
want to merge anything back to maint-5.2[46] then yes, go ahead as
long as at least two other committers have approved. (I doubt another
maint-5.22 will get made now.) If it's in blead and ripe for cherry-
picking anyway then it can equally go in the voting file instead and
I'll pick it up myself in due course.

I see this was backported to maint-5.26 as
ddb03b7, but I don't see it in
maint-5.24
nor maint-5.22 and I don't see the commits in the votes files.

Should it be?

Tony

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Aug 23, 2017

From @tonycoz

On Wed, Aug 23, 2017 at 03​:12​:21PM +0200, Sawyer X wrote​:

On 23 August 2017 at 15​:09, Steve Hay via perl5-security-report <
perl5-security-report@​perl.org> wrote​:

Commit ddb03b7 wasn't a backport to
maint-5.26; it was committed before 5.26.0 was released, i.e. before
maint-5.26 was even created. I didn't realize this had happened, but I
will add it to the maint-5.24 voting file now since there is now a
commit to be voting for. (In general, I'm unsure how voting should
work on security tickets that aren't public yet...)

I presume the security list needs to vote.

The commit is public, so we can vote on it.

Since the commit is public, we could probably make this ticket public,
but it might be nice to hold off on that until it's in a 5.24.x
release.

We can then both make it public and resolve it.

Tony

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Aug 23, 2017

From @steve-m-hay

On 23 August 2017 at 14​:52, Tony Cook <tony@​develop-help.com> wrote​:

On Wed, Aug 23, 2017 at 03​:12​:21PM +0200, Sawyer X wrote​:

On 23 August 2017 at 15​:09, Steve Hay via perl5-security-report <
perl5-security-report@​perl.org> wrote​:

Commit ddb03b7 wasn't a backport to
maint-5.26; it was committed before 5.26.0 was released, i.e. before
maint-5.26 was even created. I didn't realize this had happened, but I
will add it to the maint-5.24 voting file now since there is now a
commit to be voting for. (In general, I'm unsure how voting should
work on security tickets that aren't public yet...)

I presume the security list needs to vote.

The commit is public, so we can vote on it.

Since the commit is public, we could probably make this ticket public,
but it might be nice to hold off on that until it's in a 5.24.x
release.

We can then both make it public and resolve it.

I've now added the commit to the 5.24 voting file.

Note that wIth 5.26.1 due imminently (though other security fixes are
possibly needed for it too), it's likely that 5.24.3 won't be released
until late September / early October.

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Aug 30, 2017

From @steve-m-hay

On Wed, 23 Aug 2017 14​:06​:02 -0700, shay wrote​:

On 23 August 2017 at 14​:52, Tony Cook <tony@​develop-help.com> wrote​:

On Wed, Aug 23, 2017 at 03​:12​:21PM +0200, Sawyer X wrote​:

On 23 August 2017 at 15​:09, Steve Hay via perl5-security-report <
perl5-security-report@​perl.org> wrote​:

Commit ddb03b7 wasn't a backport to
maint-5.26; it was committed before 5.26.0 was released, i.e. before
maint-5.26 was even created. I didn't realize this had happened, but I
will add it to the maint-5.24 voting file now since there is now a
commit to be voting for. (In general, I'm unsure how voting should
work on security tickets that aren't public yet...)

I presume the security list needs to vote.

The commit is public, so we can vote on it.

Since the commit is public, we could probably make this ticket public,
but it might be nice to hold off on that until it's in a 5.24.x
release.

We can then both make it public and resolve it.

I've now added the commit to the 5.24 voting file.

Note that wIth 5.26.1 due imminently (though other security fixes are
possibly needed for it too), it's likely that 5.24.3 won't be released
until late September / early October.

Update​: This commit is now in the maint-5.24 branch, so we're just waiting for a 5.24.3 release now - hopefully around a month or so from now.

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Nov 29, 2017

From @iabyn

On Fri, Aug 04, 2017 at 02​:41​:22PM +0300, Eyal Itkin wrote​:

Thanks for the update.

As steve issued a "green light" for back porting the fixes to 5.2[46], it
seems that the code fixes for the format string vulnerability can be closed
now in all relevant versions.

As far as I'm aware, the fix has now been backported and released in all
maintenence versions of perl.

In addition, since the original report of the vulnerability was sent
roughly 3 months ago, it would be very appreciated if the fixes will be
integrated, and I'll get your approval to disclose the vulnerability to the
perl-ibb bounty program through hackerOne. The disclosure won't be a public
disclosure, however the Internet Bug Bounty mandates that the ticket will
be opened only after the project's security team will approve it.

I am happy for the bug to be disclosed to perl-ibb.

I'll also shortly mark this ticket as resolved and move it to the public
queue.

--
"I do not resent criticism, even when, for the sake of emphasis,
it parts for the time with reality".
  -- Winston Churchill, House of Commons, 22nd Jan 1941.

@p5pRT

This comment has been minimized.

Copy link
Collaborator Author

@p5pRT p5pRT commented Apr 9, 2018

@iabyn - Status changed from 'open' to 'resolved'

@p5pRT p5pRT closed this Apr 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant
You can’t perform that action at this time.