WordPress.org

Make WordPress Core

Opened 7 months ago

Closed 5 months ago

Last modified 5 months ago

#40085 closed defect (bug) (fixed)

Audio/video uploads are broken in 4.2.13 and 4.3.9

Reported by: uzegonemad Owned by: joemcgill
Milestone: 4.7.4 Priority: normal
Severity: blocker Version: 3.7.19
Component: Media Keywords: has-patch fixed-major
Focuses: Cc:

Description (last modified by SergeyBiryukov)

As the title states, audio/video uploads are broken in the 4.2.13 and 4.3.9 security releases.

These releases backport a function for validating metadata, wp_kses_post_deep, but this function wasn't introduced until 4.4.

Uploading an audio/video file will respond with an undefined function error.

Relevant 4.2 commit: [40148]

Relevant 4.3 commit: [40153]

Trac doesn't let me choose 4.2.13, so I've filed this against trunk.

Attachments (2)

40085.diff (1.0 KB) - added by joemcgill 7 months ago.
Media Library mp3 upload.png (100.1 KB) - added by lukecavanagh 6 months ago.
mp3 Media Library patch fix test

Download all attachments as: .zip

Change History (41)

#1 @SergeyBiryukov
7 months ago

  • Description modified (diff)
  • Milestone changed from Awaiting Review to 4.7.4

Hi @uzegonemad, welcome to Trac! Thanks for the report.

I can confirm the issue. [40148] was backported to all branches from 3.7 to 4.3, so they all appear to be affected.

#2 @uzegonemad
7 months ago

I just noticed that as well. It looks like this affects the following releases:

  • 3.7.19
  • 3.8.19
  • 3.9.17
  • 4.0.16
  • 4.1.16
  • 4.2.13
  • 4.3.9

This ticket was mentioned in Slack in #forums by sergey. View the logs.


7 months ago

#5 @johnbillion
7 months ago

  • Keywords needs-patch added
  • Severity changed from normal to blocker
  • Version changed from trunk to 3.7.19

#6 @joemcgill
7 months ago

  • Owner set to joemcgill
  • Status changed from new to assigned

