8

Should get_template_directory_uri() be escaped using esc_url()?

I ask because taking an example from the default theme Twenty Fifteen the same function is used in two different places but only in one is it escaped.

wp_enqueue_style( 'twentyfifteen-ie', get_template_directory_uri() . '/css/ie.css', array( 'twentyfifteen-style' ), '20141010' );

-

<script src="<?php echo esc_url( get_template_directory_uri() ); ?>/js/html5.js"></script>

I would say that that get_template_directory_uri() does not need to be generated as the URL cannot be manipulated externally.

4
  • This question is very attractive, I am sure you will receive multiple answers @grappler.
    – prosti
    Dec 25, 2016 at 20:40
  • 2
    WordPress encourages late escaping, right before output. In the latter case this is being done because the function's output is displayed right away. In the latter case, the output is further processed by WP_Styles which does all the escaping (see WP_Styles::_css_href()).
    – swissspidy
    Dec 29, 2016 at 19:12
  • Even if you escape it what you will get? Escape on a malicious URL? Check my post @swissspidy.
    – prosti
    Dec 29, 2016 at 22:17
  • To be pedantic: The WP_Styles instance lives in a global variable (WordPress …) and can be replaced easily.
    – fuxia
    Dec 30, 2016 at 12:39

3 Answers 3

5

In that function we find a hook:

return apply_filters( 
    'template_directory_uri', 
    $template_dir_uri, 
    $template, 
    $theme_root_uri
);

So, yes, the URI can be changed by plugins, and you should escape its returned value.

The same principle applies to all WordPress URI functions, like get_home_url(), get_site_url() and so on. Keep in mind that there are not only good plugin developers out there. Some make mistakes, maybe just very small ones that happen only in some circumstances.

In case of wp_enqueue_style(), WordPress does escape the URL by default. But that is a wrapper for the global WP_Styles instance, and this in turn can be replaced easily – even with a less safe version. That's not very likely, but you should be aware of this possibility.

Unfortunately, WP itself doesn't follow the better safe than sorry directive. It doesn't even escape translations. My advice is not to look at the core themes for best practices. Always look at the source of the data and see if it can be compromised.

11
  • How do you propose that filter can be manipulated with get_template_directory_uri though? The entire function for reference: developer.wordpress.org/reference/functions/…
    – Jami Gibbs
    Dec 25, 2016 at 12:53
  • The filter's paramaters are set within the function itself: $template = str_replace( '%2F', '/', rawurlencode( get_template() ) ); $theme_root_uri = get_theme_root_uri( $template ); $template_dir_uri = "$theme_root_uri/$template";
    – Jami Gibbs
    Dec 25, 2016 at 12:56
  • @jami0821 A plugin can just write add_filter('template_directory_uri',function(){ return 'javascript:alert("Hacked!")'; });
    – fuxia
    Dec 25, 2016 at 13:06
  • I would love to be proven wrong here but the filter's parameters are being set within the function so how could that be overwritten by a plugin outside of its scope?
    – Jami Gibbs
    Dec 25, 2016 at 13:30
  • 1
    @MarkKaplun Addressed in my last edit.
    – fuxia
    Dec 30, 2016 at 14:30
3

Will try to make swissspidy's comment into an answer. Short version - it depends.

Escaping should not be applied randomly as double escaping might produce a url (or any kind of content) which do not match the intended url. Escaping should be applied only before output. Therefor the context is more important then the specific function that calculates the URL.

In your example, the first snippet just enqueues the URL and do not output it. The responsibility for escaping is delegated further into the wordpress stack to the code that actually output it, and that is the reason it is not escaped.

The second snippet does the output and that is why the url is being escaped.

So how do you know when you should not escape? Hopefully somewhere in the future the documentation of wordpress APIs will include that information, but for now either follow the full code path until the actual output, or test your theme under "funny" urls

0

Here is our function in question.

