Bug 766805 (CVE-2011-4606)

Summary: (CVE-2011-4606) rocksndiamonds: creates ~/.rocksndiamonds/ directory as world-writable
Product: [Other] Security Response Reporter: Vincent Danen <vdanen>
Component: vulnerabilityAssignee: Red Hat Product Security <security-response-team>
Status: CLOSED ERRATA QA Contact:
Severity: low Docs Contact:
Priority: low    
Version: unspecifiedCC: tcallawa
Target Milestone: ---Keywords: Security
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2015-08-22 15:02:10 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On: 766810    
Bug Blocks:    
Attachments:
Description Flags
Patch to resolve directory security issue none

Description Vincent Danen 2011-12-12 16:20:30 UTC 
It was reported [1] that the rocksndiamonds game would create it's configuration/cache directory (~/.rocksndiamonds/) as world-writable.  If a user's home directory was mode 0755 (the default is 0700), a local attacker could delete one of the cache files and replace it with a symbolic link to some other file they did not have access to.  When the victim loaded the game again, that file would be overwritten.

[1] http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=651620
Comment 1 Vincent Danen 2011-12-12 16:25:41 UTC 
Created rocksndiamonds tracking bugs for this issue

Affects: fedora-all [bug 766810]
Comment 2 Kurt Seifried 2011-12-12 17:42:03 UTC 
this issue has been assigned CVE-2011-4606 http://seclists.org/oss-sec/2011/q4/498
Comment 3 Tom "spot" Callaway 2011-12-12 19:39:06 UTC 
Created attachment 545893 [details]
Patch to resolve directory security issue

I believe this patch will resolve the security concern, although, I'd like for someone on the Red Hat Security team to double-check my work before I send it upstream and kick off Fedora builds.
Comment 4 Vincent Danen 2011-12-12 22:09:24 UTC 
(In reply to comment #3)
> Created attachment 545893 [details]
> Patch to resolve directory security issue

At a quick glance, I'm not sure it will.  Shouldn't there be another else in there?  If we're not running setgid, then we're testing for permissions_class (no idea what it's set to), but what if that fails?  There's no "else" there if that test fails.

I don't have time at the moment to look at the patch in context; I suppose sending it upstream for peer review might be best and if they have something better/different, we can use what they have (this is a pretty low-impact issue, so I don't think we need to rush it -- I'd rather upstream review the whole thing first (I don't even know if they've been made aware of it)).
Comment 5 Tom "spot" Callaway 2011-12-12 22:40:24 UTC 
So, the context for that code is this (in english):

If the code is running setgid,
  then, be sure to set the umask appropriately.
Else (it is not setgid)
  then, force world writeable permissions (+w) in addition to DIR_PERMS_PRIVATE 
  (0700)

I changed it to this:

If the code is running setgid,
  then, be sure to set the umask appropriately.
Else (it is not setgid)
  If the directory is explicitly supposed to be created as a public directory
    then and only then, force world writeable permissions (+w) in addition to 
    DIR_PERMS_PUBLIC(0755) (because that is set whenever a directory is created 
    as a public directory)

When the directory is explicitly supposed to be created as a private directory (which the user ~/.rocksndiamonds directory is already set as), it passes through 
createDirectory() with DIR_PERMS_PRIVATE.

It is possible that there is still a security concern in the setgid case, but since we don't run that way for obvious reasons, I don't lose sleep here.

I'll send it upstream for more feedback.
Comment 6 Vincent Danen 2011-12-12 22:58:08 UTC 
Oh, so the default then is for the directories to be created 0700?  In that case, I think the patch looks fine.  Thanks for taking the time to explain the context.
Comment 7 Tom "spot" Callaway 2011-12-12 23:13:32 UTC 
Yes, unless you tell createDirectory() that you're making a "Public Directory" (permission_class == PERMS_PUBLIC). Which it never does for the ~/.rocksndiamonds dir.
Comment 8 Vincent Danen 2011-12-13 19:13:57 UTC 
Makes sense.  Thanks!
Comment 9 Fedora Update System 2012-02-21 01:27:27 UTC 
rocksndiamonds-3.3.0.1-5.fc16 has been pushed to the Fedora 16 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 10 Fedora Update System 2012-02-21 01:32:11 UTC 
rocksndiamonds-3.3.0.1-5.fc15 has been pushed to the Fedora 15 stable repository.  If problems still persist, please make note of it in this bug report.