perltidy: insecure permissions of temporary files

Debian Bug report logs - #740670
perltidy: insecure permissions of temporary files

Toggle useless messages

Message #3 received at submit@bugs.debian.org (full text, mbox, reply):

From: Jakub Wilk <jwilk@debian.org>

To: Debian Bug Tracking System <submit@bugs.debian.org>

Subject: perltidy: insecure permissions of temporary files

Date: Mon, 3 Mar 2014 23:27:09 +0100

Package: perltidy
Version: 20120701-1
Tags: security

perltidy creates temporary files with default permissions, potentially 
disclosing information that wouldn't be otherwise accessible to local 
users. Temporary files should be always created with mode 0600.

The vulnerable code is:

sub make_temporary_filename {

    # [...]
    my $name = "perltidy.TMP";
    if ( $^O =~ /win32|dos/i || $^O eq 'VMS' || $^O eq 'MacOs' ) {
        return $name;
    }
    eval "use POSIX qw(tmpnam)";
    if ($@) { return $name }
    use IO::File;

    # just make a couple of tries before giving up and using the default
    for ( 0 .. 3 ) {
        my $tmpname = tmpnam();
        my $fh = IO::File->new( $tmpname, O_RDWR | O_CREAT | O_EXCL );
        if ($fh) {
            $fh->close();
            return ($tmpname);
            last;
        }
    }
    return ($name);
}

-- 
Jakub Wilk

Message #8 received at 740670@bugs.debian.org (full text, mbox, reply):

From: Don Armstrong <don@debian.org>

To: Jakub Wilk <jwilk@debian.org>, 740670@bugs.debian.org

Subject: Re: Bug#740670: perltidy: insecure permissions of temporary files

Date: Mon, 3 Mar 2014 14:52:33 -0800

On Mon, 03 Mar 2014, Jakub Wilk wrote:
> perltidy creates temporary files with default permissions,
> potentially disclosing information that wouldn't be otherwise
> accessible to local users. Temporary files should be always created
> with mode 0600.

Thanks for the report. This whole bit should probably be ripped out and
replaced by File::Temp::tempfile too, as tmpnam isn't even remotely
secure, either.

-- 
Don Armstrong                      http://www.donarmstrong.com

Creativity can be a social contribution, but only in so far
as society is free to use the results. 
 -- Richard M Stallman _GNU Manifesto_

Message #13 received at 740670@bugs.debian.org (full text, mbox, reply):

From: Murray McAllister <mmcallis@redhat.com>

To: oss-security@lists.openwall.com

Cc: 740670@bugs.debian.org

Subject: possible CVE requests: perltidy insecure temporary file usage

Date: Tue, 04 Mar 2014 13:49:35 +1100

Good morning,

Jakub Wilk and Don Armstrong are discussing in 
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=740670 1) perltidy 
creating a temporary file with default permissions instead of 0600 2) 
the use of tmpnam().

From that bug:

    my $name = "perltidy.TMP";
    if ( $^O =~ /win32|dos/i || $^O eq 'VMS' || $^O eq 'MacOs' ) {
        return $name;
    }

Would this be a separate issue on those platforms (predictable temporary 
file in current working directory, run perltidy in attacker-controlled 
directory...)? On perltidy-20090616-2.1.el6.src.rpm this was only called 
when using the "-html" option and a pod file as input, and looks to then 
possibly open it insecurely:

    else {
        $tmpfile = Perl::Tidy::make_temporary_filename();
    }
    my $fh_tmp = IO::File->new( $tmpfile, 'w' );

Trying with a much newer version on Fedora, I received errors about 
tmpnam not working and it didn't appear to be called, but haven't spent 
time debugging that yet.

Regarding other platforms:

    my $name = "perltidy.TMP";
    if ( $^O =~ /win32|dos/i || $^O eq 'VMS' || $^O eq 'MacOs' ) {
        return $name;
    }
    eval "use POSIX qw(tmpnam)";
    if ($@) { return $name }

Is the POSIX module a core part of Perl, as in, the "return $name" part 
will never be called?

Regarding the use of tmpnam, is it safe/not an issue if you open the 
resulting filename with O_CREAT and O_EXCL (as perltidy does)?

I am not sure if these 	qualify for CVEs but I believe the 
"perltidy.TMP" on Windows or Mac OS X etc would.

Thanks,

--
Murray McAllister / Red Hat Security Response Team

Message #18 received at 740670@bugs.debian.org (full text, mbox, reply):

From: Don Armstrong <don@debian.org>

To: mmcallis@redhat.com, 740670@bugs.debian.org

