Bug 751886 (CVE-2011-4115) - CVE-2011-4115 perl-Parallel-ForkManager: insecure temporary file usage
Summary: CVE-2011-4115 perl-Parallel-ForkManager: insecure temporary file usage
Keywords:
Status: CLOSED ERRATA
Alias: CVE-2011-4115
Product: Security Response
Classification: Other
Component: vulnerability
Version: unspecified
Hardware: All
OS: Linux
low
low
Target Milestone: ---
Assignee: Red Hat Product Security
QA Contact:
URL:
Whiteboard:
Depends On: 751887 751888 751889
Blocks:
TreeView+ depends on / blocked
 
Reported: 2011-11-07 22:25 UTC by Vincent Danen
Modified: 2019-09-29 12:48 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2016-06-10 21:34:43 UTC
Embargoed:


Attachments (Terms of Use)

Description Vincent Danen 2011-11-07 22:25:28 UTC
It was reported [1] that Parallel:ForkManager handles temporary files insecurely.  This can be used by a malicious local user to perform certain actions, such as overwriting arbitrary files with symlink attacks.

There does not yet seem to be an upstream fix.

[1] https://rt.cpan.org/Public/Bug/Display.html?id=68298

Comment 1 Vincent Danen 2011-11-07 22:26:29 UTC
Created perl-Parallel-ForkManager tracking bugs for this issue

Affects: fedora-all [bug 751887]
Affects: epel-5 [bug 751888]
Affects: epel-6 [bug 751889]

Comment 2 Jason Tibbitts 2011-11-09 00:09:43 UTC
Guess I should comment here instead of in one of the tracking bugs.

Bottom line is that this has been open upstream since May: https://rt.cpan.org/Public/Bug/Display.html?id=68298

As far as I can tell, upstream is completely unresponsive; there have been no comments to pretty much any of the bugs open on all of his packages on CPAN.

So far my searching hasn't turned up any patches from any other distro, but it's always possible that I'm missing something.  I'm not really up on the current state of cross-distro security collaboration so if anyone has any guidance, I'd be happy to hear it.

Packages requiring this one:
netdisco
perl-FusionInventory-Agent-Task-NetDiscovery
perl-FusionInventory-Agent-Task-SNMPQuery

Honestly at this point I'd really like to just drop it from the distro, but that may not be an option.  What remains is to fix it, but that doesn't appear trivial.

The module uses files in /tmp for communication between the master process and its children.  The children write out a file with a predictable name and the master knows where to look for data when a child exits.  You can't randomize the name because the predictability is important to how things work.

The master could pass a random name, in the environment or something, but that still gives an attacker plenty of time to get in there and predict the filename that will be used, create it, and do various 

The master could pass an open file handle or something, but that changes the API.

I wonder if it would be sufficient to create a random mode 700 directory in /tmp and just use that.  Honestly I'm no security expert and I certainly don't want to attempt a fix that doesn't actually help.  I've tried that before and found that the experts generally get rather derisive when you say you've fixed something but they can still find a problem with it.

Comment 3 Tomas Hoger 2011-11-10 09:17:49 UTC
I gave this some look yesterday, and I can't see any nice way to fix this either.

Safely creating temporary directory under /tmp would probably be the least invasive way to fix, but there's a problem with the clean-up at the end.  It seems it's valid to use this module in a way that parent process forks bunch of child processes, let them do their work and exit before they finish.  Hence this approach should leave a temporary directory behind on each run of such script.

Changing child code to create the file safely does not seem like an option either.  If the predictable file name is used, attacker may "DoS" the child (create temp file so that child can not while its "reply" data structure).  Child can block (keep re-trying to create the file) or exit (let parent parse malicious file), but cases seem wrong.  As a side note, in current implementation, parent process tries to read the temp file even when the script is not written to use this data passing feature.  Hence attacker can easily make parent deserialize arbitrary malicious Storable blob.  Storable man page only mention it's unsafe when storing / retrieving of CODE is allowed (not by default), but still sounds like "don't do this".  Even if it's safe, that may have other side effects (block parent on read, make it use extra memory, trigger side effects when opening special files).

Parent process can create temp file safely, but can not use predictable file name with its current format, as it does not know child pid before fork.  Hence it'd need to use random name and have it stored in both parent and child data structs.  Of course, this has the same problem as temp dir idea in cases where parent exits before child.

One improvement can be to create temp file only when script wants to use structure passing (and hence parent is expected to wait for child), i.e. when on_finish callback is set.  However, even though docs say to declare callbacks before first start, it's apparent you can set them after start, as run_on_finish accepts optional pid argument to set child-specific callback.  I'm not sure what the intended semantic was, as setting callback after run is racy and the callback may not be run if child exits quickly.

One idea is to make new's tempdir parameter mandatory if script wants to use structure passing, and document that caller should provide a safe temporary directory (i.e. not /tmp) - such as directory that is not world writeable (/home/user/something or /var/lib/something), or safely created under /tmp.  Of course, that's something that we can't do without upstream.

We can downgrade module to the F14 version that lacks this new feature ;).

Comment 4 Jason Tibbitts 2011-11-10 18:11:51 UTC
I like the idea of making tempdir mandatory.  It conveniently both shows us if any consuming code would have the security issue and pushes responsibility for that bit of security elsewhere.  Let me whip something up.

I believe the maintainers of the packages which depend on Parallel::ForkManager are CC'd on this ticket already; could either of you check to see if your packages would have issues with this or if they even use the problematic feature?

Comment 5 Remi Collet 2011-11-12 07:55:51 UTC
According to fusioninventory upstream, this dependecy is no more used (probably detected because of old code, not cleaned but no more used).

I will clear this dependency in next (coming soon) version.

Comment 6 Petr Pisar 2013-10-17 17:40:40 UTC
Upstream has resolved this issue.

According to Gentoo, version 1.02 is fixed <http://www.gentoo.org/security/en/glsa/glsa-201310-11.xml>. Upstream states version 1.0.0 brings the fix <http://cpansearch.perl.org/src/SZABGAB/Parallel-ForkManager-1.05/Changes>. Please note that upstream has changed version schema in between, thus 1.02 is equaled to 1.20.0.

Comment 7 Tomas Hoger 2013-10-17 18:41:28 UTC
It seems the fix is:
http://code.google.com/p/perl-parallel-forkmanager/source/detail?r=42011ee

Additional related fixes:
http://code.google.com/p/perl-parallel-forkmanager/source/detail?r=80a4c5c
http://code.google.com/p/perl-parallel-forkmanager/source/detail?r=a89abfb

AFAICS, this is affected by the clean up concern mentioned in comment 3, albeit you can argue that apps where parent exists before child should not really expect child to be able to (safely) pass their results back.

Comment 8 Jason Tibbitts 2013-10-17 18:45:41 UTC
To be honest I had given up hope that upstream would ever reawaken.  I'll have a look at the latest version.


Note You need to log in before you can comment on or make changes to this bug.