WordPress.org

Make WordPress Core

Opened 10 years ago

Last modified 5 months ago

#11623 accepted defect (bug)

review options list and update sanitize_option()

Reported by: dd32 Owned by: dd32
Milestone: Future Release Priority: normal
Severity: normal Version: 2.9
Component: Security Keywords: needs-patch needs-unit-tests
Focuses: Cc:
PR Number:

Description

A lot of options have been added since 2.0.5, and as a result, not all of them have been added to sanitize_option()

Ideally, Options which are to be (int) or absint() should have a filter applied to them here.

Attached patch is for the first option thats brought this up, 'start_of_week' which is tested to be int in some function uses, ignored elsewhere.

I've set this to security as its preventive security..

Attachments (1)

11623.diff (384 bytes) - added by dd32 10 years ago.

Download all attachments as: .zip

Change History (11)

@dd32
10 years ago

#1 @hakre
10 years ago

  • Keywords dev-feedback added

You write "ideally", does that mean that a patch could reflect more options but you only but that one in from the other ticket?

#2 @nacin
10 years ago

  • Keywords has-patch dev-feedback removed

Yea, it looks like dd32 created this ticket with the idea of updating sanitize_option() with all options that should be sanitized but aren't, and he just started off with 'start_of_week' (which came out of #10397).

Looks like there are 94 options listed in schema.php and 39 options sanitized in sanitize_option(). The remaining 55 should be checked to see whether they should be sanitized as well, and I imagine we should also check the current 39 to ensure they are being sanitized properly.

#3 @nacin
10 years ago

  • Type changed from enhancement to defect (bug)

#4 @dd32
10 years ago

  • Keywords has-patch added
  • Milestone changed from 3.0 to 3.1
  • Owner changed from ryan to dd32
  • Status changed from new to accepted

#5 @nacin
10 years ago

  • Keywords early added

#6 @nacin
9 years ago

  • Keywords 3.2-early added; has-patch early removed
  • Milestone changed from Awaiting Triage to Future Release

#7 @chriscct7
5 years ago

  • Keywords needs-patch added; 3.2-early removed

#8 @chriscct7
4 years ago

  • Keywords needs-unit-tests added

#10 follow-up: @iandunn
10 months ago

@dd32, do you think this is no longer needed, or should it be reopened?

#11 in reply to: ↑ 10 @dd32
10 months ago

Replying to iandunn:

@dd32, do you think this is no longer needed, or should it be reopened?

@iandunn IMHO: There exists options which could certainly be sanitized as a hardening exercise to preempt any future security issues pertaining to the usage of them unsanitized.

I do not know of any specific vulnerabilities, but I do know that the options handled by the list hasn't changed significantly in the last 9 years. Many options are sanitized elsewhere though, so the lack of the option from the function doesn't mean it's a problem.

As to whether this is worth re-opening, That would depend on if someone was willing to go through the core options, verify the uses of them all and pre-emptively add the simple-validators where needed. If there's not, there's no point in having this ticket still open when there's no known (and nothing came up in near-ten-years which points to this) issues related to it.

Note: See TracTickets for help on using tickets.