[VULNERABILITY] Authenticated file upload vulnerability - [leading to shell command execution]

[1337kid]

Overview

By default, an authenticated user cannot upload a PHP file due to mime type checks but it can be bypassed by adding a GIF header at the beginning of the PHP file. "finfo" considers a PHP file with GIF header as a GIF file instead of PHP.

// shell.php
GIF89a;
<?php system($_GET["cmd"]); ?>

The above PHP code can be uploaded, bypassing mime type checks.

Command execution

Possible fix

Add a file extension checking mechanism to prevent upload of PHP file ..etc

[shoaiyb]

Hey there,

Thanks for reaching out and bringing this issue to my attention.

This issue initially seemed like a self-attack to me when I was first writing the code, but now I am convinced that an innocent authenticated user could potentially upload the file.

I have identified two to three possible ways to reproduce this issue, including using .htaccess, the PHP runtime configuration, and the file extension checking.

I have tested all these methods, and they all worked for me. However, I'm unsure about the best approach or whether they will work on every installation.

---

The extension checking mechanism will prevent any type of file without extension from being uploaded, including text files. The file extension will be the same as the file name if it has no extension. So, unless the file name is in the list of whitelisted extensions, it will never be uploaded.

$extension = explode( '.', $basename );
$extension = strtolower( end( $extension ) );
$extensions = $this->_l( 'media_extension',
  array(
    // Extensions whitelist
  )
);
if ( ! in_array( $extension, $extensions ) ) {
  $msg = 'File extension not allowed';
  return false;
}

However, I have considered files without extensions and thought about checking if a file is a plain text file and allowing the upload that way.

$extension = explode( '.', $basename );
$extension = strtolower( end( $extension ) );
$extensions = $this->_l( 'media_extension',
  array(
    // Extensions whitelist
  )
);
if ( ! in_array( $extension, $extensions ) ) {
  if ( $extension !== $basename || 'text/plain' !== $type ) {
    $msg = 'File extension not allowed';
    return false;
  }
}

---

Disabling script execution using .htaccess is more of a blacklisting approach rather than whitelisting, so there is no guarantee that the blacklisted scripts are the only scripts that can be uploaded.

AddHandler cgi-script .php .pl .jsp .asp .sh .cgi
Options -ExecCGI

---

[1337kid]

The one with extension whitelist looks fine. If the user indeed wants to upload codes, an extension changing mechanism can be induced. For example: script.php to script.php.txt or script_php.txt

[shoaiyb]

The one with extension whitelist looks fine. If the user indeed wants to upload codes, an extension changing mechanism can be induced. For example: script.php to script.php.txt or script_php.txt

Do you agree with the "checking if a file is a plain text file and allowing the upload" way?

[1337kid]

Yes, I agree with that. It's a good idea.

[shoaiyb]

Yes, I agree with that. It's a good idea.

Good 👍
The fix will soon be at the develop branch for additional testing.

One more thing, can you please take a look and see if any of the default allowed MIME types should be removed?

[1337kid]

Alright 👍
Lemme check the mime types

[1337kid]

One more thing, can you please take a look and see if any of the default allowed MIME types should be removed?

A PHP file with random bytes on the first line will get uploaded.
example:

// file.php
asdfghj34567yhjndtetstetlhbkERFVj65678;
<?php system($_GET['cmd']); ?>

This file will bypass mime type checks. I think this file is categorized as application/octet-stream by finfo due to random data at the beginning.
Btw, there is no point in removing application/octet-stream . Any file with random data will be considered as application/octet-stream and file need not be an executable. Adding an extension whitelist is the better choice.

[shoaiyb]

Yeah, the extension checking will also fix that.

Anyway, below are the extension to be whitelisted:

avi
avif
css
doc
docx
flv
gif
htm
html
ico
jpeg
jpg
kdbx
m4a
mkv
mov
mp3
mp4
mpg
ods
odt
ogg
ogv
pdf
png
ppt
pptx
psd
rar
svg
txt
xls
xlsx
webm
webp
wmv
zip

[1337kid]

The whitelist looks clean. 👍