WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 2 years ago

#17821 new defect (bug)

wp_get_nav_menu_object() doesn't check when passing to get_term()

Reported by: kawauso Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 3.1.4
Component: Menus Keywords: has-patch
Focuses: Cc:

Description

wp_get_nav_menu_object() initially passes the menu string to get_term() to see if it's an ID. However, get_term() expects an integer or object and type-casts any non-objects passed into integers. This results in a menu name i.e. '10-ton elephant' being reduced to '10', which it then uses as a term ID, potentially producing unexpected results.

Attachments (3)

17821.diff (636 bytes) - added by kawauso 3 years ago.
17821.2.diff (1.7 KB) - added by kawauso 3 years ago.
17821.3.diff (1.7 KB) - added by kawauso 3 years ago.
s/menu_obj/menu

Download all attachments as: .zip

Change History (13)

kawauso3 years ago

comment:1 kawauso3 years ago

Patch sets $menu_obj to false initially and removes the redundant end false check (get_term_by() will return an object or false, if get_term() has passed null then it causes a get_term_by() call). Also adds an is_numeric() check to get_term().

comment:2 kawauso3 years ago

Steps to reproduce:

  1. Add a nav menu and find its term ID
  2. Call wp_get_nav_menu_object() with a string beginning with that ID
  3. The nav menu will be returned despite the passed string not being a valid menu name

comment:3 follow-up: kawauso3 years ago

wp_get_nav_menu_to_edit() incorrectly passed the menu object to is_nav_menu() (it is only meant to take an id, slug or name). Following patch adds check logic from is_nav_menu() in place of the is_nav_menu() call.

wp_get_nav_menu_to_edit() can still return null when the conditions are not met, so the PHPDoc @return should be expanded to reflect this or handling added to convert null values into WP_Error objects.

kawauso3 years ago

kawauso3 years ago

s/menu_obj/menu

comment:4 follow-up: cburtbcit3 years ago

I can confirm that I have run into this bug as well. The diff provided appears to solve the problem. I don't think it's that unlikely that someone would name a page with an integer as the first character or two in the title so in my opinion it's important that this patch be included in the next release of WordPress. I see that the version number for the bug is listed as 3.1.4 but I'm experiencing this in 3.2.1 as well. Thank you!

comment:5 in reply to: ↑ 4 kawauso3 years ago

  • Milestone changed from Awaiting Review to 3.3

Replying to cburtbcit:
The Version refers to the earliest version affected by the bug.

comment:6 in reply to: ↑ 3 duck_2 years ago

Replying to kawauso:

wp_get_nav_menu_to_edit() incorrectly passed the menu object to is_nav_menu() (it is only meant to take an id, slug or name). Following patch adds check logic from is_nav_menu() in place of the is_nav_menu() call.

Although passing an object was undocumented should we be careful about backwards compatibility here? As you say even core passed an object to is_nav_menu() / wp_get_nav_menu_object().

comment:7 ryan2 years ago

  • Milestone changed from 3.3 to Future Release

comment:8 cburtbcit2 years ago

I've recently had to apply the patch provided by kawauso once again. It still works and does not seem to produce any unexpected behavior. What's keeping this from being included in the next update? Thanks!

comment:9 duck_2 years ago

  • Milestone changed from Future Release to 3.4

comment:10 ryan2 years ago

  • Milestone changed from 3.4 to Future Release
Note: See TracTickets for help on using tickets.