Waaaaat / yt-dl-webui

Youtube Downloader WebUI
0 stars 0 forks source link

shellcode injection, suggestion for improvement #1

Open adeliktas opened 4 years ago

adeliktas commented 4 years ago

looking at the code im seeing no sanitization of user input therefore a shell code injection will high likely be possible. Also variables(paths) can be relative instead of hardcoding it best regards

Waaaaat commented 4 years ago

looking at the code im seeing no sanitization of user input therefore a shell code injection will high likely be possible. Also variables(paths) can be relative instead of hardcoding it best regards

@adeliktas thanks for the useful input, i have absolutly no knowhow about php and i appreciate your comment i will learn how to do it and try to add it to the code - it my take some time

Waaaaat commented 4 years ago

@adeliktas Hi there, a long time ago you wrote a comment, i tried to add the sanitization but i wasn't abe, every time i tryied the new variable was just blank, but i was able to ad an validation of the url. Do you have any idea why it could have ended up blank? Or am I just too stupid?

adeliktas commented 4 years ago

@adeliktas Hi there, a long time ago you wrote a comment, i tried to add the sanitization but i wasn't abe, every time i tryied the new variable was just blank, but i was able to ad an validation of the url. Do you have any idea why it could have ended up blank? Or am I just too stupid?

First of all i used the wrong term to describe the issue. I think you would call it a remote code execution by unsanitized user input.

Your vulnerability lies within $DLURL or the user input being part of the execution to be precise: exec("cd $FDL && youtube-dl $DLParameters $DLFormat $DLURL");

Generally you should try to reduce using exec (shell execution) and use existing php functions instead, which could help to make the code cross platform. Since you want to use the youtube-dl Python tool this will remain the same. But for Example mkdir/(fopen/fwrite) to create a file/folder instead of exec ("mkdir -p $FODL");

Also there is many things to consider for sanitization, like how it is exploitable by using certain characters or code. But due to it's complexitiy i do not recommend to try to simply sanitize it yourself, but rather use existing and maintained php input sanitization libraries.

Even though im saying that i didnt make the effort to do so for some other simple tools of mine. I used the second best method in my opinion to only allow a chosen set of characters to be used as input. In the case that you will use this method just for youtube/urls you could simply use regex with preg_replace to allow only whitelisted characters. So in your case it would probably be the characters [a-zA-Z0-9:/?=&.] $var = preg_replace("/[^a-zA-Z0-9\:\/\?\=\&.]+/", "", $var);

<input type="text" name="URL" placeholder="Please Enter the URL of your Youtube Video">
Change type to url:
<input type="url" name="URL" placeholder="Please Enter the URL of your Youtube Video">

Also im not currently using your tool/code so i can't debug it with php tools and detecting it within the code would a bit tedious or not possible if the problem is within the configuration of php or missing php libraries.

Try to narrow down which method is causing the issue. Check the Data of your variables and what is the url of the blank page?