potential race condition with temporary files
Submitted by an unknown user
Assigned to Lucas Beeler
Link to original bug (#718169)
Description
---- Reported by shotwell-maint@gnome.bugs 2011-11-18 20:26:00 -0800 ----
Original Redmine bug id: 4411
Original URL: http://redmine.yorba.org/issues/4411
Searchable id: yorba-bug-4411
Original author: Andrew McNabb
Original description:
In AppDirs.vala, the get_temp_dir function creates a temporary directory in a deterministic manner. Usually I would expect a call to mkdtemp, which ensures that the directory has a unique name and 0700 permissions.
I haven't looked into this very deeply, but it looks like this may be a security problem.
---- Additional Comments From shotwell-maint@gnome.bugs 2013-05-01 11:38:00 -0700 ----
History
Comment 1
Updated by Jonas Bushart over 1 year ago
- File 0001-Fix-security-issue-related-4411.patch added
I think this is a good idea, but using mkdtemp works not well because this function gets called often (which spams tmp folder).
But restricting access to Shotwells tmp should be done.
Comment 2
Updated by Adam Dingle over 1 year ago
- Status changed from Open to Review
- Assignee set to Lucas Beeler
- Priority changed from High to Urgent
- Target version set to 0.12
Lucas, please review.
Comment 3
Updated by Andrew McNabb over 1 year ago
Using mkdtemp wouldn't require creating a brand new directory each time the function gets called. Rather, it could create a temporary directory once and then return that directory at each subsequent call.
The two problems that I can think of with the patch:
-
If I don't like user 1000, I can create a /tmp/shotwell-1000 directory to stop them from using shotwell.
-
I can create /tmp/shotwell-1000 with permissions 777. Then user 1000 can create /tmp/shotwell-1000/1234. Then I can delete the directory /tmp/shotwell-1000/1234 and recreate it. Now the temp directory is owned by me.
Using mkdtemp is a best practice for a reason.
Comment 4
Updated by Jonas Bushart over 1 year ago
Sure, you are right. I haven't thought of this.
I will just write a new patch.
Comment 5
Updated by Jonas Bushart over 1 year ago
- File 0001-Fixes-4411-by-calling-mkdtemp.patch added
There is a new private var for saving a tmp dir.
If this is null invoce mkdtemp with
Environment.get_tmp_dir() + "/shotwell-XXXXXX"
as template.
Comment 6
Updated by Lucas Beeler over 1 year ago
- Category set to 4
- Status changed from Review to 5
- Resolution set to fixed
Fixed in 095f5637.
Comment 7
Updated by Charles Lindsay 7 months ago
- Status changed from 5 to Fixed
--- Bug imported by chaz@yorba.org 2013-11-25 21:55 UTC ---
This bug was previously known as bug 4411 at http://redmine.yorba.org/show_bug.cgi?id=4411 Imported an attachment (id=262236) Imported an attachment (id=262237)
Unknown Component Using default product and component set in Parameters Unknown milestone "unknown in product shotwell. Setting to default milestone for this product, "---". Setting qa contact to the default for this product. This bug either had no qa contact or an invalid one.
Version: 0.12
Resolution: RESOLVED FIXED