Skip Menu |
Report information
Id: 133204
Status: open
Priority: 0/
Queue: perl5

Owner: Nobody
Requestors: jmenon [at] isi.edu
Cc:
AdminCc:

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



To: "perl5-security-report [...] perl.org" <perl5-security-report [...] perl.org>
Date: Wed, 16 May 2018 00:58:13 +0000
Subject: Integer overflow leading to buffer overflow
CC: Christopher Hauser <hauser [...] isi.edu>
From: Jayakrishna Menon <jmenon [...] isi.edu>
Download (untitled) / with headers
text/plain 1.1k

Hi,


As a part of an academic project, we have discovered an integer overflow in Perl which subsequently leads to a heap overflow.


The vulnerability is present in Perl_my_setenv @ util.c : 2070


2070: void Perl_my_setenv(pTHX_ const char *nam, const char *val) {

...

2166:         const int nlen = strlen(nam);

...

2171:         vlen = strlen(val);

2172:         new_env = (char*)safesysmalloc((nlen + vlen + 2) * sizeof(char));


Here, since the arguments nam and val are user controlled, the 32 bit integers nlen and vlen are also under the control of the attacker. Therefore, if nam and val are two very long strings, the addition at 2172 would result in an integer overflow.


The new_env would therefore be a chunk of a size which is smaller than the sum of the lengths of the two input strings.


This new_env is subsequently used in a call to memcpy to copy nlen bytes from nam and vlen bytes from val.

This results in a buffer overflow on the heap with attacker controlled input.


We have attached a perl script that demonstrates this vulnerability.


Regards
Jayakrishna Menon and Christophe Hauser
Information Sciences Institute
University of Southern California

Download test.pl
text/x-perl 44b

Message body is not shown because sender requested not to inline it.

Subject: Re: [perl #133204] Integer overflow leading to buffer overflow
From: Dave Mitchell <davem [...] iabyn.com>
Date: Wed, 16 May 2018 12:22:40 +0100
To: perl5-security-report [...] perl.org
Download (untitled) / with headers
text/plain 1.5k
On Tue, May 15, 2018 at 05:59:26PM -0700, Jayakrishna Menon wrote: Show quoted text
> The vulnerability is present in Perl_my_setenv @ util.c : 2070 > > > 2070: void Perl_my_setenv(pTHX_ const char *nam, const char *val) { > > ... > > 2166: const int nlen = strlen(nam); > > ... > > 2171: vlen = strlen(val); > > 2172: new_env = (char*)safesysmalloc((nlen + vlen + 2) * sizeof(char)); > > > Here, since the arguments nam and val are user controlled, the 32 bit > integers nlen and vlen are also under the control of the attacker. > Therefore, if nam and val are two very long strings, the addition at > 2172 would result in an integer overflow. > > The new_env would therefore be a chunk of a size which is smaller than > the sum of the lengths of the two input strings.
Show quoted text
> $inp = 'A' x 0x7fffffff; > $ENV{$inp} = $inp;
I don't think this has realistic security implications. I think it would be unlikely to find perl code in the wild which allowed an attacker to control both the key and value of a %ENV setting to the extent that both could be made around 2^31 bytes in length; and if such code did exist, it would probably already be exploitable if it allowed hackers to set or modify chosen env vars (e.g. PATH, LD_PRELOAD). The fix would appear to be to generally replace ints and I32s in Perl_my_setenv with Size_t's or similar. -- Spock (or Data) is fired from his high-ranking position for not being able to understand the most basic nuances of about one in three sentences that anyone says to him. -- Things That Never Happen in "Star Trek" #19
Subject: Re: [perl #133204] Integer overflow leading to buffer overflow
From: Jayakrishna Menon <jmenon [...] isi.edu>
To: "perl5-security-report-followup [...] perl.org" <perl5-security-report-followup [...] perl.org>
Date: Wed, 16 May 2018 16:31:02 +0000
Download (untitled) / with headers
text/plain 1.9k

Just to clarify,


The attacker doesn't need to control both the key and value of a %ENV setting to trigger this bug.

If the attacker can create a value of 0xFFFFFFFF bytes in length, this bug would be triggered.


From: Dave Mitchell via RT <perl5-security-report-followup@perl.org>
Sent: Wednesday, May 16, 2018 4:22:53 AM
To: Jayakrishna Menon
Subject: Re: [perl #133204] Integer overflow leading to buffer overflow
 
On Tue, May 15, 2018 at 05:59:26PM -0700, Jayakrishna Menon wrote:
> The vulnerability is present in Perl_my_setenv @ util.c : 2070
>
>
> 2070: void Perl_my_setenv(pTHX_ const char *nam, const char *val) {
>
> ...
>
> 2166:         const int nlen = strlen(nam);
>
> ...
>
> 2171:         vlen = strlen(val);
>
> 2172:         new_env = (char*)safesysmalloc((nlen + vlen + 2) * sizeof(char));
>
>
> Here, since the arguments nam and val are user controlled, the 32 bit
> integers nlen and vlen are also under the control of the attacker.
> Therefore, if nam and val are two very long strings, the addition at
> 2172 would result in an integer overflow.
>
> The new_env would therefore be a chunk of a size which is smaller than
> the sum of the lengths of the two input strings.

> $inp = 'A' x 0x7fffffff;
> $ENV{$inp} = $inp;

I don't think this has realistic security implications. I think it would
be unlikely to find perl code in the wild which allowed an attacker
to control both the key and value of a %ENV setting to the extent that
both could be made around 2^31 bytes in length; and if such code did
exist, it would probably already be exploitable if it allowed hackers to
set or modify chosen env vars (e.g. PATH, LD_PRELOAD).

The fix would appear to be to generally replace ints and I32s in
Perl_my_setenv with Size_t's or similar.


--
Spock (or Data) is fired from his high-ranking position for not being able
to understand the most basic nuances of about one in three sentences that
anyone says to him.
    -- Things That Never Happen in "Star Trek" #19

Date: Thu, 28 Jun 2018 22:54:54 +0000
To: "perl5-security-report-followup [...] perl.org" <perl5-security-report-followup [...] perl.org>
Subject: Re: [perl #133204] Integer overflow leading to buffer overflow
From: Jayakrishna Menon <jmenon [...] isi.edu>
Download (untitled) / with headers
text/plain 2.5k

It has been more than 30 days since I have received any response regarding this issue.

I had also made a pull request to patch this vulnerability[0].


However, that hasn't been merged even after 24 days.


If there are some changes required in the patch I proposed in the pull request, please let me know and I will make the changes accordingly.


[0] https://github.com/Perl/perl5/pull/13


Regards

Jayakrishna Menon


From: Jayakrishna Menon
Sent: Wednesday, May 16, 2018 9:31:02 AM
To: perl5-security-report-followup@perl.org
Subject: Re: [perl #133204] Integer overflow leading to buffer overflow
 

Just to clarify,


The attacker doesn't need to control both the key and value of a %ENV setting to trigger this bug.

If the attacker can create a value of 0xFFFFFFFF bytes in length, this bug would be triggered.


From: Dave Mitchell via RT <perl5-security-report-followup@perl.org>
Sent: Wednesday, May 16, 2018 4:22:53 AM
To: Jayakrishna Menon
Subject: Re: [perl #133204] Integer overflow leading to buffer overflow
 
On Tue, May 15, 2018 at 05:59:26PM -0700, Jayakrishna Menon wrote:
> The vulnerability is present in Perl_my_setenv @ util.c : 2070
>
>
> 2070: void Perl_my_setenv(pTHX_ const char *nam, const char *val) {
>
> ...
>
> 2166:         const int nlen = strlen(nam);
>
> ...
>
> 2171:         vlen = strlen(val);
>
> 2172:         new_env = (char*)safesysmalloc((nlen + vlen + 2) * sizeof(char));
>
>
> Here, since the arguments nam and val are user controlled, the 32 bit
> integers nlen and vlen are also under the control of the attacker.
> Therefore, if nam and val are two very long strings, the addition at
> 2172 would result in an integer overflow.
>
> The new_env would therefore be a chunk of a size which is smaller than
> the sum of the lengths of the two input strings.

> $inp = 'A' x 0x7fffffff;
> $ENV{$inp} = $inp;

I don't think this has realistic security implications. I think it would
be unlikely to find perl code in the wild which allowed an attacker
to control both the key and value of a %ENV setting to the extent that
both could be made around 2^31 bytes in length; and if such code did
exist, it would probably already be exploitable if it allowed hackers to
set or modify chosen env vars (e.g. PATH, LD_PRELOAD).

The fix would appear to be to generally replace ints and I32s in
Perl_my_setenv with Size_t's or similar.


--
Spock (or Data) is fired from his high-ranking position for not being able
to understand the most basic nuances of about one in three sentences that
anyone says to him.
    -- Things That Never Happen in "Star Trek" #19

Date: Fri, 29 Jun 2018 14:41:28 +0100
Subject: Re: [perl #133204] Integer overflow leading to buffer overflow
CC: "perl5-security-report-followup [...] perl.org" <perl5-security-report-followup [...] perl.org>
From: Dave Mitchell <davem [...] iabyn.com>
To: Jayakrishna Menon <jmenon [...] isi.edu>
Download (untitled) / with headers
text/plain 1.5k
On Thu, Jun 28, 2018 at 10:54:54PM +0000, Jayakrishna Menon wrote: Show quoted text
> It has been more than 30 days since I have received any response regarding this issue. > > I had also made a pull request to patch this vulnerability[0]. > > > However, that hasn't been merged even after 24 days.
Note that the github repo is just a read-only mirror of the real perl repository; we don't accept push requests from it. Show quoted text
> If there are some changes required in the patch I proposed in the pull > request, please let me know and I will make the changes accordingly.
I've just pushed the following, which is similar to your commit, but goes further. commit 34716e2a6ee2af96078d62b065b7785c001194be Author: David Mitchell <davem@iabyn.com> AuthorDate: Fri Jun 29 13:37:03 2018 +0100 Commit: David Mitchell <davem@iabyn.com> CommitDate: Fri Jun 29 14:03:57 2018 +0100 Perl_my_setenv(); handle integer wrap RT #133204 Wean this function off int/I32 and onto UV/Size_t. Also, replace all malloc-ish calls with a wrapper that does overflow checks, In particular, it was doing (nlen + vlen + 2) which could wrap when the combined length of the environment variable name and value exceeded around 0x7fffffff. The wrapper check function is probably overkill, but belt and braces... NB this function has several variant parts, #ifdef'ed by platform type; I have blindly changed the parts that aren't compiled under linux. M util.c -- You live and learn (although usually you just live).
RT-Send-CC: perl5-security-report [...] perl.org
Download (untitled) / with headers
text/plain 3.8k
On Fri, 29 Jun 2018 06:41:54 -0700, davem wrote: Show quoted text
> On Thu, Jun 28, 2018 at 10:54:54PM +0000, Jayakrishna Menon wrote:
> > It has been more than 30 days since I have received any response > > regarding this issue. > > > > I had also made a pull request to patch this vulnerability[0]. > > > > > > However, that hasn't been merged even after 24 days.
> > Note that the github repo is just a read-only mirror of the real perl > repository; we don't accept push requests from it. >
> > If there are some changes required in the patch I proposed in the > > pull > > request, please let me know and I will make the changes accordingly.
> > I've just pushed the following, which is similar to your commit, > but goes further. > > commit 34716e2a6ee2af96078d62b065b7785c001194be > Author: David Mitchell <davem@iabyn.com> > AuthorDate: Fri Jun 29 13:37:03 2018 +0100 > Commit: David Mitchell <davem@iabyn.com> > CommitDate: Fri Jun 29 14:03:57 2018 +0100 > > Perl_my_setenv(); handle integer wrap > > RT #133204 > > Wean this function off int/I32 and onto UV/Size_t. > Also, replace all malloc-ish calls with a wrapper that does > overflow checks, > > In particular, it was doing (nlen + vlen + 2) which could wrap when > the combined length of the environment variable name and value > exceeded around 0x7fffffff. > > The wrapper check function is probably overkill, but belt and > braces... > > NB this function has several variant parts, #ifdef'ed by platform > type; I have blindly changed the parts that aren't compiled under > linux. > > M util.c
+ sl = l * size; + if (sl < l) + goto panic; In the general case where size > 1 its possible for this multiply to overflow but still have sl > l. A safer check is something like: if (MEM_SIZE_MAX / size < l) goto panic; Given that S_env_alloc() is only called with size != 1 when incrementally increasing the length of environ, this can't be practically attacked (environ would need to occupy a significant part of the address space), but might be a problem if the same code is re-used elsewhere or we have similar checks elsewhere. _MEM_WRAP_WILL_WRAP() does appear to do the right thing. None of the other code that calls croak_memory_wrap() does multiplication checks, so don't include this flaw. As to whether this is a security issue... I don't think it's possible to cause the overflow on a 32-bit system, since you'd need enough free address space to store a 2GB string, which is unlikely. It is (obviously) possible on a 64-bit system. If an attacker has control over the contents of the value that a perl program stores into a environment variable, and that perl program allows the value + name to be over 2GB, they can write beyond the end of the allocated buffer with attacker supplied data. This is a serious security issue. Writing beyond the end of an allocated buffer can be turned into code execution attacks, depending on the platform. Mitigations: Allowing an untrusted user to store ~2GB of data seems generally unsafe to me, and a program that uses that for example to supply data to a child process is going to run into system limits on common platforms much earlier[1][2]. Countering that, the program might be running on a platform without such limits, and have no need for portability. It also comes from someone[3] who grew up with fairly small memory systems, modern code should expect to work with larger amounts of data. If a perl user, mistaken or not decides to store a large amount of data in an environment variable we should either succeed or fail due to system limits, not write beyond the end of a buffer. So at this point I think this is a security issue[4]. Tony [1] https://blogs.msdn.microsoft.com/oldnewthing/20100203-00/?p=15083 [2] https://unix.stackexchange.com/questions/336934/raise-128kib-limit-on-environment-variables-in-linux [3] Me. [4] As painful as that is.
To: "perl5-security-report-followup [...] perl.org" <perl5-security-report-followup [...] perl.org>
Date: Fri, 3 Aug 2018 03:02:11 +0000
Subject: Re: [perl #133204] Integer overflow leading to buffer overflow
From: Jayakrishna Menon <jmenon [...] isi.edu>
Download (untitled) / with headers
text/plain 4.3k

Thank you for your attention.


I am glad that you spared some time to investigate this issue.

As you said, I also feel that this is indeed a serious security issue which is why I reported it.


Hence, I would like to know if I could request a CVE-ID for this bug and if so, the procedures to obtain it.


Regards

Jayakrishna Menon


From: Tony Cook via RT <perl5-security-report-followup@perl.org>
Sent: Wednesday, August 1, 2018 7:14:32 PM
To: Jayakrishna Menon
Subject: [perl #133204] Integer overflow leading to buffer overflow
 
On Fri, 29 Jun 2018 06:41:54 -0700, davem wrote:
> On Thu, Jun 28, 2018 at 10:54:54PM +0000, Jayakrishna Menon wrote:
> > It has been more than 30 days since I have received any response
> > regarding this issue.
> >
> > I had also made a pull request to patch this vulnerability[0].
> >
> >
> > However, that hasn't been merged even after 24 days.
>
> Note that the github repo is just a read-only mirror of the real perl
> repository; we don't accept push requests from it.
>
> > If there are some changes required in the patch I proposed in the
> > pull
> > request, please let me know and I will make the changes accordingly.
>
> I've just pushed the following, which is similar to your commit,
> but goes further.
>
> commit 34716e2a6ee2af96078d62b065b7785c001194be
> Author:     David Mitchell <davem@iabyn.com>
> AuthorDate: Fri Jun 29 13:37:03 2018 +0100
> Commit:     David Mitchell <davem@iabyn.com>
> CommitDate: Fri Jun 29 14:03:57 2018 +0100
>
> Perl_my_setenv(); handle integer wrap
>
> RT #133204
>
> Wean this function off int/I32 and onto UV/Size_t.
> Also, replace all malloc-ish calls with a wrapper that does
> overflow checks,
>
> In particular, it was doing (nlen + vlen + 2) which could wrap when
> the combined length of the environment variable name and value
> exceeded around 0x7fffffff.
>
> The wrapper check function is probably overkill, but belt and
> braces...
>
> NB this function has several variant parts, #ifdef'ed by platform
> type; I have blindly changed the parts that aren't compiled under
> linux.
>
> M       util.c

+    sl = l * size;
+    if (sl < l)
+        goto panic;

In the general case where size > 1 its possible for this multiply to overflow but still have sl > l.

A safer check is something like:

  if (MEM_SIZE_MAX / size < l)
      goto panic;

Given that S_env_alloc() is only called with size != 1 when incrementally increasing the length of environ, this can't be practically attacked (environ would need to occupy a significant part of the address space), but might be a problem if the same code is re-used elsewhere or we have similar checks elsewhere.

_MEM_WRAP_WILL_WRAP() does appear to do the right thing.  None of the other code that calls croak_memory_wrap() does multiplication checks, so don't include this flaw.

As to whether this is a security issue...

I don't think it's possible to cause the overflow on a 32-bit system, since you'd need enough free address space to store a 2GB string, which is unlikely.

It is (obviously) possible on a 64-bit system.

If an attacker has control over the contents of the value that a perl program stores into a environment variable, and that perl program allows the value + name to be over 2GB, they can write beyond the end of the allocated buffer with attacker supplied data.

This is a serious security issue.

Writing beyond the end of an allocated buffer can be turned into code execution attacks, depending on the platform.

Mitigations:

Allowing an untrusted user to store ~2GB of data seems generally unsafe to me, and a program that uses that for example to supply data to a child process is going to run into system limits on common platforms much earlier[1][2].  Countering that, the program might be running on a platform without such limits, and have no need for portability.

It also comes from someone[3] who grew up with fairly small memory systems, modern code should expect to work with larger amounts of data.

If a perl user, mistaken or not decides to store a large amount of data in an environment variable we should either succeed or fail due to system limits, not write beyond the end of a buffer.

So at this point I think this is a security issue[4].

Tony

[1] https://blogs.msdn.microsoft.com/oldnewthing/20100203-00/?p=15083
[2] https://unix.stackexchange.com/questions/336934/raise-128kib-limit-on-environment-variables-in-linux
[3] Me.
[4] As painful as that is.
RT-Send-CC: perl5-security-report [...] perl.org
Download (untitled) / with headers
text/plain 561b
On Thu, 02 Aug 2018 20:02:48 -0700, jmenon@isi.edu wrote: Show quoted text
> Thank you for your attention. > > > I am glad that you spared some time to investigate this issue. > > As you said, I also feel that this is indeed a serious security issue > which is why I reported it. > > > Hence, I would like to know if I could request a CVE-ID for this bug > and if so, the procedures to obtain it.
I'd like to wait a few more days to see if anyone else has an opinion. I'm not the only one whose opinion counts here. Sorry about the extended delay in handling this. Tony
RT-Send-CC: perl5-security-report [...] perl.org
Download (untitled) / with headers
text/plain 139b
I plan to request a CVE ID for this issue in the next couple of days. If you've already requested an ID, please let me know. Thanks, Tony
Date: Mon, 24 Sep 2018 18:13:10 +0000
Subject: Re: [perl #133204] Integer overflow leading to buffer overflow
To: "perl5-security-report-followup [...] perl.org" <perl5-security-report-followup [...] perl.org>
From: Jayakrishna Menon <jmenon [...] isi.edu>
Download (untitled) / with headers
text/plain 482b

Thank you for the update.


And I have not requested a CVE ID for this issue.


Regards

Jayakrishna Menon


From: Tony Cook via RT <perl5-security-report-followup@perl.org>
Sent: Sunday, September 23, 2018 11:40:01 PM
To: Jayakrishna Menon
Subject: [perl #133204] Integer overflow leading to buffer overflow
 
I plan to request a CVE ID for this issue in the next couple of days.

If you've already requested an ID, please let me know.

Thanks,
Tony
Subject: Re: [perl #133204] Integer overflow leading to buffer overflow
Date: Mon, 24 Sep 2018 18:22:37 +0000
To: "perl5-security-report-followup [...] perl.org" <perl5-security-report-followup [...] perl.org>
From: Jayakrishna Menon <jmenon [...] isi.edu>
Download (untitled) / with headers
text/plain 477b

Thank you for the update


I have not requested a CVE ID for this issue.


Regards

Jayakrishna Menon


From: Tony Cook via RT <perl5-security-report-followup@perl.org>
Sent: Sunday, September 23, 2018 11:40:01 PM
To: Jayakrishna Menon
Subject: [perl #133204] Integer overflow leading to buffer overflow
 
I plan to request a CVE ID for this issue in the next couple of days.

If you've already requested an ID, please let me know.

Thanks,
Tony
RT-Send-CC: perl5-security-report [...] perl.org
Download (untitled) / with headers
text/plain 211b
On Sun, 23 Sep 2018 23:40:01 -0700, tonyc wrote: Show quoted text
> I plan to request a CVE ID for this issue in the next couple of days. > > If you've already requested an ID, please let me know.
This is CVE-2018-18311. Tony
To: "perl5-security-report-followup [...] perl.org" <perl5-security-report-followup [...] perl.org>
Date: Mon, 15 Oct 2018 22:48:10 +0000
Subject: Re: [perl #133204] [CVE-2018-18311] Integer overflow leading to buffer overflow
From: Jayakrishna Menon <jmenon [...] isi.edu>
CC: Christopher Hauser <hauser [...] isi.edu>
Download (untitled) / with headers
text/plain 517b

Thank you so much for that.


Regards

Jayakrishna Menon


From: Tony Cook via RT <perl5-security-report-followup@perl.org>
Sent: Monday, October 15, 2018 3:38:35 PM
To: Jayakrishna Menon
Subject: [perl #133204] [CVE-2018-18311] Integer overflow leading to buffer overflow
 
On Sun, 23 Sep 2018 23:40:01 -0700, tonyc wrote:
> I plan to request a CVE ID for this issue in the next couple of days.
>
> If you've already requested an ID, please let me know.

This is CVE-2018-18311.

Tony
RT-Send-CC: perl5-porters [...] perl.org
Moved to public queue with the release of 5.26.3 and 5.28.1.
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 524b
On Thu, 29 Nov 2018 11:14:29 -0800, shay wrote: Show quoted text
> Moved to public queue with the release of 5.26.3 and 5.28.1.
Who has write access to the github repository? This pull request https://github.com/Perl/perl5/pull/13 should be closed. (Orthogonally, we should put some instructions up on github informing people that pull requests are not merged directly, but if they accompany their patch with a submission to the RT queue, the porters will be suitably informed of the patch's existence and can guide the submitter further.)


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