File: wp-includes/theme.php
320: /**
321:  * Retrieve theme directory URI.
322:  *
323:  * @since 1.5.0
324:  *
325:  * @return string Template directory URI.
326:  */
327: function get_template_directory_uri() {
328:    $template = str_replace( '%2F', '/', rawurlencode( get_template() ) );
329:    $theme_root_uri = get_theme_root_uri( $template );
330:    $template_dir_uri = "$theme_root_uri/$template";
331: 
332:    /**
333:     * Filters the current theme directory URI.
334:     *
335:     * @since 1.5.0
336:     *
337:     * @param string $template_dir_uri The URI of the current theme directory.
338:     * @param string $template         Directory name of the current theme.
339:     * @param string $theme_root_uri   The themes root URI.
340:     */
341:    return apply_filters( 'template_directory_uri', $template_dir_uri, $template, $theme_root_uri );
342: }
343: 

You write something like this in your plugin:

add_filter('template_directory_uri',function(){ return 'http://badserver.com/'; });

And you will have something like this on a website:

enter image description here

Bright is the future because I see a WP_Theme class doesn't implement the filter in the WP_Theme::get_template_directory_uri() method. We know slowly WordPress is getting better and better and more encapsulated.

File: wp-includes/class-wp-theme.php
898:    /**
899:     * Returns the URL to the directory of a theme's "template" files.
900:     *
901:     * In the case of a child theme, this is the URL to the directory of the
902:     * parent theme's files.
903:     *
904:     * @since 3.4.0
905:     * @access public
906:     *
907:     * @return string URL to the template directory.
908:     */
909:    public function get_template_directory_uri() {
910:        if ( $this->parent() )
911:            $theme_root_uri = $this->parent()->get_theme_root_uri();
912:        else
913:            $theme_root_uri = $this->get_theme_root_uri();
914: 
915:        return $theme_root_uri . '/' . str_replace( '%2F', '/', rawurlencode( $this->template ) );
916:    }

More notes

You see someone defines in the theme someting like this:

define( 'TEMPL_URI', get_template_directory_uri() );

They say it is because they would like to make 'TEMPL_URI' unfilterable. They are wrong. If a plugin set a bad filter this will happen before the theme, so the 'TEMPL_URI' will be dirty.

However, it is OK to define the constants, I just wanted to say the constant will not protect you.


Another thing I found in Twenty Seventeen. In there I saw get_theme_file_uri function ( @since 4.7 )

File: wp-includes/link-template.php
4026: /**
4027:  * Retrieves the URL of a file in the theme.
4028:  *
4029:  * Searches in the stylesheet directory before the template directory so themes
4030:  * which inherit from a parent theme can just override one file.
4031:  *
4032:  * @since 4.7.0
4033:  *
4034:  * @param string $file Optional. File to search for in the stylesheet directory.
4035:  * @return string The URL of the file.
4036:  */
4037: function get_theme_file_uri( $file = '' ) {
4038:   $file = ltrim( $file, '/' );
4039: 
4040:   if ( empty( $file ) ) {
4041:       $url = get_stylesheet_directory_uri();
4042:   } elseif ( file_exists( get_stylesheet_directory() . '/' . $file ) ) {
4043:       $url = get_stylesheet_directory_uri() . '/' . $file;
4044:   } else {
4045:       $url = get_template_directory_uri() . '/' . $file;
4046:   }
4047: 
4048:   /**
4049:    * Filters the URL to a file in the theme.
4050:    *
4051:    * @since 4.7.0
4052:    *
4053:    * @param string $url  The file URL.
4054:    * @param string $file The requested file to search for.
4055:    */
4056:   return apply_filters( 'theme_file_uri', $url, $file );
4057: }

Again this filter theme_file_uri may impose a security problem like @toscho explained for get_template_directory_uri.

Conclusion

When the security is in question we need to be very careful. The problem in here is deeper than just escaping the single URL. It considers the whole WordPress plugins security model.

Plugins do not need get_template_directory_uri() function and the filter in there to do the bad things. They can execute with the same privileges as the WordPress core — they can do anything.

Malicious JavaScript that bad plugins may inject may read your passwords while you type.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.