6.0.0-git
2019-03-19

[#7646] Driver 'file' fails to open files with '..' anywhere in name
Summary Driver 'file' fails to open files with '..' anywhere in name
Queue Horde Framework Packages
Queue Version Git master
Type Bug
State Resolved
Priority 2. Medium
Owners
Requester andrew (at) aklabs (dot) net
Created 2008-11-05 (3786 days ago)
Due
Updated 2010-06-29 (3185 days ago)
Assigned 2008-11-06 (3785 days ago)
Resolved 2010-06-29 (3185 days ago)
Milestone
Patch No

History
2010-06-29 04:53:36 Michael Slusarz Version ⇒ Git master
Queue ⇒ Horde Framework Packages
State ⇒ Resolved
 
2010-06-29 04:52:48 Git Commit Comment #10 Reply to this comment
Changes have been made in Git for this ticket:

MFB: Bug #7646
Submitted by: Valentin.Vidic@CARNet.hr
Allow access to files with multiple consecutive dots in the name

Revision   Changes    Path
1.1.2.7    +4 -2      framework/VFS/lib/VFS/file.php
1.36.4.35  +2 -1      framework/VFS/package.xml

http://git.horde.org/diff.php/framework/VFS/lib/VFS/file.php?rt=horde-git&r1=079bd8d84c09d7fbef27cdf291f3d94ed203b5d7&r2=083dda2bd5e0b8408cc0b9c18a4a20416806214e
http://git.horde.org/diff.php/framework/VFS/package.xml?rt=horde-git&r1=079bd8d84c09d7fbef27cdf291f3d94ed203b5d7&r2=083dda2bd5e0b8408cc0b9c18a4a20416806214e
2010-06-07 11:11:16 Valentin (dot) Vidic (at) carnet (dot) hr Comment #8
New Attachment: file.php.diff Download
Reply to this comment
Noticed the same problem: gollem doesn't work correctly for files with 
multiple consecutive dots in name. Patch for this is attached. Since 
basename already removes directory path from the name there is no need 
to remove consecutive dots from the file name. The only security 
problem to check is the file name equal to ".."
2008-12-22 03:08:23 Chuck Hagenbuch State ⇒ No Feedback
 
2008-12-15 02:54:21 Chuck Hagenbuch Comment #7 Reply to this comment
Ping again?
2008-11-17 18:04:49 andrew (at) aklabs (dot) net Comment #6 Reply to this comment
Are you going to provide an updated patch?
Yes, sorry, I've been in the thick of things recently. I'm going to 
try to get time for another patch later today. If I don't provide a 
patch by 5 pm EST November 18 2008, then someone else can feel free to 
step in.
2008-11-17 15:23:06 Jan Schneider Comment #5 Reply to this comment
Are you going to provide an updated patch?
2008-11-06 15:17:13 Chuck Hagenbuch Patch ⇒ No
 
2008-11-06 15:16:54 Chuck Hagenbuch Deleted Original Message
 
2008-11-06 15:16:48 Chuck Hagenbuch Comment #4 Reply to this comment
I just used a regex 'cause it was the only way I knew for sure to
only check the beginning of the string. I'll submit another patch
later today that uses pcre instead.
http://php.net/substr



etc...



Thanks!
2008-11-06 13:38:33 andrew (at) aklabs (dot) net Comment #3 Reply to this comment
What about paths like file.pdf/../../../etc/passwd ?
I tried that, and it's stripped out. Any filename that has / in it (on 
my unix box, at least) will only use the portion after the last / , as 
if you had run 'basename' against it. So in this case the file is 
simply renamed 'passwd' in the current directory.
Much less importantly, ereg_* is deprecated and against Horde CS;
please use the pcre functions instead (although this particular case
doesn't even need a regex).
I just used a regex 'cause it was the only way I knew for sure to only 
check the beginning of the string. I'll submit another patch later 
today that uses pcre instead.
2008-11-06 01:36:42 Chuck Hagenbuch Comment #2
State ⇒ Feedback
Reply to this comment
What about paths like file.pdf/../../../etc/passwd ?



Much less importantly, ereg_* is deprecated and against Horde CS; 
please use the pcre functions instead (although this particular case 
doesn't even need a regex).
2008-11-05 22:23:58 andrew (at) aklabs (dot) net Comment #1
Type ⇒ Bug
State ⇒ Unconfirmed
Priority ⇒ 2. Medium
Summary ⇒ Driver 'file' fails to open files with '..' anywhere in name
Queue ⇒ Gollem
Milestone ⇒
Patch ⇒ Yes
New Attachment: file.php.patch
Reply to this comment
This may already be fixed upstream in latest head, and if so, please 
forgive. I am using 1.0.3 because it's what's in Ubuntu 8.04 LTS's 
repository as of the latest apt-get update.



When using the 'file' VFS driver on a Linux host using Horde 3.1.7, 
IMP H3 4.1.4 and Gollem H3 1.0.3, users are unable to open (or attach 
to IMP outgoing messages), any files that contain '..' anywhere in the 
file name. Test case:



Create a file in a VFS share with the filename 'test.pdf'. Opens correctly.

Rename the file to 'test..pdf'. The file will silently fail to attach 
to IMP messages, and will fail to view with the following error:



-------------------------------------------------



Warning: file_get_contents(/vfsdir/horde//filepdf) 
[function.file-get-contents]: failed to open stream: No such file or 
directory in /usr/share/horde3/lib/VFS/file.php on line 82



Warning: Cannot modify header information - headers already sent by 
(output started at /usr/share/horde3/lib/VFS/file.php:82) in 
/usr/share/horde3/lib/Horde/Browser.php on line 978



Warning: Cannot modify header information - headers already sent by 
(output started at /usr/share/horde3/lib/VFS/file.php:82) in 
/usr/share/horde3/lib/Horde/Browser.php on line 984



Warning: Cannot modify header information - headers already sent by 
(output started at /usr/share/horde3/lib/VFS/file.php:82) in 
/usr/share/horde3/lib/Horde/Browser.php on line 1003



-----------------------------------------------



Solution: I opened up /usr/share/horde3/lib/VFS/file.php and found the 
error inside of _getNativePath where '..' is replaced with ''. The 
reason for this is obvious (security), but the method failed to take 
into account situations like this where the user just accidentally put 
two ..'s before an extension. I replaced the str_replace call with an 
ereg_replace call to only do this at the beginning of the filename. 
Works like a charm. I tried naming files things like 
'../sneakyfile.pdf' and such, and gollem wasn't freaked out by any 
tests I could do.



Patch is attached to bug report in unified diff format.

Saved Queries