WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 3 years ago

Last modified 13 months ago

#40133 closed defect (bug) (worksforme)

Adding nonce for additional fields when hooking to attachment_fields_to_edit causes media library grid view not to work

Reported by: dingo_d Owned by:
Milestone: Priority: normal
Severity: normal Version: 4.7.3
Component: Media Keywords:
Focuses: Cc:

Description

When I'm adding additional fields to attachment_fields_to_edit hook, to show them on 'Attachment Details' screen, if I want to add a nonce field to ensure the security when saving these new fields, the media screen just hangs, and the loader just spins on and on.

I'm not getting any error in my error log, so I'm not quite sure why this happens.

I tried adding it as just

wp_nonce_field( 'new_attachment_fields', 'new_attachment_fields_nonce' );

Or as this

$form_fields['nonce']['label'] = '';
$form_fields['nonce']['input'] = 'html';
$form_fields['nonce']['html'] = wp_nonce_field( 'new_attachment_fields', 'new_attachment_fields_nonce' );

but in each time the loader hangs, and I cannot see any images in the grid view, and access the 'Attachment Details' screen.

Now I can add the fields without nonce, but how safe is that?

I'm updating post meta of the attachment, using update_post_meta() function, which properly sanitizes everything before writing it to the database, so is there a need to worry about this, or not?

Technically you can enable the 'Attachment Details' to appear on the front end using wp.media handler (for whatever reason), which exposes it to the public.

Is it necessary to add the nonce check before updating post meta, or am I being overly cautious/paranoid? :D

Change History (6)

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


3 years ago

#2 @antpb
3 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to invalid
  • Status changed from new to closed

hello @dingo_bastard Thanks so much for your ticket! This is fixed in a later version of WordPress than the one reported from (4.7.3) and a nonce is now passed to ajax action. @joemcgill has confirmed in our recent media component meeting that this issue is no longer valid. Going to go ahead and close this out. :)

#3 @antpb
3 years ago

  • Resolution changed from invalid to fixed

#4 @dingo_bastard
3 years ago

Thanks for letting me know @antpb :)

#5 @dd32
3 years ago

  • Reporter changed from dingo_bastard to dingo_d

#6 @SergeyBiryukov
13 months ago

  • Resolution changed from fixed to worksforme

Just changing the resolution to worksforme for accuracy, as it's not quite clear when or where exactly this was fixed.

Note: See TracTickets for help on using tickets.