sebsauvage / MinigalNano

GNU Affero General Public License v3.0
161 stars 50 forks source link

Problem if `..` in dir name #89

Closed Niols closed 9 years ago

Niols commented 9 years ago

Hello,

I'm using .. in my dir names for days ranges ((2014-09-01..08)+Super+Party), and I can't access my directory.

It comes from this part of the code I guess (in index.php, line 158) :

$requestedDir = '';
if (!empty($_GET['dir']))
    $requestedDir = $_GET['dir'];
$thumbdir = rtrim('photos/' . $requestedDir, '/');

//$thumbdir = str_replace('/..', '', $thumbdir); // Prevent directory traversal attacks.
if (strstr($thumbdir, '..') !== false) {
    $requestedDir = '';
    $thumbdir = rtrim('photos/', '/');
}

Maybe we could find a way to change this test (strstr($thumbdir, '..') !== false) without lowering the security ? Something like strstr($thumndir, '/..') !== false ?

Niols

tmos commented 9 years ago

I have to admit that this security fix was quite important, and I wasn't involved in it, so I can't really say if /.. is less safe than ... Maybe someone with more experience on this type of topic on this may help ?

In my opinion, if this is not very possible, I think we can consider this as very specific, and don't fix it.

tmos commented 9 years ago

@Aldarone just got that fixed !

Aldarone commented 9 years ago

Humpf, rouvre la, j'avais pas pensé aux liens symboliques et ma correction empêche leur usage.

tmos commented 9 years ago

Raté, ça m'apprendra à aller trop vite !

Niols commented 9 years ago

Hehe, on ne peut pas tout avoir du premier coup. Merci @Aldarone et @tmos ! :)