Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Download File/Image returns zero-length data #129

Closed
BenKoerber opened this issue Aug 6, 2012 · 14 comments
Closed

Download File/Image returns zero-length data #129

BenKoerber opened this issue Aug 6, 2012 · 14 comments
Labels

Comments

@BenKoerber
Copy link

Problem:
When trying to download "Files & Images" which have been attached to an item, one receives an empty file (zero-content length).

Affected Version:
2.1.7 (2.1.8)

System-Info:

  • OS: Debian (Linux pw 2.6.32-11-pve Ldap/Ou configuration #1 SMP Wed Apr 11 07:17:05 CEST 2012 i686 GNU/Linux)
  • Webserver: Apache/2.2.16 (Debian)
  • PHP: Version 5.3.3-7+squeeze13
  • Browser: Mozilla/5.0 (X11; Ubuntu; Linux i686; rv:14.0) Gecko/20100101 Firefox/14.0.1

Checked:

  • The files to be downloaded exists in the ./upload folder
  • The file in the ./upload folder has the right content
  • The file in the ./upload folder resolves to the right name (via DB-lookup)
  • The files in the ./upload should be accessible
    -> chmod www-data:www-data ./upload/*
    -> chmod 0644 ./upload/*

Best,
Ben.

@nilsteampassnet
Copy link
Owner

I've just tested on 2 systems: The 1st is a wampp server on Windows and the other Ubuntu 11.04.
With both I succeed to get my attached files.

Can you check what happens if "upload" folder has 0777 rights?

@BenKoerber
Copy link
Author

Hey Nils,

I tried again and changed the permissions on the folder as well as the files to 0777 - unfortunately, I am still downloading empty files. Since uploading and pretty much everything else works just fine, I do not think that this problem boils down to an installation issue ...
I've also tried on my local system (Ubuntu 12.04 / 32 bit) - same problem. However, I played around with some image files and they are to be downloadable - here is the link used in the frontend-view

However the attached txt-file will be downloaded somewhat differently. If I try to download an attached text file, the following link is used in the frontend (returns zero-length content):

However, when trying to access the text-file the same way I can accessed the jpeg, the text-file will be downloadable:

Well, while is a workaround I still wonder about security - is it by design, that the content of the upload folder is directly accessible and if so, why? (Or have I just screwed the security model by messing with chmod?)

Best,
Ben.

@nilsteampassnet
Copy link
Owner

I'll take time to analyze why you are facing this issue.
For sure, if you have it, something is wrong ... the only thing is that I need to reproduce in order to correct it.
Nevertheless I can do a code review on this specific part ... with the details you gave me, I know where to focus ;-)

Concerning security, sure it is better to put the "upload" folder beyond the "www" folder so that it is not accessible from "external". There has been some discussions about this specific point. I've not found time to propose a native solution because actually mainly users are using the tool inside their local domain with no external access.

But I've planned to do a simple change for next release that consists in adding a setting for "path to upload folder". This will permit an experimented webmaster to define a protected folder. But in that case, he will have to define also the redirection rules on the server.

Cheers

@BenKoerber
Copy link
Author

That sounds great! If I can help somehow, please let me know ...
Also thanks for the clarification on the "upload"-folder issue.

Best,
Ben.

@BenKoerber
Copy link
Author

BTW: Here is my five cents regarding the attachments: wouldn't it be even better to store the attachments in the database? I wouldn't expect the files to be extraordinary large so that size isn't an issue. However, when going down that road, make sure to create an 1:n relationship to the new table (teampass_files_data table) that is lazily loaded.
Best,
Ben.

@nilsteampassnet
Copy link
Owner

Thank you for the suggestion. I didn't think about it because focusing on files, but sure you are right, this could be a very good solution.
I need to think about it.

@niallfleming
Copy link

Incidentally I'm having this issue too, I've not managed to find what the issue is yet.

My server is pretty much the same as Ben's setup, Debian Squeeze with Apache and PHP 5.3

@BenKoerber
Copy link
Author

@niallfleming: Welcome to the Club ;)
@nilsteampassnet: Did you find some time to further look into this problem? As pointed out, I am (and supposedly Niall as well) glad to assit.

@rluigjes
Copy link

I had the same issue when downloading files uploaded in the files tab of a key.
The errors in my apache log were:
PHP Notice: Undefined index: sub in /var/www/teampass/sources/downloadFile.php on line 25, .....
PHP Warning: readfile(..//63a8b186f9bf7fe78b20f61957614e60): failed to open stream: No such file or directory in /var/www/teampass/sources/downloadFile.php on line 25, ......
So i looked up the file and after that the exact url. It occured to me the GET var 'sub' was blank, so it was looking for a file in a ..// dir.
For my install I could fix it myself by checking for the 'sub' and if not present or empty, using 'upload' as directory.

possibly related two identical other error lines with each file download:
File does not exist: /var/www/teampass/"includes, referer: http://..*/teampass/index.php?page=items&group=7&id=19

Hopefully this helps.

Edit: I accidently a word.

@nilsteampassnet
Copy link
Owner

Added a new field for Path and URL for Upload in 2.1.9

I'll add to relase 2.2, the storage in database.

@BenKoerber
Copy link
Author

Nils, that sounds great. Thx!

nilsteampassnet added a commit that referenced this issue Sep 2, 2012
 * #126-#132-#130-#131-#139-#129-#141-#146
 * Italian translation
 * Find page - focus in search box (contributor: Jay2k1)
nilsteampassnet added a commit that referenced this issue Sep 2, 2012
 * #126-#132-#130-#131-#139-#129-#141-#146
 * Italian translation
 * Find page - focus in search box (contributor: Jay2k1)
@adrianmeca
Copy link

The upload to the database will be a great feature.
When would you thing the 2.2 will be released?

Nils, thanks for the work and for this great tool

@nilsteampassnet
Copy link
Owner

You're welcome ;-)

2.2 should be ready for end of this year (December 2012).
You know, this version will include some major changes that could take a long time to implement (at least to implement the upgrades from previous releases).
And the new "theming" should also take much time (I estimate to 1 month of work only on that).

----- Mail original -----
De: "adrianmeca" notifications@github.com
À: "nilsteampassnet/TeamPass" TeamPass@noreply.github.com
Cc: "Nils Laumaillé" nils@teampass.net
Envoyé: Mercredi 12 Septembre 2012 14:45:35
Objet: Re: [TeamPass] Download File/Image returns zero-length data (#129)

The upload to the database will be a great feature.
When would you thing the 2.2 will be released?

Nils, thanks for the work and for this great tool


Reply to this email directly or view it on GitHub .

@lexa2
Copy link

lexa2 commented Dec 21, 2012

Well, it seems that this issue is still not fixed as of version that is currently available in Git repo. It could be "fixed" by adding:

if (!isset($_GET['sub'])) $_GET['sub'] = "upload";

into downloadFile.php right before the readfile() call.

But looking at the downloadFile.php code makes me fear about having THAT insecure software installed on the server - and by "that insecure" I mean TeamPass. Using unfiltered user input in filesystem-related operations is extremely dangerous. One could easily fetch any file from server apache user has access to by passing required filename in "sub" and terminating it with zero byte. readfile would simply ignore rest of the string and attacker would be able to read any file he/she wants, including settings.php and other important files that should be kept secure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants