Make WordPress Core

Opened 4 years ago

Closed 8 months ago

Last modified 6 months ago

#54078 closed defect (bug) (wontfix)

Underscore appended to media file on upload

Reported by: spielautomat4's profile spielautomat4 Owned by:
Milestone: Priority: normal
Severity: normal Version: 5.8
Component: Upload Keywords: dev-feedback
Focuses: administration Cc:

Description

I noticed that a random underscore is appended to media files, when uploading them in an article. Im using the Classic Editor.

The original file name was:
AB-LET.2018.133.AXH1.jpg

Once uploaded, it became:
AB-LET.2018.133.AXH1_.jpg

There was no prior file uploaded with that name (at least the media gallery does not find any).

Change History (6)

#1 follow-ups: @karpstrucking
4 years ago

  • Keywords reporter-feedback added; needs-patch removed

@spielautomat4 is this happening for all images or just this one in particular? if just this one, can you try renaming the file locally before uploading?

#2 in reply to: ↑ 1 @spielautomat4
4 years ago

Replying to karpstrucking:

@spielautomat4 is this happening for all images or just this one in particular? if just this one, can you try renaming the file locally before uploading?

Other files upload properly, there no underscore will be appended.

I suppose, it's due to the periods and/or trailing number in the file name.

#3 in reply to: ↑ 1 @spielautomat4
4 years ago

I have done some more testing and can confirm, that it's due to the periods in the file name.

The trailing number is no problem. I have tried with another WordPress installation as well.

Test case 1:
AB-LET.2018.133.AXH1.jpg -> AB-LET.2018.133.AXH1_.jpg [FAULTY]

Test case 2:
AB-LET.2018.133.AXH.jpg -> AB-LET.2018.133.AXH_.jpg [FAULTY]

Test case 3:
AB-LET_2018_133_AXH1.jpg -> AB-LET_2018_133_AXH1.jpg [OK]

Test case 4:
AB-LET_2018_133_AXH.jpg -> AB-LET_2018_133_AXH.jpg [OK]

Last edited 4 years ago by spielautomat4 (previous) (diff)

#4 @karpstrucking
4 years ago

  • Keywords dev-feedback added; reporter-feedback removed

It looks like this is actually an intended function of sanitize_file_name() (https://core.trac.wordpress.org/browser/tags/5.8/src/wp-includes/formatting.php#L2047)

	/*
	 * Loop over any intermediate extensions. Postfix them with a trailing underscore
	 * if they are a 2 - 5 character long alpha string not in the allowed extension list.
	 */
	foreach ( (array) $parts as $part ) {
		$filename .= '.' . $part;

		if ( preg_match( '/^[a-zA-Z]{2,5}\d?$/', $part ) ) {
			$allowed = false;
			foreach ( $mimes as $ext_preg => $mime_match ) {
				$ext_preg = '!^(' . $ext_preg . ')$!i';
				if ( preg_match( $ext_preg, $part ) ) {
					$allowed = true;
					break;
				}
			}
			if ( ! $allowed ) {
				$filename .= '_';
			}
		}
	}

#5 @callumbw95
8 months ago

  • Resolution set to wontfix
  • Status changed from new to closed

Hi All,
I have just taken a look into this, and it looks like the code hasn't changed around this from 5.8 to 6.8. I believe this is intended behaviour, as appending the underscore at the end of the filename stop's potential security risks:

The Security Risk: Double Extension Attacks 🦹

The primary threat is a malicious file disguised as a safe one. An attacker might upload a file named something like my-image.php.jpg.

The Intent: The attacker hopes the system will validate the file based on the final .jpg extension and classify it as a harmless image.
The Danger: However, some web servers (particularly older or misconfigured Apache servers) can be tricked. They might ignore the final .jpg and interpret the file based on the .php extension, allowing it to be executed as a server-side script. This would let the attacker run malicious code on your server.

The Solution: Neutralizing the Threat 🛡️

By programmatically inserting an underscore before the final extension, the system effectively defuses this threat.

Let's see how our example filename is transformed:

  • Original Malicious Filename: my-image.php.jpg
  • Sanitized Filename: my-image.php_.jpg

The sanitization logic identifies the potentially harmful .php part and changes it to .php_. This simple change completely breaks the attack vector. The server will no longer recognize .php_ as a valid executable extension. It will now only see the final, safe .jpg extension and treat the file correctly as an image.

In short, placing an underscore in that specific location surgically neutralizes the dangerous part of the filename without changing the final file type that the system and user expect. It's a targeted security fix for a well-known vulnerability.

As of such I don't believe we should change this functionality and instead this ticket can be closed. However if you do have any comments or would like to reopen the conversation please do.

#6 @peterwilsoncc
6 months ago

  • Milestone Awaiting Review deleted
Note: See TracTickets for help on using tickets.