WordPress.org

Make WordPress Core

Opened 8 years ago

Last modified 5 months ago

#17821 new defect (bug)

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

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

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 8 years ago.
17821.2.diff (1.7 KB) - added by kawauso 8 years ago.
17821.3.diff (1.7 KB) - added by kawauso 8 years ago.
s/menu_obj/menu

Download all attachments as: .zip

Change History (14)

@kawauso
8 years ago

#1 @kawauso
8 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().

#2 @kawauso
8 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

#3 follow-up: @kawauso
8 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.

@kawauso
8 years ago

@kawauso
8 years ago

s/menu_obj/menu

#4 follow-up: @cburtbcit
8 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!

#5 in reply to: ↑ 4 @kawauso
8 years ago

  • Milestone changed from Awaiting Review to 3.3

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

#6 in reply to: ↑ 3 @duck_
8 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().

#7 @ryan
8 years ago

  • Milestone changed from 3.3 to Future Release

#8 @cburtbcit
8 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!

#9 @duck_
8 years ago

  • Milestone changed from Future Release to 3.4

#10 @ryan
7 years ago

  • Milestone changed from 3.4 to Future Release

#11 @xandanet
5 years ago

Ran across this again in 4.0.1. Can anyone verify?

I worked around the unexpected behaviour by skipping straight to:

get_term_by( 'name', $menu_title, 'nav_menu' );

instead, from my plugin code.

Note: See TracTickets for help on using tickets.