4

ms-files.php contains the following code fragment:

$file = rtrim( BLOGUPLOADDIR, '/' ) . '/' . str_replace( '..', '', $_GET[ 'file' ] );
if ( !is_file( $file ) ) {
    status_header( 404 );
    die( '404 — File not found.' );
}

This code determines whether or not the file can be found. However, it does something that I find... Odd. It removes all double-periods ("..") from the string. Normally, this doesn't matter, but we've had users upload files with two periods in a row (sigh), causing this portion of the code to incorrectly report code 404 (not found).

I'm tempted to remove the strange double-period-removing nature of this code, but I fear that I'll inadvertently break something else. Why is this functionality present as it is coded?

Thanks! :)

Edit: To be clear, it would appear that double-periods are NOT invalid in Wordpress, but they can't be served up by this bit of code... So, they're not invalid, but when accessed via this mechanism, 404 is incorrectly reported.

Improve this question
5
  • en.wikipedia.org/wiki/Directory_traversal_attack
    – s_ha_dum
    Commented Jan 23, 2014 at 1:12
  • 1
    I just came back to answer my own question with that. Ha!
    – rinogo
    Commented Jan 23, 2014 at 1:12
  • The question still remains, though - Why does Wordpress allow us to upload files with filenames of this format if it will refuse to serve them up later? Perhaps it's simply a defect?
    – rinogo
    Commented Jan 23, 2014 at 1:13
  • 1
    A file name with double periods is not in of itself a security risk or invalid.
    – Wyck
    Commented Jan 23, 2014 at 1:20
  • 1
    @Wyck: True, but I am fairly sure that security is the reason for stripping the double periods nonetheless. (I have seen this in other applications.) One might argue that better regex is in order though.
    – s_ha_dum
    Commented Jan 23, 2014 at 15:05

1 Answer 1

Reset to default
4

Looks like this is a defect. Ish.

As @s_ha_dum mentioned, this is to prevent a directory traversal attack. However, Wordpress' Media Library will happily let you upload a file with two or more periods in a row, even if ms-files.php will refuse to serve it up.

So, nothing is technically "broken", but this certainly isn't ideal. There's a patch to prevent this, but it looks like it was never actually added to the core:

https://core.trac.wordpress.org/attachment/ticket/16189/check%20for%20double%20periods.diff

More info on this defect: https://core.trac.wordpress.org/ticket/12756

Improve this answer

Your Answer

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

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