Cc: oss-security@lists.openwall.com, Jakub Wilk <jwilk@debian.org>

Subject: Re: Bug#740670: possible CVE requests: perltidy insecure temporary file usage

Date: Fri, 7 Mar 2014 18:39:40 -0800

On Tue, 04 Mar 2014, Murray McAllister wrote:
> Jakub Wilk and Don Armstrong are discussing in
> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=740670 1) perltidy
> creating a temporary file with default permissions instead of 0600
> 2) the use of tmpnam().

The following trivial patch fixes this issue by just using File::Temp
instead:

http://git.donarmstrong.com/?p=perltidy.git;a=blob;f=debian/patches/fix_insecure_tmpnam_usage_740670
 
I'm currently preparing an upload which will resolve this issue for
Debian in unstable and testing; I'm not certain if it necessitates a CVE
or security update in stable, but if anyone feels that way, I don't mind
preparing one.

-- 
Don Armstrong                      http://www.donarmstrong.com

listen, what you do in the privacy
of your neighbour's house while they're away
is your own business
 -- a softer world #511
    http://www.asofterworld.com/index.php?id=511

Message #23 received at 740670@bugs.debian.org (full text, mbox, reply):

From: Salvatore Bonaccorso <carnil@debian.org>

To: Don Armstrong <don@debian.org>

Cc: mmcallis@redhat.com, 740670@bugs.debian.org, Jakub Wilk <jwilk@debian.org>

Subject: Re: Bug#740670: possible CVE requests: perltidy insecure temporary file usage

Date: Sat, 8 Mar 2014 07:48:05 +0100

[Message part 1 (text/plain, inline)]

Hi Don

(dropping oss-security, as Debian specific discussion should not go to
the list there, keeping Murray):

On Fri, Mar 07, 2014 at 06:39:40PM -0800, Don Armstrong wrote:
> On Tue, 04 Mar 2014, Murray McAllister wrote:
> > Jakub Wilk and Don Armstrong are discussing in
> > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=740670 1) perltidy
> > creating a temporary file with default permissions instead of 0600
> > 2) the use of tmpnam().
> 
> The following trivial patch fixes this issue by just using File::Temp
> instead:
> 
> http://git.donarmstrong.com/?p=perltidy.git;a=blob;f=debian/patches/fix_insecure_tmpnam_usage_740670
>  
> I'm currently preparing an upload which will resolve this issue for
> Debian in unstable and testing; I'm not certain if it necessitates a CVE
> or security update in stable, but if anyone feels that way, I don't mind
> preparing one.

I have marked this issue 'no-dsa' in the security-tracker. It does not
need to be released trough security. If you have some other changes
for perltidy for oldstable and stable going trough a proposed-update
this though would be great to have included too.

Regards,
Salvatore

[signature.asc (application/pgp-signature, inline)]

Message #28 received at 740670@bugs.debian.org (full text, mbox, reply):

From: cve-assign@mitre.org

To: mmcallis@redhat.com

Cc: cve-assign@mitre.org, oss-security@lists.openwall.com, 740670@bugs.debian.org

Subject: Re: possible CVE requests: perltidy insecure temporary file usage

Date: Sat, 8 Mar 2014 22:16:08 -0500 (EST)

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Use CVE-2014-2277 for the issue in which, on all platforms, the
filename string returned by make_temporary_filename might be used for
an attacker's symlink before that filename is used by the perltidy
code to write lines into a file.

> $^O =~ /win32|dos/i || $^O eq 'VMS' || $^O eq 'MacOs'
> Would this be a separate issue on those platforms

We typically don't assign separate CVE IDs in cases where, for the
same version of the software, the vulnerability details are similar
but non-identical on different operating systems.


> Regarding the use of tmpnam, is it safe/not an issue if you open the 
> resulting filename with O_CREAT and O_EXCL (as perltidy does)?

Possibly it depends on the version of Perl or the operating system's
libraries. Maybe someone else knows the precise details. The
http://archives.neohapsis.com/archives/bugtraq/2000-02/0018.html post
claims 'because a symlink can point to nowhere, the O_EXCL|O_CREAT
test does not suffice: you might still end up making a "new" file,
even one that you own, that's somewhere else than you think it is.' On
at least some recent Linux platforms, that behavior apparently does
not occur. Specifically, if the first argument to IO::File->new is a
symlink, and the target of the symlink is a nonexistent file like
/home/victim/.forward, and O_EXCL|O_CREAT is used, then
/home/victim/.forward is not created.

This question might be relatively unimportant because O_EXCL|O_CREAT
was only used in the IO::File->new call for choosing a filename.
O_EXCL|O_CREAT wasn't used in IO::File->new call that came immediately
after the make_temporary_filename call. This, for example, doesn't
cover the case of a mode 0777 current working directory.

