opencaching / opencaching-pl

The source code of Opencaching.PL (and some other domains)
https://opencaching.pl/
GNU General Public License v3.0
22 stars 33 forks source link

oc_dynamic and security #173

Open GoogleCodeExporter opened 8 years ago

GoogleCodeExporter commented 8 years ago
oc_dynamic is a clever way of dealing with dynamically generated files ouside 
the application code (maintained by SVN automatic updates). 

The fact that user input and user upload can influence the contents of data 
stored in these directories raises security concerns. I will try to deal 
analyze each of them as far as my understanding of the site is at this time. 

I will also analyze from the perspective of forensic analysis I've performed on 
several different PHP application websites that were penetrated. 

Original issue reported on code.google.com by andrixnet on 10 Nov 2014 at 9:33

GoogleCodeExporter commented 8 years ago
oc_dyamic:

okapi-* = temporary files by OKAPI

@rygielski: should these files be readable to the outside world?

Original comment by andrixnet on 10 Nov 2014 at 9:35

GoogleCodeExporter commented 8 years ago
oc_dynamic/download/zip:

Temporary place where ZIP files are generated and offered for download. 

This directory should be restricted to allow read of ZIP files only and disable 
PHP engine.

Original comment by andrixnet on 10 Nov 2014 at 9:37

GoogleCodeExporter commented 8 years ago
oc_dynamic/images :

User uploads (images). 

Besides sanity checks in PHP code, which might be circumvented, this directory 
should have PHP engine disabled and only allowed image file extensions to be 
read from it. 

Original comment by andrixnet on 10 Nov 2014 at 9:39

GoogleCodeExporter commented 8 years ago
oc_dynamic/lib/* :

Smarty templates compile dir (according to /lib/common.inc.php

Since PHP engine must run in this dir, it contains dynamically created files, 
it is writeable, further analysis must be done on how to secure it.

Original comment by andrixnet on 10 Nov 2014 at 9:43

GoogleCodeExporter commented 8 years ago
oc_dynamic/mp3: 

User uploads (audio files). 

Besides sanity checks in PHP code, which might be circumvented, this directory 
should have PHP engine disabled and only allowed audio file extensions to be 
read from it. 

Original comment by andrixnet on 10 Nov 2014 at 9:44

GoogleCodeExporter commented 8 years ago
> okapi-* = temporary files by OKAPI
> @rygielski: should these files be readable to the outside world?

Via HTTP? No. These are used internally only.

Original comment by rygielski on 10 Nov 2014 at 9:48

GoogleCodeExporter commented 8 years ago
oc_dynamic/searchdata: 

This is a bit tricky. Application generates some data related to searches, then 
/lib/mapper_okapi.php and /lib/xmlmap.php use it to perform SQL query "LOAD 
DATA INFILE". 

This has the additional __UNDOCUMENTED__ requirement that directory permissions 
and file permissions allow the UNIX user of the database to read the generated 
files. 
What it means is that the entire directory path up to this directory must have 
at least world execute permission and the files generated and saved in this 
directory must be world readable. 

webserver should not allow read from this directory at all. 

Original comment by andrixnet on 10 Nov 2014 at 9:50

GoogleCodeExporter commented 8 years ago
oc_dynamic/tmp/importcaches: 

temporary place for XML files handled by /util.sec/importcaches/import.php

Based on this assessment, it should only be accessible to local server and PHP 
engine should be disabled.

Original comment by andrixnet on 10 Nov 2014 at 9:53

GoogleCodeExporter commented 8 years ago
oc_dynamic/tmp: 

temporary place where some images are created, notably QR code. 

PHP engine should be disabled. 
Only allowed file types must be served by webserver.

Original comment by andrixnet on 10 Nov 2014 at 9:54

GoogleCodeExporter commented 8 years ago
oc_dynamic/tpl/stdstyle/html: 

cached data genereted for templates. 

Since PHP engine must run in this dir, it contains dynamically created files, 
it is writeable, further analysis must be done on how to secure it.

Original comment by andrixnet on 10 Nov 2014 at 9:55

GoogleCodeExporter commented 8 years ago
oc_dynamic/wigo:

I found no reference in the code to explain this directory. 

Original comment by andrixnet on 10 Nov 2014 at 9:56

GoogleCodeExporter commented 8 years ago
Spam spam spam. You could have written all this in one comment, you know :)

Original comment by rygielski on 10 Nov 2014 at 9:59

GoogleCodeExporter commented 8 years ago
    Alias /images/uploads/      /srv/ocpl/oc_dynamic/images/uploads/
    Alias /uploads/             /srv/ocpl/oc_dynamic/images/uploads/
    Alias /upload/              /srv/ocpl/oc_dynamic/images/uploads/
    Alias /wigo/                /srv/ocpl/oc_dynamic/wigo/
    Alias /download/            /srv/ocpl/oc_dynamic/download/
    Alias /images/statpics/     /srv/ocpl/oc_dynamic/images/statpics/
    Alias /images/mini-mapa/    /srv/ocpl/oc_dynamic/images/mini-mapa/
    Alias /mp3/                 /srv/ocpl/oc_dynamic/mp3/
    Alias /tmp/                 /srv/ocpl/oc_dynamic/tmp/

These are the aliases defined in server config. 

While not all paths under oc_dynamic/ are explicitely exported by the 
webserver, the fact that PHP code is run in at least some of them, the fact 
that malicious code could be injected and run remains a real possibility and a 
potential security risk. 

I admit that the spread of OPENCACHING-PL code is tiny (3 sites) and the 
complexity of a potential exploit is unknown (possibly high). 
Yet the risk exists. 

This is why I have done this analysis and I hope we can do something to reduce 
the risks.

Original comment by andrixnet on 10 Nov 2014 at 10:05

GoogleCodeExporter commented 8 years ago
@rygielski: :-) I can understand the feeling regarding the email notifications. 
However I have found that is easier (and preferable) to treat each subsection 
separately, to be able to refer to it individually.

Regarding comment #6: oc_dynamic/ is not explicitly exported with an alias 
(even though subdirectories inside it are), thus files here should not be 
visible via HTTP request.

Consequently the following should be enough, and only enabled in the 
directories that actually need it.
php_flag engine off

Original comment by andrixnet on 10 Nov 2014 at 10:12

GoogleCodeExporter commented 8 years ago

Original comment by wloczynutka on 10 Nov 2014 at 10:43

GoogleCodeExporter commented 8 years ago
Haha. If we need to refer to them individually, then perhaps the best choice 
would be to report each of these subsections as a separate issue ;)

Original comment by rygielski on 10 Nov 2014 at 10:49

GoogleCodeExporter commented 8 years ago
1. I agree with andrixnet about the level of splitting information (each 
directory per comment) and synthesis them (one issue).
2. Regarding #11, this is a folder where OC admins place WIGO files, when 
requested by users. Maybe upload of those files should be automated, like 
upload of images.

Take a look at http://opencaching.pl/viewcache.php?cacheid=34806

Original comment by boguslaw...@gmail.com on 5 Dec 2014 at 11:11