Make WordPress Core

Opened 8 years ago

Closed 5 years ago

#36342 closed defect (bug) (wontfix)

No check to validate supplied author in export_wp()

Reported by: themiked's profile theMikeD Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: Priority: normal
Severity: normal Version: 3.1
Component: Export Keywords: has-patch
Focuses: Cc:

Description (last modified by DrewAPicture)

One of the options for export_wp() is to filter by author, but that filter option is not validated, it's used verbatum in the wpdb call. This should be validated first no?

Attachments (1)

36342.diff (1.4 KB) - added by Mte90 6 years ago.

Download all attachments as: .zip

Change History (10)

#1 @DrewAPicture
8 years ago

  • Description modified (diff)
  • Summary changed from No check to valide supplied author in wp_export() to No check to validate supplied author in export_wp()

#2 @chriscct7
8 years ago

  • Keywords 4.6-early added
  • Version trunk deleted

#3 @SergeyBiryukov
8 years ago

  • Keywords needs-patch added

#4 @netweb
8 years ago

  • Version set to 3.1

@Mte90
6 years ago

#5 @Mte90
6 years ago

  • Keywords has-patch added; needs-patch removed

We used this ticket for the Italian core-help meeting for a live coding.
After an analysis we saw that https://codex.wordpress.org/Class_Reference/wpdb#Protect_Queries_Against_SQL_Injection_Attacks prepare already sanitize the data.
Also there was other parameters that wasn't sanitized, in any case we wasn't sure if this ticket is still valid but was an easy interesting example about how to do it a patch.
So if the patch is still valid, we done, in other case we got fun and learned more about the process (and this ticket can be closed) :-)

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


6 years ago

#7 @SergeyBiryukov
6 years ago

  • Milestone changed from Awaiting Review to 5.0
  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

#8 @johnbillion
5 years ago

  • Keywords 4.6-early removed
  • Milestone changed from 5.0 to 5.1

#9 @pento
5 years ago

  • Milestone 5.1 deleted
  • Resolution set to wontfix
  • Status changed from reviewing to closed

The issue isn't that esc_sql() needs to be run on these arguments, it's that wp_export() doesn't check if the author exists before adding it to the query.

I don't think it's really necessary, if they attempt to export an author that doesn't exist (or doesn't have any posts), it'll just return an empty export.

Note: See TracTickets for help on using tickets.