> 1) perltidy creating a temporary file with default permissions instead of 0600

We're not sure that this should be a vulnerability with a CVE
assignment, even though it is a violation of development standards in
some parts of the community. For example:

  http://cwe.mitre.org/data/definitions/378.html

says "Potential Mitigations ... Temporary files should be writable and
readable only by the process which own the file."

(Obviously, "own" is a typo of "owns" there. MITRE will probably fix
that later.)

It looks like the most common use case is for perltidy to read a .pl
file in the current working directory, and then create a
corresponding .pl.tdy output file in the current working directory,
with the default permissions. In this specific scenario, using default
permissions for the temporary file in the current working directory
might not be considered a security problem. Apparently there are other
use cases in which an attacker might have read access to the temporary
file but lack read access to the .tdy file. It's not clear whether
addressing that had been a perltidy design goal.

(The general counterargument to the "always mode 600" principle is
that it had been historically common to have a multi-person
development effort with a strict policy that all files must always be
group-readable. If something goes wrong when one developer is working,
and it's the responsibility of a second developer to clean up at a
time when the first developer isn't available, then one might really
want all relevant information -- including any possible left-over
temporary files -- to be accessible to the second developer.)

> Is the POSIX module a core part of Perl, as in, the "return $name" part 
> will never be called?

It's conceivable that that depends on the version of Perl, but in any
case the answer doesn't affect how many CVE IDs are needed.

