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
Comments
From eyal.itkin@gmail.comHello, In a security audit to the sprintf implementation in perl (version 5.24.1) *Technical Details:* - precis - represents a format's precision, and can be any size_t value *PoC Script:* print sprintf("%2000.2000f this is a spacer %4000.4294967245a", 1, *Crash trace - tested on a 32 bit linux machine:* Program terminated with signal SIGSEGV, Segmentation fault. As you can see, the first %f print enlarged the buffer, and the *Threat Analysis:* This report demonstrates a potential code execution vulnerability, in case - "Generic" logging \ monitoring module that receives format + args, and Such kind of vulnerabilities are still common in modern code projects, and *Conclusion:* In case there are any questions regarding the vulnerability or the report, |
From @iabynOn Fri, May 05, 2017 at 03:01:51PM -0700, Eyal Itkin wrote:
The same vulnerability persists in blead. Here is my provisional analysis. This function, Perl_sv_vcatpvfn_flags, is perl's lowest-level sprintf-ish There appear to be three places within that function where memory is The main SvGROW() allocates extra space in the output string to append the A second SvGROW() is used when appending a bad format element as-is, e.g. Finally, a Newx(PL_efloatbuf, PL_efloatsize, char) is done for Given the complexity of this function, it's possible that the description When dealing with floating-point formats like e/f/g/a, perl allocates a As the OP points out, there are a number of calculations done to determine This means that part of the f/p string will be written beyond the end of As the OP demonstrated, it's possible to overrun a PL_op->op_ppaddr field So this is bad, but the question is how bad. Obviously, any badly-written logging code which accepts a hostile sprintf But with a buggy perl implementation, this could cause an existing DoS My gut feeling is that given the limited "palette" of byte values that I will have a patch ready shortly for the specific PL_efloatbuf issues. So in the short term, is this issue serious enough that my patch or -- |
The RT System itself - Status changed from 'new' to 'open' |
From @iabynOn Mon, May 08, 2017 at 06:17:27PM +0100, Dave Mitchell wrote:
With the pumpking's consent I've just committed v5.25.12-29-gbd1a29f to That patch is also suitable for maint-5.22 and maint-5.24. -- |
From eyal.itkin@gmail.comThanks for the feedback. I saw the commit in the git and it seems to solve Please notify me when the commit will be merged to all relevant branches, Thanks again for the fast reply. On May 9, 2017 10:25 AM, "Dave Mitchell via RT" <perl5-security-report@perl.
|
From @iabynOn Tue, May 09, 2017 at 08:24:51AM +0100, Dave Mitchell wrote:
Steve, would it be ok for me to cherry-pick these into maint-5.22 and To everyone else - I'm currently auditing/hardening the code in -- |
From @khwilliamsonOn 05/15/2017 04:15 AM, Dave Mitchell wrote:
I'm trying to grok what '5.16.09' means here, without success. |
From @iabynOn Mon, May 15, 2017 at 09:39:03AM -0600, Karl Williamson wrote:
That's an astoundingly badly typed 5.26.0 -- |
From @xsawyerxHi Eyal, Do you have a CVE number to go along with the disclosure? On Tue, May 9, 2017 at 10:02 AM, Eyal Itkin <eyal.itkin@gmail.com> wrote:
|
From eyal.itkin@gmail.comNo, I didn't issue a CVE id for this vulnerability. Issuing CVE ids to open source interpreter languages is always tricky, as If you prefer that a CVE id will be assigned to this vulnerability, I can On May 16, 2017 00:12, "Sawyer X via RT" <perl5-security-report@perl.org>
|
From @xsawyerxIf someone hasn't expressed an opinion that this requires a CVE by now, we Thanks. :) On Tue, May 16, 2017 at 8:08 AM, Eyal Itkin <eyal.itkin@gmail.com> wrote:
|
From @demerphqPersonally I would say that the use of untrusted format strings is rare in So I guess we are somewhere in the middle of the Ruby and PHP guys on this 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 Thanks. :)
|
From eyal.itkin@gmail.comOK, sounds legit. Is there a green light to proceed with a disclosure to the Perl hackerOne Take in notice that the average time for handling a ticket in hackerOne (in Thanks again, On May 17, 2017 09:53, "yves orton via RT" <perl5-security-report@perl.org> Personally I would say that the use of untrusted format strings is rare in So I guess we are somewhere in the middle of the Ruby and PHP guys on this 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 Thanks. :)
|
From @LeontOn Sun, May 28, 2017 at 5:27 PM, Eyal Itkin <eyal.itkin@gmail.com> wrote:
It doesn't seem to have been backported to 5.24 and 5.22 yet. Leon |
From eyal.itkin@gmail.comThanks 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" <
|
From @iabynOn Sat, Jun 03, 2017 at 02:57:56PM +0200, Leon Timmermans wrote:
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 -- |
From @xsawyerxI think we should backport it. However, Steve is working on 5.24.2 and On 7 June 2017 at 12:23, Dave Mitchell <davem@iabyn.com> wrote:
|
From @steve-m-hayOn Wed, 07 Jun 2017 03:23:36 -0700, davem wrote:
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. |
From eyal.itkin@gmail.comThanks for the update. As steve issued a "green light" for back porting the fixes to 5.2[46], it In addition, since the original report of the vulnerability was sent Thanks again for your cooperation, On Fri, Jul 28, 2017 at 8:37 PM, Steve Hay via RT <
|
From @tonycozOn Fri, 28 Jul 2017 10:37:34 -0700, shay wrote:
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 |
From eyal.itkin@gmail.comI I didn't write the fix, nor did I committed it to 5.26. If you look at The bottom line is that it was fixed in 5.26, and the fixes were NOT yet I do not have the matching permissions for it, and I really hope that On Aug 23, 2017 09:15, "Tony Cook via RT" <perl5-security-report@perl.org>
|
From @steve-m-hayCommit ddb03b7 wasn't a backport to On 23 August 2017 at 09:34, Eyal Itkin <eyal.itkin@gmail.com> wrote:
|
From @xsawyerxOn 23 August 2017 at 15:09, Steve Hay via perl5-security-report <
I presume the security list needs to vote.
|
From @tonycozOn Wed, Aug 23, 2017 at 03:12:21PM +0200, Sawyer X wrote:
The commit is public, so we can vote on it. Since the commit is public, we could probably make this ticket public, We can then both make it public and resolve it. Tony |
From @steve-m-hayOn 23 August 2017 at 14:52, Tony Cook <tony@develop-help.com> wrote:
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 |
From @steve-m-hayOn Wed, 23 Aug 2017 14:06:02 -0700, shay wrote:
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. |
From @iabynOn Fri, Aug 04, 2017 at 02:41:22PM +0300, Eyal Itkin wrote:
As far as I'm aware, the fix has now been backported and released in all
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 -- |
@iabyn - Status changed from 'open' to 'resolved' |
Migrated from rt.perl.org#131260 (status was 'resolved')
Searchable as RT131260$
The text was updated successfully, but these errors were encountered: