p410n3 / YDL-UI

Minimalistic, portable and selfhosted Interface for the famous youtube-dl CLI tool.
The Unlicense
82 stars 8 forks source link

dl.php needs security update #10

Closed irfantogluk closed 6 years ago

irfantogluk commented 6 years ago

Other folders can be reached via dl.php

image

p410n3 commented 6 years ago

Thanks a alot! This definitely sucks and will be fixed when I am home. (Or you make a pull request). Stripping / should work but I need to test first.

irfantogluk commented 6 years ago

Only stripping / will not enaugh I think. We should check strlen or regex of folder value. If it's empty like dl.php?folder= it will show us the main folder of the YDL-UI I'm at work now, if I'll back home before you, I'll make a pull request

p410n3 commented 6 years ago

True.

I also thought about creating an auth file that contains the $md5_date aka the Folder Name.

If the script does not find a file named .auth-token (or whatever name) with the Content of it being the $_GET['folder'] variable, it will die();

What do you think?

irfantogluk commented 6 years ago

You have done the whole project without being connected to a DB or DB-like file. I think an update like this be good. if (!preg_match('/^[a-f0-9]{32}$/', $_GET['folder']) die(); However, this may not be enough for some future updates.

p410n3 commented 6 years ago

Looks good. Thanks for the help.

p410n3 commented 6 years ago

Ok i fixed the issue. Your solution did however have one issue. If someone entered a folder var that didnt exists but matched the regex he would've seen the current working dir. I have implemented an additional check to see if the folder we try to go to actually exists.

If its not too much asked, I would lijke you to test first before I make a new Release.

Thanks for contributing 😄

irfantogluk commented 6 years ago

Tested and it works now. Thanks for fixing.

p410n3 commented 6 years ago

Thanks for finding it!