- -- 
CVE assignment team, MITRE CVE Numbering Authority
M/S M300
202 Burlington Road, Bedford, MA 01730 USA
[ PGP key available through http://cve.mitre.org/cve/request_id.html ]
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.14 (SunOS)

iQEcBAEBAgAGBQJTG9yVAAoJEKllVAevmvms884IALYMJ4O0dcep0uIKRR84BIRZ
153u8FdoqUOSYQqQyowraXdpHYgSqkXjv2Rr/ATCIx//EehZU4nTRyBJ9Y5VtwCF
pncZZBz4cOzoKv2Q+7BjsIuU8PDz8wRR+1kXr1/lnyvtMRqUO49y2pzGbdSDGZs6
+TZ5/KjBiDMHGFUOV+wd9sWE1S4dV9h4CiipyL8WxAaaeAl95zZbEfSSDcXoWqI8
2CkXB03o4lUSvjvDkt07+zZn4R9a0BuFIM626spRlMO9H132KhCpF2Hc73px4sWl
xGjYfN1IE53JnhrMgKrzwcSzQXBXiCEPk+gMdTjqR+frcF4+RC29fwnjp1g7Afo=
=EM42
-----END PGP SIGNATURE-----

Message #33 received at 740670-close@bugs.debian.org (full text, mbox, reply):

From: Don Armstrong <don@debian.org>

To: 740670-close@bugs.debian.org

Subject: Bug#740670: fixed in perltidy 20130922-1

Date: Sun, 09 Mar 2014 05:33:33 +0000

Source: perltidy
Source-Version: 20130922-1

We believe that the bug you reported is fixed in the latest version of
perltidy, which is due to be installed in the Debian FTP archive.

A summary of the changes between this version and the previous one is
attached.

Thank you for reporting the bug, which will now be closed.  If you
have further comments please address them to 740670@bugs.debian.org,
and the maintainer will reopen the bug report if appropriate.

Debian distribution maintenance software
pp.
Don Armstrong <don@debian.org> (supplier of updated perltidy package)

(This message was generated automatically at their request; if you
believe that there is a problem with it please contact the archive
administrators by mailing ftpmaster@ftp-master.debian.org)


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Format: 1.8
Date: Fri, 07 Mar 2014 18:18:01 -0800
Source: perltidy
Binary: perltidy
Architecture: source all
Version: 20130922-1
Distribution: unstable
Urgency: medium
Maintainer: Don Armstrong <don@debian.org>
Changed-By: Don Armstrong <don@debian.org>
Description: 
 perltidy   - Perl script indenter and reformatter
Closes: 740559 740670
Changes: 
 perltidy (20130922-1) unstable; urgency=medium
 .
   * New upstream release (closes: #740559)
   * Update VCS location after switching to git
   * Fix insecure tmpfile creation (closes: #740670)
Checksums-Sha1: 
 82372b8e0f38356d296eafe129fe95c47d4acbde 1826 perltidy_20130922-1.dsc
 9582498f5c2c9bd12bd4baf05211a1e978c20c06 422005 perltidy_20130922.orig.tar.gz
 fac6cd67aa5c7bfb521df800348b779df5947905 3872 perltidy_20130922-1.debian.tar.xz
 3a706e3a5583f0c36aa935f2ea78f4238de83edd 344238 perltidy_20130922-1_all.deb
Checksums-Sha256: 
 7f140774e0e63489ec8937bad2cca1564da2ab67034620f1f360de2c3972e835 1826 perltidy_20130922-1.dsc
 e58cd6f00067dc814d2cecab209087d3b282648371b5a2ebd6f48484c030b762 422005 perltidy_20130922.orig.tar.gz
 4ef680355c8d53dc7b30b28f589f55bcec3ebf7cb9741be74c760911d86ca522 3872 perltidy_20130922-1.debian.tar.xz
 810420262dd606895661ce959929059f3a546116358763d27f6ce8bbe13ff457 344238 perltidy_20130922-1_all.deb
Files: 
 9fa93b9de219749854ce1d86595fac76 1826 devel optional perltidy_20130922-1.dsc
 efc831bc9f238ae037dae22c41b6ba31 422005 devel optional perltidy_20130922.orig.tar.gz
 0fa0cdb8817f6faf4cb97efa3d3ebb25 3872 devel optional perltidy_20130922-1.debian.tar.xz
 43c3fa3051ab9f20e60960fa1e28a9ac 344238 devel optional perltidy_20130922-1_all.deb

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQIVAwUBUxv6baLRMAHZjA+6AQpfXQ//XWyf8IcCwYparyWmayFJbSp4nj904dA1
XFRDu2dKcKa1GOsGqrd1eSSCHogn4y8mKmcfiZv53j0A22xDtIXQS8NqTv0llUpl
4ICxMYa2aJDqR7ry58aekvzIPLZ8cbfwvw9kSqzL2SdVTHrL8J6LNzM66+1CluD2
IYolQF88rLkfWV6AeXW9o239kz0g+UcD3DGBbnw3k5FDqoVFc5HIA0xQ0E595gxq
OoMx9TbzGbiX6vjcbkSYT1VBUavlK35WUIYi3Rd4otrDOZPqO1q4UIrn9/eIOdkX
BXcgmOPEVWuEzUwz6OjoKkomGTiuqqj0VCICyq3hiSl0EmdD3htUbHJXAVf/Vb5G
6/cfE+0L1nqOXWaftVvxY4Os4ZNvpfkrqb21yb5P9dBWiaf23w6YkQrey+bju5bm
hJyaaTeMtKbfo+e/iw0g083c2WOcOQNaB2GeJZGcxvgNCRJwbg2v5mnOC/ZQ8GoF
QsN72GiNvhOJgy39UySnZGBn64QZ7Ppss/0JyIDv/Nl6jIDPrvmZ9wGUFCq+W//2
lwwxyPtx69VLezLuLOWV6EzJghTTrPkxMokuN6D+GDaDmynyu06rtCruJH7oqV92
CyLND8Fdn2lZkJ32f0TfkO7kMRK0UUoiZokB9nCXHKCLx5ipccbyEBoue3NNEi59
PDKdckV8pfw=
=w6XL
-----END PGP SIGNATURE-----

Message #38 received at 740670@bugs.debian.org (full text, mbox, reply):

From: Don Armstrong <don@debian.org>

To: perltidy@users.sourceforge.net, 740670@bugs.debian.org

Subject: Re: Bug#740670: possible CVE requests: perltidy insecure temporary file usage

Date: Mon, 10 Mar 2014 10:28:26 -0700

On Fri, 07 Mar 2014, Don Armstrong wrote:
> On Tue, 04 Mar 2014, Murray McAllister wrote:
> > Jakub Wilk and Don Armstrong are discussing in
> > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=740670 1) perltidy
> > creating a temporary file with default permissions instead of 0600
> > 2) the use of tmpnam().
> 
> The following trivial patch fixes this issue by just using File::Temp
> instead:
> 
> http://git.donarmstrong.com/?p=perltidy.git;a=blob;f=debian/patches/fix_insecure_tmpnam_usage_740670
>  
> I'm currently preparing an upload which will resolve this issue for
> Debian in unstable and testing; I'm not certain if it necessitates a CVE
> or security update in stable, but if anyone feels that way, I don't mind
> preparing one.

I just wanted to draw your attention to this patch; it fixes the
insecure tmpnam and temporary file creation by using
File::Temp::tempfile. A CVE has been given,
https://security-tracker.debian.org/tracker/CVE-2014-2277


-- 
Don Armstrong                      http://www.donarmstrong.com

If I had a letter, sealed it in a locked vault and hid the vault
somewhere in New York. Then told you to read the letter, thats not
security, thats obscurity. If I made a letter, sealed it in a vault,
gave you the blueprints of the vault, the combinations of 1000 other
vaults, access to the best lock smiths in the world, then told you to
read the letter, and you still can't, thats security.
 -- Bruce Schneier

Message #43 received at 740670@bugs.debian.org (full text, mbox, reply):

From: Steven Hancock <s7078hancock@gmail.com>

To: "perltidy@users.sourceforge.net" <perltidy@users.sourceforge.net>, "740670@bugs.debian.org" <740670@bugs.debian.org>

Subject: Re: Bug#740670: possible CVE requests: perltidy insecure temporary file usage

Date: Mon, 10 Mar 2014 14:46:12 -0700

[Message part 1 (text/plain, inline)]

Don, Thanks, I will put it in the next release. Steve

On Monday, March 10, 2014, Don Armstrong <don@debian.org> wrote:

> On Fri, 07 Mar 2014, Don Armstrong wrote:
> > On Tue, 04 Mar 2014, Murray McAllister wrote:
> > > Jakub Wilk and Don Armstrong are discussing in
> > > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=740670 1) perltidy
> > > creating a temporary file with default permissions instead of 0600
> > > 2) the use of tmpnam().
> >
> > The following trivial patch fixes this issue by just using File::Temp
> > instead:
> >
> >
> http://git.donarmstrong.com/?p=perltidy.git;a=blob;f=debian/patches/fix_insecure_tmpnam_usage_740670
> >
> > I'm currently preparing an upload which will resolve this issue for
> > Debian in unstable and testing; I'm not certain if it necessitates a CVE
> > or security update in stable, but if anyone feels that way, I don't mind
> > preparing one.
>
> I just wanted to draw your attention to this patch; it fixes the
> insecure tmpnam and temporary file creation by using
> File::Temp::tempfile. A CVE has been given,
> https://security-tracker.debian.org/tracker/CVE-2014-2277
>
>
> --
> Don Armstrong                      http://www.donarmstrong.com
>
> If I had a letter, sealed it in a locked vault and hid the vault
> somewhere in New York. Then told you to read the letter, thats not
> security, thats obscurity. If I made a letter, sealed it in a vault,
> gave you the blueprints of the vault, the combinations of 1000 other
> vaults, access to the best lock smiths in the world, then told you to
> read the letter, and you still can't, thats security.
>  -- Bruce Schneier
>