Initially thought we could simply backport both wp_kses_post_deep() and map_deep() to these branches, but am thinking we should revert and use a more targeted approach than wp_kses_post_deep() (see: @ocean90 's comment on 40075).

Also need to investigate why we aren't seeing failing tests on these branches and make sure we have coverage for these code paths.

@joemcgill
7 months ago

#7 @joemcgill
7 months ago

  • Keywords has-patch 2nd-opinion added; needs-patch removed

40085.diff moves the call to wp_kses_post() inside wp_add_id3_tag_data() which I think sanitizes all the metadata that might be used in a display context. This should also avoid #40075. Testing and feedback welcome.

This ticket was mentioned in Slack in #core by swissspidy. View the logs.


6 months ago

#9 @dlh
6 months ago

I tested 40085.diff with some audio files on the 4.3, 4.2, and 3.9 branches.

Although I can't speak intelligently about the use of wp_kses_post() itself in this context, I can at least report that the uploads succeeded, and the files and metadata seemed OK.

I would also note there was a PHP 7-specific fatal error in 3.9: Fatal error: 'break' not in the 'loop' or 'switch' context in /wp-includes/ID3/getid3.lib.php on line 285. It looks like that break was removed in [29734].

#10 @joemcgill
6 months ago

Thanks for testing, @dlh. I'm not sure we need to patch a PHP 7 specific bug as far back as 3.9, but it's good to have documented.

This ticket was mentioned in Slack in #core by swissspidy. View the logs.


6 months ago

This ticket was mentioned in Slack in #core-media by joemcgill. View the logs.


6 months ago

#13 @mikeschroder
6 months ago

Took a look at the patch.

This makes sense, but it'd be good to have someone familiar with ID3 like @wonderboymusic to take a look and make sure there isn't anything that wp_kses_post() sanitizes that commonly occurs in ID3 comments and would need to be preserved.

This ticket was mentioned in Slack in #core-media by joemcgill. View the logs.


6 months ago

#15 @lukecavanagh
6 months ago

@joemcgill

I tested 40058.diff and it does fix the issues in #40075. But a JPEG thumbnail image is still generated besides having the mp3 display with the cover artwork correctly in the media library.

@lukecavanagh
6 months ago

mp3 Media Library patch fix test

#16 @joemcgill
6 months ago

  • Keywords 2nd-opinion removed

Thanks @lukecavanagh. I think that may have been existing behavior so the image could be used as a post_thumbnail for the attachment page, but will have to investigate further.

#17 @lukecavanagh
6 months ago

@joemcgill 

Okay in that case, then it makes sense as to why the image would be generated.

This ticket was mentioned in Slack in #forums by sergey. View the logs.


6 months ago

#19 @SergeyBiryukov
6 months ago

Uploaded a workaround plugin for both this ticket and #40075, so that people on support forums could use it until the next minor release is available: https://wordpress.org/plugins/correct-audio-video-uploads/

The plugin is based on 40085.diff.

Last edited 6 months ago by SergeyBiryukov (previous) (diff)

This ticket was mentioned in Slack in #core-media by joemcgill. View the logs.


6 months ago

This ticket was mentioned in Slack in #core by swissspidy. View the logs.


6 months ago

This ticket was mentioned in Slack in #core by swissspidy. View the logs.


6 months ago

#23 @joemcgill
6 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 40400:

Fix broken audio/video functions when sanitizing ID3 data

This fixes a bug where running wp_kses_post_deep() on all the ID3
tag data corrupted blob data.

Fixes #40075, #40085.

#24 @joemcgill
6 months ago

  • Keywords fixed-major added
  • Resolution fixed deleted
  • Status changed from closed to reopened

#25 @joemcgill
6 months ago

[40400] needs to be backported back to the 3.7 branch.

This ticket was mentioned in Slack in #core-media by joemcgill. View the logs.


5 months ago

This ticket was mentioned in Slack in #core by swissspidy. View the logs.


5 months ago

#28 @swissspidy
5 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 40460:

Fix broken audio/video functions when sanitizing ID3 data

This fixes a bug where running wp_kses_post_deep() on all the ID3
tag data corrupted blob data.

Fixes #40075, #40085.

Merges [40400] to the 4.7 branch.

#29 @swissspidy
5 months ago

In 40461:

Fix broken audio/video functions when sanitizing ID3 data

This fixes a bug where running wp_kses_post_deep() on all the ID3
tag data corrupted blob data.

See #40075, #40085.

Merges [40400] to the 4.6 branch.

#30 @swissspidy
5 months ago

In 40462:

Fix broken audio/video functions when sanitizing ID3 data

This fixes a bug where running wp_kses_post_deep() on all the ID3
tag data corrupted blob data.

See #40075, #40085.

Merges [40400] to the 4.5 branch.

#31 @swissspidy
5 months ago

In 40463:

Fix broken audio/video functions when sanitizing ID3 data

This fixes a bug where running wp_kses_post_deep() on all the ID3
tag data corrupted blob data.

See #40075, #40085.

Merges [40400] to the 4.4 branch.

#32 @swissspidy
5 months ago

In 40464:

Fix broken audio/video functions when sanitizing ID3 data

This fixes a bug where running wp_kses_post_deep() on all the ID3
tag data corrupted blob data.

See #40075, #40085.

Merges [40400] to the 4.3 branch.

#33 @swissspidy
5 months ago

In 40465:

Fix broken audio/video functions when sanitizing ID3 data

This fixes a bug where running wp_kses_post_deep() on all the ID3
tag data corrupted blob data.

See #40075, #40085.

Merges [40400] to the 4.2 branch.

#34 @swissspidy
5 months ago

In 40466:

Fix broken audio/video functions when sanitizing ID3 data

This fixes a bug where running wp_kses_post_deep() on all the ID3
tag data corrupted blob data.

See #40075, #40085.

Merges [40400] to the 4.1 branch.

#35 @swissspidy
5 months ago

In 40467:

Fix broken audio/video functions when sanitizing ID3 data

This fixes a bug where running wp_kses_post_deep() on all the ID3
tag data corrupted blob data.

See #40075, #40085.

Merges [40400] to the 4.0 branch.

#36 @swissspidy
5 months ago

In 40468:

Fix broken audio/video functions when sanitizing ID3 data

This fixes a bug where running wp_kses_post_deep() on all the ID3
tag data corrupted blob data.

See #40075, #40085.

Merges [40400] to the 3.9 branch.

#37 @swissspidy
5 months ago

In 40469:

Fix broken audio/video functions when sanitizing ID3 data

This fixes a bug where running wp_kses_post_deep() on all the ID3
tag data corrupted blob data.

See #40075, #40085.

Merges [40400] to the 3.8 branch.

#38 @swissspidy
5 months ago

In 40470:

Fix broken audio/video functions when sanitizing ID3 data

This fixes a bug where running wp_kses_post_deep() on all the ID3
tag data corrupted blob data.

See #40075, #40085.

Merges [40400] to the 3.7 branch.

This ticket was mentioned in Slack in #core by swissspidy. View the logs.


5 months ago

Note: See TracTickets for help on using tickets.