OddOneOut / bwp-minify

A WordPress Minification plugin that relies on Minify PHP library and WordPress's enqueueing system to serve minified CSS and JS to your audience
http://betterwp.net/wordpress-plugins/bwp-minify/
49 stars 23 forks source link

Directory traversal "../" in minify path is incorrectly changed to "./" #65

Closed dave-newson closed 8 years ago

dave-newson commented 8 years ago

When either a script or CSS URL includes directory traversal (../), the minifier gets passed an incorrect path, where "../" has been modified to "./"

Code

\BWP_MINIFY::process_media_source

Examples

process_media_source('http://example.com/plugin/assets/js/derp/../main.js')
// Outcome: plugin/assets/derp/js/./main.js
// Desired: plugin/assets/js/main.js
process_media_source('http://example.com/plugin/assets/js/derp/derp/../../main.js')
// Outcome: plugin/assets/js/derp/derp/././main.js
// Desired: plugin/assets/js/main.js

Possible solution

process_media_source already does some hairy stuff with the path string, so why not try resolving the traversal with regex: http://stackoverflow.com/questions/20522605/what-is-the-best-way-to-resolve-a-relative-path-like-realpath-for-non-existing

    // ~line 2680
    // Regex for resolving relative paths
    $regex = '#\/*[^/\.]+/\.\.#Uu';
        while (preg_match($regex, $src)) {
        $src = preg_replace($regex, '', $src);
    }

Still goes south if you try to traverse higher than the root, but that's probably OK as its a bad URL anyway:

$this->process_media_source('http://example.com/../../main.js')
// Outcome: ././main.js

Alternatively before the call to $src = str_replace('./', '/', $src);, replace all instances of '../' with '' (empty string), like what happpens with the real URL.

kminh commented 8 years ago

This is actually a duplicate of https://github.com/OddOneOut/bwp-minify/issues/50 , so I will close this issue for now. If you want to reopen, let me know.