[Message part 2 (text/html, inline)]

Message #48 received at 740670@bugs.debian.org (full text, mbox, reply):

From: Steven Hancock <s7078hancock@gmail.com>

To: "740670@bugs.debian.org" <740670@bugs.debian.org>

Subject: Re: Bug#740670: Info received (Bug#740670: possible CVE requests: perltidy insecure temporary file usage)

Date: Fri, 28 Mar 2014 06:30:39 -0700

[Message part 1 (text/plain, inline)]

This patch is in version 20140328 at CPAN.


On Mon, Mar 10, 2014 at 2:48 PM, Debian Bug Tracking System <
owner@bugs.debian.org> wrote:

> Thank you for the additional information you have supplied regarding
> this Bug report.
>
> This is an automatically generated reply to let you know your message
> has been received.
>
> Your message is being forwarded to the package maintainers and other
> interested parties for their attention; they will reply in due course.
>
> Your message has been sent to the package maintainer(s):
>  Don Armstrong <don@debian.org>
>
> If you wish to submit further information on this problem, please
> send it to 740670@bugs.debian.org.
>
> Please do not send mail to owner@bugs.debian.org unless you wish
> to report a problem with the Bug-tracking system.
>
> --
> 740670: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=740670
> Debian Bug Tracking System
> Contact owner@bugs.debian.org with problems
>

[Message part 2 (text/html, inline)]

Message #53 received at 740670@bugs.debian.org (full text, mbox, reply):

From: Don Armstrong <don@donarmstrong.com>

To: Steven Hancock <s7078hancock@gmail.com>, 740670@bugs.debian.org

Subject: Re: Bug#740670: Info received (Bug#740670: possible CVE requests: perltidy insecure temporary file usage)

Date: Fri, 28 Mar 2014 09:47:22 -0700

Control: tag -1 fixed-upstream

On Fri, 28 Mar 2014, Steven Hancock wrote:
> This patch is in version 20140328 at CPAN.

Awesome. Thanks Steven! I'll get this packaged for Debian shortly.

-- 
Don Armstrong                      http://www.donarmstrong.com

"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot

Send a report that this bug log contains spam.