WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 4 years ago

#5422 closed defect (bug) (fixed)

Sanitize plugin update information

Reported by: Viper007Bond Owned by: westi
Priority: normal Milestone: 2.8
Component: Security Version: 2.3.1
Severity: critical Keywords: needs-patch
Cc:

Description

See ​wp-hackers discussion.

The update data retrieved from WP.org is trusted to be safe and HTML encoded. We shouldn't make this assumption, plus we should to kses the plugin's name.

Attached is a proposed patch. Seems to work okay.

Attachments (6)

5422.patch​ (1.9 KB) - added by Viper007Bond 6 years ago.
5422.2.patch​ (2.4 KB) - added by hakre 4 years ago.
Patch against 2.8 bleeding
5422.3.patch​ (2.4 KB) - added by hakre 4 years ago.
wp_nonce_url did not need the attr()
5422.4.patch​ (2.4 KB) - added by hakre 4 years ago.
Fixed Patch
5442.post-11258.patch​ (2.0 KB) - added by hakre 4 years ago.
Some Fix for invalid entities and little code cleanup.
5442.post-11258.2.patch​ (2.0 KB) - added by hakre 4 years ago.
Some Fix for invalid entities and little code cleanup.

Download all attachments as: .zip

Change History (23)

Viper007Bond — 6 years ago

comment:1 westi — 6 years ago

  • Owner changed from anonymous to westi
  • Status changed from new to assigned

comment:2 Denis-de-Bernardy — 4 years ago

  • Component changed from Administration to Security
  • Milestone changed from 2.9 to 2.8

+1 to that. See also #7875

comment:3 hakre — 4 years ago

  • Keywords 2nd-opinion removed
  • Severity changed from normal to critical

+1. putput should be properly encoded / formatted! this is security related and solved, so please fix.

comment:4 DD32 — 4 years ago

  • Keywords dev-feedback added

This is also useful for if anyone decides to implement their own version checking, While WordPress trusts WordPress.org, It might not be the case that a non-dot-org update checker may not be as nice..

comment:5 hakre — 4 years ago

The patch is pretty old, I will create an update. attr() should be used.

hakre — 4 years ago

Patch against 2.8 bleeding

hakre — 4 years ago

wp_nonce_url did not need the attr()

comment:6 hakre — 4 years ago

#5422 fixed, kses added as well.

comment:7 Denis-de-Bernardy — 4 years ago

  • Keywords tested added; dev-feedback removed

patch applies cleanly. clean_url should be used on the urls. else good to go imo.

comment:8 Denis-de-Bernardy — 4 years ago

  • Keywords needs-patch added; has-patch tested removed
  • Milestone changed from 2.8 to Future Release

patch is broken

hakre — 4 years ago

Fixed Patch

comment:9 hakre — 4 years ago

  • Keywords has-patch added; needs-patch removed
  • Milestone changed from Future Release to 2.8

Please Check.

comment:10 Denis-de-Bernardy — 4 years ago

  • Keywords tested commit added

comment:11 ryan — 4 years ago

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

(In [11258]) Sanitize plugin update information. Props hakre, Viper007Bond. fixes #5422

hakre — 4 years ago

Some Fix for invalid entities and little code cleanup.

hakre — 4 years ago

Some Fix for invalid entities and little code cleanup.

comment:12 hakre — 4 years ago

& were not properly handeled. wp_nonce_url does not need it in input and the other three urls needed a esc_attr() to have them.

comment:13 hakre — 4 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

comment:14 Denis-de-Bernardy — 4 years ago

shouldn't clean_url() be used here, rather?

comment:15 Denis-de-Bernardy — 4 years ago

  • Keywords needs-patch added; has-patch tested commit removed

comment:16 hakre — 4 years ago

clean_url() can be used, esc_attr() does the job as well: see #9432 .

comment:17 azaozz — 4 years ago

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

(In [11376]) Sanitize plugin update information, props hakre, fixes #5422

Note: See TracTickets for help on using tickets.