shell_exec url

[Nibbels]

https://github.com/e-alfred/ocdownloader/blob/fac43d42e8249ca06f3e9003b5355dd88d491bcb/controller/lib/youtube.php#L51

$Output = shell_exec ($this->YTDLBinary . ' -i \'' . $this->URL . '\' --get-url --get-filename' . ($ExtractAudio ? ' -f bestaudio -x' : ' -f best') . ($this->ForceIPv4 ? ' -4' : '') . (is_null ($Proxy) ? '' : $Proxy));

Just to have a look at such: $this->URL is put to shell_exec. The question I ask : Is this command somehow exploitable or might this code be a problem somehow? Is this URL filtered with care?

Rightnow I only checked the surface. Dont nail me down if this hint is absolutely bogus for some reason. Same for Proxy.

[Nibbels]

Here is a point of calling /../../SERVER/fallback.sh with url-encoded url: https://github.com/e-alfred/ocdownloader/blob/master/controller/lib/curl.php#L117

    /********** PRIVATE STATIC METHODS **********/
    private static function Run ()
    {
        shell_exec (rtrim (dirname (__FILE__), '/') . '/../../SERVER/fallback.sh "' . self::$GID . '" "' . urlencode (self::$URI) . '" "' . urlencode (json_encode (self::$OPTIONS, JSON_HEX_APOS | JSON_HEX_QUOT)) . '"');
    }

[Nibbels]

CURL::AddUri ($_POST['FILE'], $OPTIONS) -> No sanitize: https://github.com/Nibbels/ocdownloader/blob/master/controller/lib/curl.php#L19 -> self::run(): https://github.com/Nibbels/ocdownloader/blob/master/controller/lib/curl.php#L115

If $_POST['FILE'] is not sanitized somehow anything might go through? -> But it is url-encoded.

we have https://github.com/Nibbels/ocdownloader/blob/master/controller/lib/tools.php#L18

    public static function CheckURL ($URL)
    {
        $URLPattern = '%^(?:(?:https?|ftp)://)(?:\S+(?::\S*)?@|\d{1,3}(?:\.\d{1,3}){3}|(?:(?:[a-z\d\x{00a1}-\x{ffff}]+-?)*[a-z\d\x{00a1}-\x{ffff}]+)(?:\.(?:[a-z\d\x{00a1}-\x{ffff}]+-?)*[a-z\d\x{00a1}-\x{ffff}]+)*(?:\.[a-z\x{00a1}-\x{ffff}]{2,6}))(?::\d+)?(?:[^\s]*)?$%iu';
        preg_match ($URLPattern, $URL, $Matches);
        if (count ($Matches) == 1)
        {
            return true;
        }
        return false;
    }

which is used on every location of "add". But I still dont like using $_POST at all.

I will review this after some sleep. Real tests other than reading just code, tips and hints are welcome!

[Nibbels]

I think we should search vor a match-function with multibyte support? http://php.net/manual/en/function.mb-ereg.php

I dont know how preg_match works but preg_replace is absolute bogus if the "search for chars" are multibyte.

[Loki3000]

I think we should search vor a match-function with multibyte support?

...)?$%iu'

"u" modificator means "support unicode" http://php.net/manual/en/reference.pcre.pattern.modifiers.php

[Nibbels]

I have this onlinegame and I wanted to prevent symbols to go into the database. It all went fine when I just preg_replaced all chars I did not like to emptys. So everything at usernames and Ids got stripped at the entrance.

That worked as long I did not include öäüß into the search-for-replace pattern. those turned out to be multibyte. With those in search pattern kind of nothing was stripped anymore.

Later I found out about mb_FUNCTIONS.

Nowadays I use them where I want to be safe. And of course: Nowadays mysqli/prepared is the better choice!!

Thatwhy my first tip was http://php.net/manual/en/function.mb-ereg-match.php instead of preg_match. But I am not very familiar with that and would have to learn first.

[Loki3000]

I think that possible solution is to transfer base64 encoded parameters through command line and decode it inside script. (it's for fallback.sh).

For youtube-dl url could be totally checked: it has only two possible parameters (video id and playlist id), that has strict naming rules.